diff mbox

Do not sanitize in lower_omp_target context (PR, sanitizer/78815).

Message ID 04ab26ff-8e08-ec38-792c-57a57e5edfc8@suse.cz
State New
Headers show

Commit Message

Martin Liška Jan. 5, 2017, 8:42 a.m. UTC 
On 01/04/2017 10:31 AM, Jakub Jelinek wrote:
> On Wed, Jan 04, 2017 at 10:19:28AM +0100, Martin Liška wrote:

>> PING^1

>>

>> On 12/16/2016 01:04 PM, Martin Liška wrote:

>>> Currently, use-after-scope relies on fact that entry point of gimplify_decl_expr

>>> is gimplify_function_tree. Fixed by checking if asan_poisoned_variables is non-null.

>>>

>>> Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

>>>

>>> Ready to be installed?

> 

> Looking at asan_poisoned_variables, my preference would be to guard:

>   asan_poisoned_variables = new hash_set<tree> ();

> with

> if (asan_sanitize_use_after_scope ()

>     && !asan_no_sanitize_address_p ())

> the delete asan_poisoned_variables; with if (asan_poisoned_variables)

> and all the poisoning stuff in the gimplifier also with if

> (asan_poisoned_variables) and no need to repeat there the asan_sanitize_use_after_scope

> () and !asan_no_sanitize_address_p () tests.

>       if (asan_poisoned_variables != NULL

>           && asan_poisoned_variables->contains (t))

> is already fine,

>       if (asan_sanitize_use_after_scope ()

>           && !asan_no_sanitize_address_p ()

>           && !is_vla

>           && TREE_ADDRESSABLE (decl)

>           && !TREE_STATIC (decl)

>           && !DECL_HAS_VALUE_EXPR_P (decl)

>           && dbg_cnt (asan_use_after_scope))

> should replace the first 2 conditions with asan_poisoned_variables,

>           if (asan_sanitize_use_after_scope ()

>               && asan_used_labels != NULL

>               && asan_used_labels->contains (label))

>             asan_poison_variables (asan_poisoned_variables, false, pre_p);

> should replace asan_sanitize_use_after_scope () with

> asan_poisoned_variables.  IMHO no need to add comments, especially not one

> mentioning omp lowering - the gimplifier is called from lots of various

> places.

> 

> 	Jakub

> 


I like your approach, I'm sending updated patch which bootstraps and survives
regression tests.

Ready to be installed?
Martin

Comments

Jakub Jelinek Jan. 5, 2017, 9:03 a.m. UTC | #1 
On Thu, Jan 05, 2017 at 09:42:54AM +0100, Martin Liška wrote:
> I like your approach, I'm sending updated patch which bootstraps and survives

> regression tests.

> 

> Ready to be installed?

> Martin


> >From 42887cf5fe7d94709ee5356fb6534c7a4fc26bff Mon Sep 17 00:00:00 2001

> From: marxin <mliska@suse.cz>

> Date: Wed, 4 Jan 2017 16:43:49 +0100

> Subject: [PATCH] Do not sanitize in an abnormal context (PR sanitizer/78815).

> 

> gcc/ChangeLog:

> 

> 2017-01-04  Martin Liska  <mliska@suse.cz>

> 

> 	PR sanitizer/78815

> 	* gimplify.c (gimplify_decl_expr): Compare to

> 	asan_poisoned_variables instread of checking flags.

> 	(gimplify_target_expr): Likewise.

> 	(gimplify_expr): Likewise.

> 	(gimplify_function_tree): Conditionally initialize

> 	asan_poisoned_variables.


Ok, thanks.

	Jakub
diff mbox

Patch

From 42887cf5fe7d94709ee5356fb6534c7a4fc26bff Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Wed, 4 Jan 2017 16:43:49 +0100
Subject: [PATCH] Do not sanitize in an abnormal context (PR sanitizer/78815).

gcc/ChangeLog:

2017-01-04  Martin Liska  <mliska@suse.cz>

	PR sanitizer/78815
	* gimplify.c (gimplify_decl_expr): Compare to
	asan_poisoned_variables instread of checking flags.
	(gimplify_target_expr): Likewise.
	(gimplify_expr): Likewise.
	(gimplify_function_tree): Conditionally initialize
	asan_poisoned_variables.
---
 gcc/gimplify.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 14e79b4b3f3..e1e9ce9e903 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -1620,8 +1620,7 @@  gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p)
 	  is_vla = true;
 	}
 
-      if (asan_sanitize_use_after_scope ()
-	  && !asan_no_sanitize_address_p ()
+      if (asan_poisoned_variables
 	  && !is_vla
 	  && TREE_ADDRESSABLE (decl)
 	  && !TREE_STATIC (decl)
@@ -6413,8 +6412,7 @@  gimplify_target_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
 	      else
 		cleanup = clobber;
 	    }
-	  if (asan_sanitize_use_after_scope ()
-	      && dbg_cnt (asan_use_after_scope))
+	  if (asan_poisoned_variables && dbg_cnt (asan_use_after_scope))
 	    {
 	      tree asan_cleanup = build_asan_poison_call_expr (temp);
 	      if (asan_cleanup)
@@ -11426,7 +11424,7 @@  gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
 	  /* If the label is used in a goto statement, or address of the label
 	     is taken, we need to unpoison all variables that were seen so far.
 	     Doing so would prevent us from reporting a false positives.  */
-	  if (asan_sanitize_use_after_scope ()
+	  if (asan_poisoned_variables
 	      && asan_used_labels != NULL
 	      && asan_used_labels->contains (label))
 	    asan_poison_variables (asan_poisoned_variables, false, pre_p);
@@ -12531,10 +12529,14 @@  gimplify_function_tree (tree fndecl)
       && !needs_to_live_in_memory (ret))
     DECL_GIMPLE_REG_P (ret) = 1;
 
-  asan_poisoned_variables = new hash_set<tree> ();
+  if (asan_sanitize_use_after_scope () && !asan_no_sanitize_address_p ())
+    asan_poisoned_variables = new hash_set<tree> ();
   bind = gimplify_body (fndecl, true);
-  delete asan_poisoned_variables;
-  asan_poisoned_variables = NULL;
+  if (asan_poisoned_variables)
+    {
+      delete asan_poisoned_variables;
+      asan_poisoned_variables = NULL;
+    }
 
   /* The tree body of the function is no longer needed, replace it
      with the new GIMPLE body.  */
-- 
2.11.0