diff mbox series

gimple-low: Remove .ASAN_MARK calls on TREE_STATIC variables [PR113531]

Message ID ZbplKibgH5/xoVso@tucnak
State Unresolved
Headers
Series gimple-low: Remove .ASAN_MARK calls on TREE_STATIC variables [PR113531] |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Jakub Jelinek Jan. 31, 2024, 3:20 p.m. UTC 
  On Wed, Jan 31, 2024 at 01:04:22PM +0100, Richard Biener wrote:
> > Like this, so far just tested on the testcase.  Ok for trunk if it passes
> > bootstrap/regtest on top of Jason's patch?
> 
> Note we fold all - well, all builtin - calls during gimple lowering.

Internal calls aren't builtin, so those aren't folded.

> Maybe we can put this special-casing there instead? (gimple-low.cc:797,
> you possibly have to replace with a GIMPLE_NOP)

But sure, we can do it there as well.
In that case I think we don't need it in asan.cc though, because
gimple-low.cc is guaranteed to be performed on all statements from
gimplification that survive in the IL.

So like this?

2024-01-31  Jakub Jelinek  <jakub@redhat.com>
	    Jason Merrill  <jason@redhat.com>

	PR c++/113531
	* gimple-low.cc (lower_stmt): Remove .ASAN_MARK calls
	on variables which were promoted to TREE_STATIC.

	* g++.dg/asan/initlist1.C: New test.



	Jakub

Comments

Richard Biener Jan. 31, 2024, 5:57 p.m. UTC | #1 
> Am 31.01.2024 um 16:20 schrieb Jakub Jelinek <jakub@redhat.com>:
> 
> On Wed, Jan 31, 2024 at 01:04:22PM +0100, Richard Biener wrote:
>>> Like this, so far just tested on the testcase.  Ok for trunk if it passes
>>> bootstrap/regtest on top of Jason's patch?
>> 
>> Note we fold all - well, all builtin - calls during gimple lowering.
> 
> Internal calls aren't builtin, so those aren't folded.
> 
>> Maybe we can put this special-casing there instead? (gimple-low.cc:797,
>> you possibly have to replace with a GIMPLE_NOP)
> 
> But sure, we can do it there as well.
> In that case I think we don't need it in asan.cc though, because
> gimple-low.cc is guaranteed to be performed on all statements from
> gimplification that survive in the IL.
> 
> So like this?

Lgtm,

Thanks,
Richard 

> 2024-01-31  Jakub Jelinek  <jakub@redhat.com>
>        Jason Merrill  <jason@redhat.com>
> 
>    PR c++/113531
>    * gimple-low.cc (lower_stmt): Remove .ASAN_MARK calls
>    on variables which were promoted to TREE_STATIC.
> 
>    * g++.dg/asan/initlist1.C: New test.
> 
> --- gcc/gimple-low.cc.jj    2024-01-03 11:51:22.133861623 +0100
> +++ gcc/gimple-low.cc    2024-01-31 16:16:21.326927572 +0100
> @@ -790,6 +790,21 @@ lower_stmt (gimple_stmt_iterator *gsi, s
>        return;
>      }
> 
> +    if (gimple_call_internal_p (stmt, IFN_ASAN_MARK))
> +      {
> +        tree base = gimple_call_arg (stmt, 1);
> +        gcc_checking_assert (TREE_CODE (base) == ADDR_EXPR);
> +        tree decl = TREE_OPERAND (base, 0);
> +        if (VAR_P (decl) && TREE_STATIC (decl))
> +          {
> +        /* Don't poison a variable with static storage; it might have
> +           gotten marked before gimplify_init_constructor promoted it
> +           to static.  */
> +        gsi_remove (gsi, true);
> +        return;
> +          }
> +      }
> +
>    /* We delay folding of built calls from gimplification to
>       here so the IL is in consistent state for the diagnostic
>       machineries job.  */
> --- gcc/testsuite/g++.dg/asan/initlist1.C.jj    2024-01-31 16:09:58.071289511 +0100
> +++ gcc/testsuite/g++.dg/asan/initlist1.C    2024-01-31 16:09:58.071289511 +0100
> @@ -0,0 +1,20 @@
> +// PR c++/113531
> +// { dg-do run { target c++11 } }
> +// { dg-additional-options "-fsanitize=address" }
> +
> +#include <initializer_list>
> +
> +void f(int) { }
> +
> +void g()
> +{
> +  for (auto i : { 1, 2, 3 })
> +    f (i);
> +  f(42);
> +}
> +
> +int main()
> +{
> +  g();
> +  g();
> +}
> 
> 
>    Jakub
>
diff mbox series

Patch

--- gcc/gimple-low.cc.jj	2024-01-03 11:51:22.133861623 +0100
+++ gcc/gimple-low.cc	2024-01-31 16:16:21.326927572 +0100
@@ -790,6 +790,21 @@  lower_stmt (gimple_stmt_iterator *gsi, s
 	    return;
 	  }
 
+	if (gimple_call_internal_p (stmt, IFN_ASAN_MARK))
+	  {
+	    tree base = gimple_call_arg (stmt, 1);
+	    gcc_checking_assert (TREE_CODE (base) == ADDR_EXPR);
+	    tree decl = TREE_OPERAND (base, 0);
+	    if (VAR_P (decl) && TREE_STATIC (decl))
+	      {
+		/* Don't poison a variable with static storage; it might have
+		   gotten marked before gimplify_init_constructor promoted it
+		   to static.  */
+		gsi_remove (gsi, true);
+		return;
+	      }
+	  }
+
 	/* We delay folding of built calls from gimplification to
 	   here so the IL is in consistent state for the diagnostic
 	   machineries job.  */
--- gcc/testsuite/g++.dg/asan/initlist1.C.jj	2024-01-31 16:09:58.071289511 +0100
+++ gcc/testsuite/g++.dg/asan/initlist1.C	2024-01-31 16:09:58.071289511 +0100
@@ -0,0 +1,20 @@ 
+// PR c++/113531
+// { dg-do run { target c++11 } }
+// { dg-additional-options "-fsanitize=address" }
+
+#include <initializer_list>
+
+void f(int) { }
+
+void g()
+{
+  for (auto i : { 1, 2, 3 })
+    f (i);
+  f(42);
+}
+
+int main()
+{
+  g();
+  g();
+}