Re: [PATCH] asan: Fix ICE during instrumentation of returns_twice calls [PR112709]

[Richard Biener <rguenther@suse.de>]

On Tue, 12 Mar 2024, Jakub Jelinek wrote:

> Hi!
> 
> The following patch on top of the previously posted ubsan/gimple-iterator
> one handles asan the same.  While the case of returning by hidden reference
> is handled differently because of the first recently posted asan patch,
> this deals with instrumentation of the aggregates returned in registers
> case as well as instrumentation of loads from aggregate memory in the
> function arguments of returns_twice calls.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

> 2024-03-12  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR sanitizer/112709
> 	* asan.cc (asan_insert_before): New function.
> 	(maybe_create_ssa_name, maybe_cast_to_ptrmode, build_check_stmt,
> 	maybe_instrument_call, asan_expand_mark_ifn): Use it instead of
> 	gsi_insert_before.
> 
> 	* gcc.dg/asan/pr112709-2.c: New test.
> 
> --- gcc/asan.cc.jj	2024-03-11 13:49:58.931045179 +0100
> +++ gcc/asan.cc	2024-03-11 18:38:29.047330489 +0100
> @@ -2561,6 +2561,21 @@ build_shadow_mem_access (gimple_stmt_ite
>    return gimple_assign_lhs (g);
>  }
>  
> +/* Insert G stmt before ITER.  If ITER is a returns_twice call,
> +   insert it on an appropriate edge instead.  */
> +
> +static void
> +asan_insert_before (gimple_stmt_iterator *iter, gimple *g)
> +{
> +  gimple *stmt = gsi_stmt (*iter);
> +  if (stmt
> +      && is_gimple_call (stmt)
> +      && (gimple_call_flags (stmt) & ECF_RETURNS_TWICE) != 0)
> +    gsi_insert_before_returns_twice_call (gsi_bb (*iter), g);
> +  else
> +    gsi_insert_before (iter, g, GSI_SAME_STMT);
> +}
> +
>  /* BASE can already be an SSA_NAME; in that case, do not create a
>     new SSA_NAME for it.  */
>  
> @@ -2574,7 +2589,7 @@ maybe_create_ssa_name (location_t loc, t
>    gimple *g = gimple_build_assign (make_ssa_name (TREE_TYPE (base)), base);
>    gimple_set_location (g, loc);
>    if (before_p)
> -    gsi_insert_before (iter, g, GSI_SAME_STMT);
> +    asan_insert_before (iter, g);
>    else
>      gsi_insert_after (iter, g, GSI_NEW_STMT);
>    return gimple_assign_lhs (g);
> @@ -2593,7 +2608,7 @@ maybe_cast_to_ptrmode (location_t loc, t
>  				  NOP_EXPR, len);
>    gimple_set_location (g, loc);
>    if (before_p)
> -    gsi_insert_before (iter, g, GSI_SAME_STMT);
> +    asan_insert_before (iter, g);
>    else
>      gsi_insert_after (iter, g, GSI_NEW_STMT);
>    return gimple_assign_lhs (g);
> @@ -2684,7 +2699,7 @@ build_check_stmt (location_t loc, tree b
>  						 align / BITS_PER_UNIT));
>    gimple_set_location (g, loc);
>    if (before_p)
> -    gsi_insert_before (&gsi, g, GSI_SAME_STMT);
> +    asan_insert_before (&gsi, g);
>    else
>      {
>        gsi_insert_after (&gsi, g, GSI_NEW_STMT);
> @@ -3025,7 +3040,7 @@ maybe_instrument_call (gimple_stmt_itera
>  	  tree decl = builtin_decl_implicit (BUILT_IN_ASAN_HANDLE_NO_RETURN);
>  	  gimple *g = gimple_build_call (decl, 0);
>  	  gimple_set_location (g, gimple_location (stmt));
> -	  gsi_insert_before (iter, g, GSI_SAME_STMT);
> +	  asan_insert_before (iter, g);
>  	}
>      }
>  
> @@ -3852,7 +3867,7 @@ asan_expand_mark_ifn (gimple_stmt_iterat
>        g = gimple_build_assign (make_ssa_name (pointer_sized_int_node),
>  			       NOP_EXPR, len);
>        gimple_set_location (g, loc);
> -      gsi_insert_before (iter, g, GSI_SAME_STMT);
> +      asan_insert_before (iter, g);
>        tree sz_arg = gimple_assign_lhs (g);
>  
>        tree fun
> --- gcc/testsuite/gcc.dg/asan/pr112709-2.c.jj	2024-03-11 18:30:59.813488200 +0100
> +++ gcc/testsuite/gcc.dg/asan/pr112709-2.c	2024-03-11 18:31:06.506396462 +0100
> @@ -0,0 +1,50 @@
> +/* PR sanitizer/112709 */
> +/* { dg-do compile } */
> +/* { dg-options "-fsanitize=address -O2" } */
> +
> +struct S { char c[1024]; } *p;
> +int foo (int);
> +
> +__attribute__((returns_twice, noipa)) int
> +bar (struct S x)
> +{
> +  (void) x.c[0];
> +  return 0;
> +}
> +
> +void
> +baz (int *y)
> +{
> +  foo (1);
> +  *y = bar (*p);
> +}
> +
> +void
> +qux (int x, int *y)
> +{
> +  if (x == 25)
> +    x = foo (2);
> +  else if (x == 42)
> +    x = foo (foo (3));
> +  *y = bar (*p);
> +}
> +
> +void
> +corge (int x, int *y)
> +{
> +  void *q[] = { &&l1, &&l2, &&l3, &&l3 };
> +  if (x == 25)
> +    {
> +    l1:
> +      x = foo (2);
> +    }
> +  else if (x == 42)
> +    {
> +    l2:
> +      x = foo (foo (3));
> +    }
> +l3:
> +  *y = bar (*p);
> +  if (x < 4)
> +    goto *q[x & 3];
> +}
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)