diff mbox series

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

Message ID ZfAcBrfzY8QKROAX@tucnak
State New
Headers show
Series asan: Fix ICE during instrumentation of returns_twice calls [PR112709] | expand

Commit Message

Jakub Jelinek March 12, 2024, 9:10 a.m. UTC
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?

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.


	Jakub

Comments

Richard Biener March 12, 2024, 1:46 p.m. UTC | #1
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
> 
>
diff mbox series

Patch

--- 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];
+}