diff mbox

Fix ICE in use-after-scope w/ -fno-tree-dce (PR, sanitize/79783).

Message ID 11a8ab88-b609-12fc-6518-bcec1589c3df@suse.cz
State New
Headers show

Commit Message

Martin Liška March 6, 2017, 7:59 a.m. UTC
On 03/03/2017 01:57 PM, Jakub Jelinek wrote:
> On Thu, Mar 02, 2017 at 06:49:32PM +0100, Martin Liška wrote:
>> It can happen with inlining and -fno-tree-dce that VAR_DECL for a SSA
>> NAME was removed and thus the poisoning should not have any usage.
>>
>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
>>
>> Ready to be installed?
>> Martin
> 
>> >From d8aa72dc1d696f5500c00b6c2f532f2a87cf58d2 Mon Sep 17 00:00:00 2001
>> From: marxin <mliska@suse.cz>
>> Date: Thu, 2 Mar 2017 11:55:00 +0100
>> Subject: [PATCH] Fix ICE in use-after-scope w/ -fno-tree-dce (PR
>>  sanitize/79783).
>>
>> gcc/ChangeLog:
>>
>> 2017-03-02  Martin Liska  <mliska@suse.cz>
>>
>> 	PR sanitize/79783
>> 	* asan.c (asan_expand_poison_ifn): Do not expand ASAN_POISON
>> 	when having a SSA NAME w/o VAR_DECL assigned to it.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2017-03-02  Martin Liska  <mliska@suse.cz>
>>
>> 	PR sanitize/79783
>> 	* g++.dg/asan/pr79783.C: New test.
>> ---
>>  gcc/asan.c                          |  5 ++++-
>>  gcc/testsuite/g++.dg/asan/pr79783.C | 19 +++++++++++++++++++
>>  2 files changed, 23 insertions(+), 1 deletion(-)
>>  create mode 100644 gcc/testsuite/g++.dg/asan/pr79783.C
>>
>> diff --git a/gcc/asan.c b/gcc/asan.c
>> index 6cdd59b7ea7..307423ced03 100644
>> --- a/gcc/asan.c
>> +++ b/gcc/asan.c
>> @@ -3107,7 +3107,10 @@ asan_expand_poison_ifn (gimple_stmt_iterator *iter,
>>  {
>>    gimple *g = gsi_stmt (*iter);
>>    tree poisoned_var = gimple_call_lhs (g);
>> -  if (!poisoned_var)
>> +
>> +  /* It can happen with inlining and -fno-tree-dce that VAR_DECL for a SSA
>> +     NAME was removed and thus the poisoning should not have any usage.  */
>> +  if (!poisoned_var || SSA_NAME_VAR (poisoned_var) == NULL_TREE)
> 
> I wonder if it wouldn't be better to do:
>   if (!poisoned_var || has_zero_uses (poisoned_var))
> 
> perhaps with -fno-tree-dce we could end up with SSA_NAME_VAR being
> non-NULL, yet no uses; in that case there is nothing to warn about.
> On the other side, in theory we could also end up with anonymous SSA_NAME
> and some uses - in that case perhaps it would be better to warn.
> So do:
>   if (SSA_NAME_VAR (poisoned_var) == NULL_TREE)
>     SSA_NAME_VAR (poisoned_var) = create_tmp_var (TREE_TYPE (poisoned_var));
>   tree shadow_var = create_asan_shadow_var (SSA_NAME_VAR (poisoned_var),
>                                             shadow_vars_mapping);
> or so?  We'll need SSA_NAME_VAR non-NULL so that we can use a default def
> later.
> 
> 	Jakub
> 

Ok, I've just prepared and tested following patch that does what Jakub suggested.Hi.
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Martin

Comments

Jakub Jelinek March 6, 2017, 8:09 a.m. UTC | #1
On Mon, Mar 06, 2017 at 08:59:09AM +0100, Martin Liška wrote:
> >> --- a/gcc/asan.c
> >> +++ b/gcc/asan.c
> >> @@ -3107,7 +3107,10 @@ asan_expand_poison_ifn (gimple_stmt_iterator *iter,
> >>  {
> >>    gimple *g = gsi_stmt (*iter);
> >>    tree poisoned_var = gimple_call_lhs (g);
> >> -  if (!poisoned_var)
> >> +
> >> +  /* It can happen with inlining and -fno-tree-dce that VAR_DECL for a SSA
> >> +     NAME was removed and thus the poisoning should not have any usage.  */
> >> +  if (!poisoned_var || SSA_NAME_VAR (poisoned_var) == NULL_TREE)
> > 
> > I wonder if it wouldn't be better to do:
> >   if (!poisoned_var || has_zero_uses (poisoned_var))
> > 
> > perhaps with -fno-tree-dce we could end up with SSA_NAME_VAR being
> > non-NULL, yet no uses; in that case there is nothing to warn about.
> > On the other side, in theory we could also end up with anonymous SSA_NAME
> > and some uses - in that case perhaps it would be better to warn.
> > So do:
> >   if (SSA_NAME_VAR (poisoned_var) == NULL_TREE)
> >     SSA_NAME_VAR (poisoned_var) = create_tmp_var (TREE_TYPE (poisoned_var));
> >   tree shadow_var = create_asan_shadow_var (SSA_NAME_VAR (poisoned_var),
> >                                             shadow_vars_mapping);
> > or so?  We'll need SSA_NAME_VAR non-NULL so that we can use a default def
> > later.
> 
> Ok, I've just prepared and tested following patch that does what Jakub suggested.Hi.
> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
> --- a/gcc/asan.c
> +++ b/gcc/asan.c

Can you please add also the suggested
 {   
   gimple *g = gsi_stmt (*iter);
   tree poisoned_var = gimple_call_lhs (g);
-  if (!poisoned_var)
+  if (!poisoned_var || has_zero_uses (poisoned_var))
     {
       gsi_remove (iter, true);
       return true;
hunk into the same function?  If we don't do DCE, we can end up with
ASAN_POISON with lhs but not really used anywhere.  If there are no uses,
it is a poisoned use.

> @@ -3113,6 +3113,10 @@ asan_expand_poison_ifn (gimple_stmt_iterator *iter,
>        return true;
>      }
>  
> +  if (SSA_NAME_VAR (poisoned_var) == NULL_TREE)
> +    SET_SSA_NAME_VAR_OR_IDENTIFIER (poisoned_var,
> +				    create_tmp_var (TREE_TYPE (poisoned_var)));
> +
>    tree shadow_var = create_asan_shadow_var (SSA_NAME_VAR (poisoned_var),
>  					    shadow_vars_mapping);
>  

Ok with that change.

	Jakub
diff mbox

Patch

From bbbd4958fb95071e703efda8119de68cc252523f Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Thu, 2 Mar 2017 11:55:00 +0100
Subject: [PATCH] Fix ICE in use-after-scope w/ -fno-tree-dce (PR
 sanitize/79783).

gcc/ChangeLog:

2017-03-02  Martin Liska  <mliska@suse.cz>

	PR sanitize/79783
	* asan.c (asan_expand_poison_ifn): Do not expand ASAN_POISON
	when having a SSA NAME w/o VAR_DECL assigned to it.

gcc/testsuite/ChangeLog:

2017-03-02  Martin Liska  <mliska@suse.cz>

	PR sanitize/79783
	* g++.dg/asan/pr79783.C: New test.
---
 gcc/asan.c                          |  4 ++++
 gcc/testsuite/g++.dg/asan/pr79783.C | 19 +++++++++++++++++++
 2 files changed, 23 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/asan/pr79783.C

diff --git a/gcc/asan.c b/gcc/asan.c
index 6cdd59b7ea7..2e7dd04075f 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -3113,6 +3113,10 @@  asan_expand_poison_ifn (gimple_stmt_iterator *iter,
       return true;
     }
 
+  if (SSA_NAME_VAR (poisoned_var) == NULL_TREE)
+    SET_SSA_NAME_VAR_OR_IDENTIFIER (poisoned_var,
+				    create_tmp_var (TREE_TYPE (poisoned_var)));
+
   tree shadow_var = create_asan_shadow_var (SSA_NAME_VAR (poisoned_var),
 					    shadow_vars_mapping);
 
diff --git a/gcc/testsuite/g++.dg/asan/pr79783.C b/gcc/testsuite/g++.dg/asan/pr79783.C
new file mode 100644
index 00000000000..939f60b2819
--- /dev/null
+++ b/gcc/testsuite/g++.dg/asan/pr79783.C
@@ -0,0 +1,19 @@ 
+// PR sanitizer/79783
+// { dg-options "-fno-tree-dce" }
+
+struct A
+{
+  static void foo(const char&) {}
+};
+
+struct B
+{
+  B() { A::foo(char()); }
+};
+
+struct C
+{
+  virtual void bar() const { B b; }
+};
+
+C c;
-- 
2.11.1