public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] asan: Fix ICE during instrumentation of returns_twice calls [PR112709]
@ 2024-03-12  9:10 Jakub Jelinek
  2024-03-12 13:46 ` Richard Biener
  0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2024-03-12  9:10 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

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.

--- 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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] asan: Fix ICE during instrumentation of returns_twice calls [PR112709]
  2024-03-12  9:10 [PATCH] asan: Fix ICE during instrumentation of returns_twice calls [PR112709] Jakub Jelinek
@ 2024-03-12 13:46 ` Richard Biener
  2024-03-13  8:21   ` [committed] asan, v2: " Jakub Jelinek
  0 siblings, 1 reply; 3+ messages in thread
From: Richard Biener @ 2024-03-12 13:46 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: gcc-patches

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)

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [committed] asan, v2: Fix ICE during instrumentation of returns_twice calls [PR112709]
  2024-03-12 13:46 ` Richard Biener
@ 2024-03-13  8:21   ` Jakub Jelinek
  0 siblings, 0 replies; 3+ messages in thread
From: Jakub Jelinek @ 2024-03-13  8:21 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

On Tue, Mar 12, 2024 at 02:46:07PM +0100, Richard Biener wrote:
> OK.

Thanks.  Here is the actually committed version which uses
gsi_safe_insert_before instead.

Bootstrapped/regtested on x86_64-linux and i686-linux, committed to
trunk.

2024-03-13  Jakub Jelinek  <jakub@redhat.com>

	PR sanitizer/112709
	* asan.cc (maybe_create_ssa_name, maybe_cast_to_ptrmode,
	build_check_stmt, maybe_instrument_call, asan_expand_mark_ifn): Use
	gsi_safe_insert_before 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
@@ -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);
+    gsi_safe_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);
+    gsi_safe_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);
+    gsi_safe_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);
+	  gsi_safe_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);
+      gsi_safe_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


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-03-13  8:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-12  9:10 [PATCH] asan: Fix ICE during instrumentation of returns_twice calls [PR112709] Jakub Jelinek
2024-03-12 13:46 ` Richard Biener
2024-03-13  8:21   ` [committed] asan, v2: " Jakub Jelinek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).