public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Martin Liška" <mliska@suse.cz>
To: Jakub Jelinek <jakub@redhat.com>
Cc: Richard Biener <richard.guenther@gmail.com>,
	GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] Speed-up use-after-scope (re-writing to SSA) (version 2)
Date: Mon, 23 Jan 2017 09:38:00 -0000	[thread overview]
Message-ID: <4e0e7dbe-e770-bc77-b25f-6f38f5125e89@suse.cz> (raw)
In-Reply-To: <20170120142717.GT1867@tucnak>

[-- Attachment #1: Type: text/plain, Size: 2674 bytes --]

On 01/20/2017 03:27 PM, Jakub Jelinek wrote:
> On Fri, Jan 20, 2017 at 03:08:21PM +0100, Martin Liška wrote:
>> Unfortunately this way would not work as clobber marks content of the memory as uninitialize
>> is different behavior that just marking a memory can be used (and maybe already contains a value).
>>
>> This shows the problem:
>>
>> #include <string.h>
>>
>> char cc;
>> char ptr[] = "sparta2";
>>
>> void get(char **x)
>> {
>>   *x = ptr;
>> }
>>   
>> int main()
>> {
>>   char *here = &cc;
>>
>>   for (;;)
>>     {
>>     next_line:
>> 	if (here == NULL)
>> 	  __builtin_abort();
>> 	get (&here);
>> 	if (strcmp (here, "sparta") == 0)
>> 	    goto next_line;
>> 	else if (strcmp (here, "sparta2") == 0)
>> 	  break;
>>     }
>> }
>>
>> With the patch, DSE would optimize out '*here = &cc;' and thus aborts. The problem is definitely
>> related to goto magic, where we are more defensive in placement of ASAN_MARK(UNPOISON,...).
>> Hope your optimization is still valid for situations w/o artificial ASAN_MARK(UNPOISON,...) placed due
>> to goto magic.
>>
>> Do we still want to do it now, or postponing to GCC 8 would be better option?
> 
> I'd still like to resolve it for GCC 7 if at all possible, I think otherwise
> -fsanitize=address is by default unnecessarily slower (so it is a regression
> anyway).

Good, I hope I have patch that finally works as we want. It add attribute to variables that
are unpoisoned as live switch variables, or are defined in a label which address is taken.

With [1] I can bootstrap-asan and the patch can bootstrap on ppc64le-redhat-linux
and survives regression tests.

I'm sending latest diff, as well as the final pair of patches I would like to install.

Ready to be installed?
Martin

> So, do we always amit ASAN_MARK(UNPOISON, ...) at the start of scope and
> then yet another ASAN_MARK(UNPOISON, ...) at the goto destination?
> At least on the above testcase that is the case, so if we say split
> ASAN_MARK_UNPOISON into something that is used at the start of scope
> (we'd emit the clobber for those) and others (we would not), then perhaps we
> could get around that.  The above is BTW a clear case where shouldn't emit
> UNPOISON on the label, as the goto doesn't cross its initialization.
> But I can see with gotos from outside of some var's scope into it we
> wouldn't handle it properly.  Perhaps for now set some
> flag/attribute/whatever on vars for which we emit the conservative
> UNPOISON and never allow those to be made non-addressable (i.e. for those
> say that POISON/UNPOISON actually makes them always addressable)?
> 
> 	Jakub
> 

[1] https://gcc.gnu.org/ml/gcc-patches/2017-01/msg01446.html


[-- Attachment #2: 0001-Speed-up-use-after-scope-v2-rewrite-into-SSA.patch --]
[-- Type: text/x-patch, Size: 13827 bytes --]

From 97a900584714411521fc8ad3c7b99b0e74955fe6 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Mon, 19 Dec 2016 15:36:11 +0100
Subject: [PATCH 1/2] Speed up use-after-scope (v2): rewrite into SSA

gcc/ChangeLog:

2016-12-22  Martin Liska  <mliska@suse.cz>

	* asan.c (create_asan_shadow_var): New function.
	(asan_expand_poison_ifn): Likewise.
	* asan.h (asan_expand_poison_ifn): New declaration.
	* internal-fn.c (expand_ASAN_POISON): Likewise.
	* internal-fn.def (ASAN_POISON): New builtin.
	* sanopt.c (pass_sanopt::execute): Expand
	asan_expand_poison_ifn.
	* tree-inline.c (copy_decl_for_dup_finish): Make function
	external.
	* tree-inline.h (copy_decl_for_dup_finish): Likewise.
	* tree-ssa.c (is_asan_mark_p): New function.
	(execute_update_addresses_taken): Rewrite local variables
	(identified just by use-after-scope as addressable) into SSA.

gcc/testsuite/ChangeLog:

2016-12-22  Martin Liska  <mliska@suse.cz>

	* gcc.dg/asan/use-after-scope-3.c: Add additional flags.
	* gcc.dg/asan/use-after-scope-9.c: Likewise and grep for
	sanopt optimization for ASAN_POISON.
---
 gcc/asan.c                                    | 109 +++++++++++++++++++++++++-
 gcc/asan.h                                    |   2 +
 gcc/internal-fn.c                             |   7 ++
 gcc/internal-fn.def                           |   1 +
 gcc/sanopt.c                                  |  11 +++
 gcc/testsuite/gcc.dg/asan/use-after-scope-3.c |   1 +
 gcc/testsuite/gcc.dg/asan/use-after-scope-9.c |   2 +
 gcc/tree-inline.c                             |   2 +-
 gcc/tree-inline.h                             |   1 +
 gcc/tree-ssa.c                                |  69 +++++++++++++---
 10 files changed, 193 insertions(+), 12 deletions(-)

diff --git a/gcc/asan.c b/gcc/asan.c
index 9a59fe4f100..4aa5a4015ea 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -32,8 +32,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-pass.h"
 #include "memmodel.h"
 #include "tm_p.h"
+#include "ssa.h"
 #include "stringpool.h"
-#include "tree-vrp.h"
 #include "tree-ssanames.h"
 #include "optabs.h"
 #include "emit-rtl.h"
@@ -59,6 +59,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "params.h"
 #include "builtins.h"
 #include "fnmatch.h"
+#include "tree-inline.h"
 
 /* AddressSanitizer finds out-of-bounds and use-after-free bugs
    with <2x slowdown on average.
@@ -3087,6 +3088,112 @@ asan_expand_check_ifn (gimple_stmt_iterator *iter, bool use_calls)
   return true;
 }
 
+/* Create ASAN shadow variable for a VAR_DECL which has been rewritten
+   into SSA.  Already seen VAR_DECLs are stored in SHADOW_VARS_MAPPING.  */
+
+static tree
+create_asan_shadow_var (tree var_decl,
+			hash_map<tree, tree> &shadow_vars_mapping)
+{
+  tree *slot = shadow_vars_mapping.get (var_decl);
+  if (slot == NULL)
+    {
+      tree shadow_var = copy_node (var_decl);
+
+      copy_body_data id;
+      memset (&id, 0, sizeof (copy_body_data));
+      id.src_fn = id.dst_fn = current_function_decl;
+      copy_decl_for_dup_finish (&id, var_decl, shadow_var);
+
+      DECL_ARTIFICIAL (shadow_var) = 1;
+      DECL_IGNORED_P (shadow_var) = 1;
+      DECL_SEEN_IN_BIND_EXPR_P (shadow_var) = 0;
+      gimple_add_tmp_var (shadow_var);
+
+      shadow_vars_mapping.put (var_decl, shadow_var);
+      return shadow_var;
+    }
+  else
+    return *slot;
+}
+
+bool
+asan_expand_poison_ifn (gimple_stmt_iterator *iter,
+			bool *need_commit_edge_insert,
+			hash_map<tree, tree> &shadow_vars_mapping)
+{
+  gimple *g = gsi_stmt (*iter);
+  tree poisoned_var = gimple_call_lhs (g);
+  if (!poisoned_var)
+    {
+      gsi_remove (iter, true);
+      return true;
+    }
+
+  tree shadow_var  = create_asan_shadow_var (SSA_NAME_VAR (poisoned_var),
+					     shadow_vars_mapping);
+
+  bool recover_p;
+  if (flag_sanitize & SANITIZE_USER_ADDRESS)
+    recover_p = (flag_sanitize_recover & SANITIZE_USER_ADDRESS) != 0;
+  else
+    recover_p = (flag_sanitize_recover & SANITIZE_KERNEL_ADDRESS) != 0;
+  tree size = DECL_SIZE_UNIT (shadow_var);
+  gimple *poison_call
+    = gimple_build_call_internal (IFN_ASAN_MARK, 3,
+				  build_int_cst (integer_type_node,
+						 ASAN_MARK_POISON),
+				  build_fold_addr_expr (shadow_var), size);
+
+  use_operand_p use_p;
+  imm_use_iterator imm_iter;
+  FOR_EACH_IMM_USE_FAST (use_p, imm_iter, poisoned_var)
+    {
+      gimple *use = USE_STMT (use_p);
+      if (is_gimple_debug (use))
+	continue;
+
+      int nargs;
+      tree fun = report_error_func (false, recover_p, tree_to_uhwi (size),
+				    &nargs);
+
+      gcall *call = gimple_build_call (fun, 1,
+				       build_fold_addr_expr (shadow_var));
+      gimple_set_location (call, gimple_location (use));
+      gimple *call_to_insert = call;
+
+      /* The USE can be a gimple PHI node.  If so, insert the call on
+	 all edges leading to the PHI node.  */
+      if (is_a <gphi *> (use))
+	{
+	  gphi *phi = dyn_cast<gphi *> (use);
+	  for (unsigned i = 0; i < gimple_phi_num_args (phi); ++i)
+	    if (gimple_phi_arg_def (phi, i) == poisoned_var)
+	      {
+		edge e = gimple_phi_arg_edge (phi, i);
+
+		if (call_to_insert == NULL)
+		  call_to_insert = gimple_copy (call);
+
+		gsi_insert_seq_on_edge (e, call_to_insert);
+		*need_commit_edge_insert = true;
+		call_to_insert = NULL;
+	      }
+	}
+      else
+	{
+	  gimple_stmt_iterator gsi = gsi_for_stmt (use);
+	  gsi_insert_before (&gsi, call, GSI_NEW_STMT);
+	}
+    }
+
+  SSA_NAME_IS_DEFAULT_DEF (poisoned_var) = true;
+  SSA_NAME_DEF_STMT (poisoned_var) = gimple_build_nop ();
+  gsi_replace (iter, poison_call, false);
+
+  return true;
+}
+
 /* Instrument the current function.  */
 
 static unsigned int
diff --git a/gcc/asan.h b/gcc/asan.h
index 2f1f2eeaba7..2895bdee645 100644
--- a/gcc/asan.h
+++ b/gcc/asan.h
@@ -30,6 +30,8 @@ extern void initialize_sanitizer_builtins (void);
 extern tree asan_dynamic_init_call (bool);
 extern bool asan_expand_check_ifn (gimple_stmt_iterator *, bool);
 extern bool asan_expand_mark_ifn (gimple_stmt_iterator *);
+extern bool asan_expand_poison_ifn (gimple_stmt_iterator *, bool *,
+				    hash_map<tree, tree> &);
 
 extern gimple_stmt_iterator create_cond_insert_point
      (gimple_stmt_iterator *, bool, bool, bool, basic_block *, basic_block *);
diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
index 5d71cb2e08d..45e4ce05b86 100644
--- a/gcc/internal-fn.c
+++ b/gcc/internal-fn.c
@@ -380,6 +380,13 @@ expand_ASAN_MARK (internal_fn, gcall *)
   gcc_unreachable ();
 }
 
+/* This should get expanded in the sanopt pass.  */
+
+static void
+expand_ASAN_POISON (internal_fn, gcall *)
+{
+  gcc_unreachable ();
+}
 
 /* This should get expanded in the tsan pass.  */
 
diff --git a/gcc/internal-fn.def b/gcc/internal-fn.def
index 4bf8383a77e..7b28b6722ff 100644
--- a/gcc/internal-fn.def
+++ b/gcc/internal-fn.def
@@ -167,6 +167,7 @@ DEF_INTERNAL_FN (ABNORMAL_DISPATCHER, ECF_NORETURN, NULL)
 DEF_INTERNAL_FN (BUILTIN_EXPECT, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
 DEF_INTERNAL_FN (ASAN_CHECK, ECF_TM_PURE | ECF_LEAF | ECF_NOTHROW, ".R...")
 DEF_INTERNAL_FN (ASAN_MARK, ECF_LEAF | ECF_NOTHROW, ".R..")
+DEF_INTERNAL_FN (ASAN_POISON, ECF_LEAF | ECF_NOTHROW | ECF_NOVOPS, NULL)
 DEF_INTERNAL_FN (ADD_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
 DEF_INTERNAL_FN (SUB_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
 DEF_INTERNAL_FN (MUL_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
diff --git a/gcc/sanopt.c b/gcc/sanopt.c
index 55e07c0c646..70b7aeb80d3 100644
--- a/gcc/sanopt.c
+++ b/gcc/sanopt.c
@@ -894,6 +894,8 @@ pass_sanopt::execute (function *fun)
   bool use_calls = ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD < INT_MAX
     && asan_num_accesses >= ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD;
 
+  hash_map<tree, tree> shadow_vars_mapping;
+  bool need_commit_edge_insert = false;
   FOR_EACH_BB_FN (bb, fun)
     {
       gimple_stmt_iterator gsi;
@@ -931,6 +933,11 @@ pass_sanopt::execute (function *fun)
 		case IFN_ASAN_MARK:
 		  no_next = asan_expand_mark_ifn (&gsi);
 		  break;
+		case IFN_ASAN_POISON:
+		  no_next = asan_expand_poison_ifn (&gsi,
+						    &need_commit_edge_insert,
+						    shadow_vars_mapping);
+		  break;
 		default:
 		  break;
 		}
@@ -962,6 +969,10 @@ pass_sanopt::execute (function *fun)
 	    gsi_next (&gsi);
 	}
     }
+
+  if (need_commit_edge_insert)
+    gsi_commit_edge_inserts ();
+
   return 0;
 }
 
diff --git a/gcc/testsuite/gcc.dg/asan/use-after-scope-3.c b/gcc/testsuite/gcc.dg/asan/use-after-scope-3.c
index 9aeed51a770..8b11bea9940 100644
--- a/gcc/testsuite/gcc.dg/asan/use-after-scope-3.c
+++ b/gcc/testsuite/gcc.dg/asan/use-after-scope-3.c
@@ -1,5 +1,6 @@
 // { dg-do run }
 // { dg-shouldfail "asan" }
+// { dg-additional-options "-O0" }
 
 int
 main (void)
diff --git a/gcc/testsuite/gcc.dg/asan/use-after-scope-9.c b/gcc/testsuite/gcc.dg/asan/use-after-scope-9.c
index 2e30deffa18..5d069dd18ea 100644
--- a/gcc/testsuite/gcc.dg/asan/use-after-scope-9.c
+++ b/gcc/testsuite/gcc.dg/asan/use-after-scope-9.c
@@ -1,5 +1,6 @@
 // { dg-do run }
 // { dg-shouldfail "asan" }
+// { dg-additional-options "-O2 -fdump-tree-asan1" }
 
 int
 main (int argc, char **argv)
@@ -15,6 +16,7 @@ main (int argc, char **argv)
   return *ptr;
 }
 
+// { dg-final { scan-tree-dump-times "= ASAN_POISON \\(\\)" 1 "asan1" } }
 // { dg-output "ERROR: AddressSanitizer: stack-use-after-scope on address.*(\n|\r\n|\r)" }
 // { dg-output "READ of size .*" }
 // { dg-output ".*'a' <== Memory access at offset \[0-9\]* is inside this variable.*" }
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index 42055bd8318..d63c70f2a12 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -5449,7 +5449,7 @@ declare_inline_vars (tree block, tree vars)
    but now it will be in the TO_FN.  PARM_TO_VAR means enable PARM_DECL to
    VAR_DECL translation.  */
 
-static tree
+tree
 copy_decl_for_dup_finish (copy_body_data *id, tree decl, tree copy)
 {
   /* Don't generate debug information for the copy if we wouldn't have
diff --git a/gcc/tree-inline.h b/gcc/tree-inline.h
index ecfae6b048e..41402a315ec 100644
--- a/gcc/tree-inline.h
+++ b/gcc/tree-inline.h
@@ -218,6 +218,7 @@ extern gimple_seq copy_gimple_seq_and_replace_locals (gimple_seq seq);
 extern bool debug_find_tree (tree, tree);
 extern tree copy_fn (tree, tree&, tree&);
 extern const char *copy_forbidden (struct function *fun);
+extern tree copy_decl_for_dup_finish (copy_body_data *id, tree decl, tree copy);
 
 /* This is in tree-inline.c since the routine uses
    data structures from the inliner.  */
diff --git a/gcc/tree-ssa.c b/gcc/tree-ssa.c
index 067143f49b8..f1826b2c9c4 100644
--- a/gcc/tree-ssa.c
+++ b/gcc/tree-ssa.c
@@ -41,6 +41,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "cfgexpand.h"
 #include "tree-cfg.h"
 #include "tree-dfa.h"
+#include "asan.h"
 
 /* Pointer map of variable mappings, keyed by edge.  */
 static hash_map<edge, auto_vec<edge_var_map> > *edge_var_maps;
@@ -1575,6 +1576,30 @@ maybe_optimize_var (tree var, bitmap addresses_taken, bitmap not_reg_needs,
     }
 }
 
+/* Return true when STMT is ASAN mark where second argument is an address
+   of a local variable.  */
+
+static bool
+is_asan_mark_p (gimple *stmt)
+{
+  if (!gimple_call_internal_p (stmt, IFN_ASAN_MARK))
+    return false;
+
+  tree addr = get_base_address (gimple_call_arg (stmt, 1));
+  if (TREE_CODE (addr) == ADDR_EXPR
+      && VAR_P (TREE_OPERAND (addr, 0)))
+    {
+      tree var = TREE_OPERAND (addr, 0);
+      unsigned addressable = TREE_ADDRESSABLE (var);
+      TREE_ADDRESSABLE (var) = 0;
+      bool r = is_gimple_reg (var);
+      TREE_ADDRESSABLE (var) = addressable;
+      return r;
+    }
+
+  return false;
+}
+
 /* Compute TREE_ADDRESSABLE and DECL_GIMPLE_REG_P for local variables.  */
 
 void
@@ -1600,17 +1625,23 @@ execute_update_addresses_taken (void)
 	  enum gimple_code code = gimple_code (stmt);
 	  tree decl;
 
-	  if (code == GIMPLE_CALL
-	      && optimize_atomic_compare_exchange_p (stmt))
+	  if (code == GIMPLE_CALL)
 	    {
-	      /* For __atomic_compare_exchange_N if the second argument
-		 is &var, don't mark var addressable;
-		 if it becomes non-addressable, we'll rewrite it into
-		 ATOMIC_COMPARE_EXCHANGE call.  */
-	      tree arg = gimple_call_arg (stmt, 1);
-	      gimple_call_set_arg (stmt, 1, null_pointer_node);
-	      gimple_ior_addresses_taken (addresses_taken, stmt);
-	      gimple_call_set_arg (stmt, 1, arg);
+	      if (optimize_atomic_compare_exchange_p (stmt))
+		{
+		  /* For __atomic_compare_exchange_N if the second argument
+		     is &var, don't mark var addressable;
+		     if it becomes non-addressable, we'll rewrite it into
+		     ATOMIC_COMPARE_EXCHANGE call.  */
+		  tree arg = gimple_call_arg (stmt, 1);
+		  gimple_call_set_arg (stmt, 1, null_pointer_node);
+		  gimple_ior_addresses_taken (addresses_taken, stmt);
+		  gimple_call_set_arg (stmt, 1, arg);
+		}
+	      else if (is_asan_mark_p (stmt))
+		;
+	      else
+		gimple_ior_addresses_taken (addresses_taken, stmt);
 	    }
 	  else
 	    /* Note all addresses taken by the stmt.  */
@@ -1866,6 +1897,24 @@ execute_update_addresses_taken (void)
 			continue;
 		      }
 		  }
+		else if (is_asan_mark_p (stmt))
+		  {
+		    tree var = TREE_OPERAND (gimple_call_arg (stmt, 1), 0);
+		    if (bitmap_bit_p (suitable_for_renaming, DECL_UID (var)))
+		      {
+			unlink_stmt_vdef (stmt);
+			if (asan_mark_p (stmt, ASAN_MARK_POISON))
+			  {
+			    gcall *call
+			      = gimple_build_call_internal (IFN_ASAN_POISON, 0);
+			    gimple_call_set_lhs (call, var);
+			    gsi_replace (&gsi, call, GSI_SAME_STMT);
+			  }
+			else
+			  gsi_remove (&gsi, true);
+			continue;
+		      }
+		  }
 		for (i = 0; i < gimple_call_num_args (stmt); ++i)
 		  {
 		    tree *argp = gimple_call_arg_ptr (stmt, i);
-- 
2.11.0


[-- Attachment #3: 0002-use-after-scope-handle-writes-to-a-poisoned-variable.patch --]
[-- Type: text/x-patch, Size: 11653 bytes --]

From 82c77c43969ebd7fcb17094820111a0075311a49 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Tue, 17 Jan 2017 16:49:29 +0100
Subject: [PATCH 2/2] use-after-scope: handle writes to a poisoned variable

gcc/testsuite/ChangeLog:

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

	* gcc.dg/asan/use-after-scope-10.c: New test.
	* gcc.dg/asan/use-after-scope-11.c: New test.
	* g++.dg/asan/use-after-scope-5.C: New test.

gcc/ChangeLog:

2017-01-16  Jakub Jelinek  <jakub@redhat.com>
	    Martin Liska  <mliska@suse.cz>

	* asan.c (asan_expand_poison_ifn): Support stores and use
	appropriate ASAN report function.
	* internal-fn.c (expand_ASAN_POISON_USE): New function.
	* internal-fn.def (ASAN_POISON_USE): Declare.
	* tree-into-ssa.c (maybe_add_asan_poison_write): New function.
	(maybe_register_def): Create ASAN_POISON_USE when sanitizing.
	* tree-ssa-dce.c (eliminate_unnecessary_stmts): Remove
	ASAN_POISON calls w/o LHS.
	* tree-ssa.c (execute_update_addresses_taken): Create clobber
	for ASAN_MARK (UNPOISON, &x, ...) in order to prevent usage of a LHS
	from ASAN_MARK (POISON, &x, ...) coming to a PHI node.
	* gimplify.c (asan_poison_variables): Add attribute
	use_after_scope_memory to variables that really needs to live
	in memory.
	* tree-ssa.c (is_asan_mark_p): Do not rewrite into SSA when
	having the attribute.
---
 gcc/asan.c                                     | 19 ++++++++++------
 gcc/gimplify.c                                 | 15 +++++++++++--
 gcc/internal-fn.c                              |  8 +++++++
 gcc/internal-fn.def                            |  1 +
 gcc/testsuite/g++.dg/asan/use-after-scope-5.C  | 23 ++++++++++++++++++++
 gcc/testsuite/gcc.dg/asan/use-after-scope-10.c | 22 +++++++++++++++++++
 gcc/testsuite/gcc.dg/asan/use-after-scope-11.c | 30 ++++++++++++++++++++++++++
 gcc/tree-into-ssa.c                            | 27 ++++++++++++++++++++++-
 gcc/tree-ssa-dce.c                             | 16 ++++++++++----
 gcc/tree-ssa.c                                 | 15 ++++++++++++-
 10 files changed, 161 insertions(+), 15 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/asan/use-after-scope-5.C
 create mode 100644 gcc/testsuite/gcc.dg/asan/use-after-scope-10.c
 create mode 100644 gcc/testsuite/gcc.dg/asan/use-after-scope-11.c

diff --git a/gcc/asan.c b/gcc/asan.c
index 4aa5a4015ea..040e949fc85 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -3117,6 +3117,8 @@ create_asan_shadow_var (tree var_decl,
     return *slot;
 }
 
+/* Expand ASAN_POISON ifn.  */
+
 bool
 asan_expand_poison_ifn (gimple_stmt_iterator *iter,
 			bool *need_commit_edge_insert,
@@ -3130,8 +3132,8 @@ asan_expand_poison_ifn (gimple_stmt_iterator *iter,
       return true;
     }
 
-  tree shadow_var  = create_asan_shadow_var (SSA_NAME_VAR (poisoned_var),
-					     shadow_vars_mapping);
+  tree shadow_var = create_asan_shadow_var (SSA_NAME_VAR (poisoned_var),
+					    shadow_vars_mapping);
 
   bool recover_p;
   if (flag_sanitize & SANITIZE_USER_ADDRESS)
@@ -3145,16 +3147,16 @@ asan_expand_poison_ifn (gimple_stmt_iterator *iter,
 						 ASAN_MARK_POISON),
 				  build_fold_addr_expr (shadow_var), size);
 
-  use_operand_p use_p;
+  gimple *use;
   imm_use_iterator imm_iter;
-  FOR_EACH_IMM_USE_FAST (use_p, imm_iter, poisoned_var)
+  FOR_EACH_IMM_USE_STMT (use, imm_iter, poisoned_var)
     {
-      gimple *use = USE_STMT (use_p);
       if (is_gimple_debug (use))
 	continue;
 
       int nargs;
-      tree fun = report_error_func (false, recover_p, tree_to_uhwi (size),
+      bool store_p = gimple_call_internal_p (use, IFN_ASAN_POISON_USE);
+      tree fun = report_error_func (store_p, recover_p, tree_to_uhwi (size),
 				    &nargs);
 
       gcall *call = gimple_build_call (fun, 1,
@@ -3183,7 +3185,10 @@ asan_expand_poison_ifn (gimple_stmt_iterator *iter,
       else
 	{
 	  gimple_stmt_iterator gsi = gsi_for_stmt (use);
-	  gsi_insert_before (&gsi, call, GSI_NEW_STMT);
+	  if (store_p)
+	    gsi_replace (&gsi, call, true);
+	  else
+	    gsi_insert_before (&gsi, call, GSI_NEW_STMT);
 	}
     }
 
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index d382eea69f8..c6f7d7c4505 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -1206,8 +1206,19 @@ asan_poison_variables (hash_set<tree> *variables, bool poison, gimple_seq *seq_p
 
   sorted_variables.qsort (sort_by_decl_uid);
 
-  for (unsigned i = 0; i < sorted_variables.length (); i++)
-    asan_poison_variable (sorted_variables[i], poison, seq_p);
+  unsigned i;
+  tree var;
+  FOR_EACH_VEC_ELT (sorted_variables, i, var)
+    {
+      asan_poison_variable (var, poison, seq_p);
+
+      /* Add use_after_scope_memory attribute for the variable in order
+	 to prevent re-written into SSA.  */
+      DECL_ATTRIBUTES (var)
+	= tree_cons (get_identifier ("use_after_scope_memory"),
+		     build_int_cst (integer_type_node, 1),
+		     DECL_ATTRIBUTES (var));
+    }
 }
 
 /* Gimplify a BIND_EXPR.  Just voidify and recurse.  */
diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
index 45e4ce05b86..0d61375462d 100644
--- a/gcc/internal-fn.c
+++ b/gcc/internal-fn.c
@@ -388,6 +388,14 @@ expand_ASAN_POISON (internal_fn, gcall *)
   gcc_unreachable ();
 }
 
+/* This should get expanded in the sanopt pass.  */
+
+static void
+expand_ASAN_POISON_USE (internal_fn, gcall *)
+{
+  gcc_unreachable ();
+}
+
 /* This should get expanded in the tsan pass.  */
 
 static void
diff --git a/gcc/internal-fn.def b/gcc/internal-fn.def
index 7b28b6722ff..fd25a952299 100644
--- a/gcc/internal-fn.def
+++ b/gcc/internal-fn.def
@@ -168,6 +168,7 @@ DEF_INTERNAL_FN (BUILTIN_EXPECT, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
 DEF_INTERNAL_FN (ASAN_CHECK, ECF_TM_PURE | ECF_LEAF | ECF_NOTHROW, ".R...")
 DEF_INTERNAL_FN (ASAN_MARK, ECF_LEAF | ECF_NOTHROW, ".R..")
 DEF_INTERNAL_FN (ASAN_POISON, ECF_LEAF | ECF_NOTHROW | ECF_NOVOPS, NULL)
+DEF_INTERNAL_FN (ASAN_POISON_USE, ECF_LEAF | ECF_NOTHROW | ECF_NOVOPS, NULL)
 DEF_INTERNAL_FN (ADD_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
 DEF_INTERNAL_FN (SUB_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
 DEF_INTERNAL_FN (MUL_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
diff --git a/gcc/testsuite/g++.dg/asan/use-after-scope-5.C b/gcc/testsuite/g++.dg/asan/use-after-scope-5.C
new file mode 100644
index 00000000000..7e28fc35e6d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/asan/use-after-scope-5.C
@@ -0,0 +1,23 @@
+// { dg-do run }
+
+int *
+__attribute__((optimize(("-O0"))))
+fn1 (int *a)
+{
+  return a;
+}
+
+void
+fn2 ()
+{
+  for (int i = 0; i < 10; i++)
+    {
+      int *a;
+      (a) = fn1 (a);
+    }
+}
+
+int main()
+{
+  fn2();
+}
diff --git a/gcc/testsuite/gcc.dg/asan/use-after-scope-10.c b/gcc/testsuite/gcc.dg/asan/use-after-scope-10.c
new file mode 100644
index 00000000000..24de8cec1ff
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asan/use-after-scope-10.c
@@ -0,0 +1,22 @@
+// { dg-do run }
+// { dg-shouldfail "asan" }
+// { dg-additional-options "-O2 -fdump-tree-asan1" }
+
+int
+main (int argc, char **argv)
+{
+  int *ptr = 0;
+
+  {
+    int a;
+    ptr = &a;
+    *ptr = 12345;
+  }
+
+  *ptr = 12345;
+  return *ptr;
+}
+
+// { dg-output "ERROR: AddressSanitizer: stack-use-after-scope on address.*(\n|\r\n|\r)" }
+// { dg-output "WRITE of size .*" }
+// { dg-output ".*'a' <== Memory access at offset \[0-9\]* is inside this variable.*" }
diff --git a/gcc/testsuite/gcc.dg/asan/use-after-scope-11.c b/gcc/testsuite/gcc.dg/asan/use-after-scope-11.c
new file mode 100644
index 00000000000..b3c4c9ec758
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asan/use-after-scope-11.c
@@ -0,0 +1,30 @@
+// { dg-do run }
+
+#include <string.h>
+
+char cc;
+char ptr[] = "sparta2";
+
+void get(char **x)
+{
+  *x = ptr;
+}
+  
+int main()
+{
+  char *here = &cc;
+
+  for (;;)
+    {
+    next_line:
+	if (here == NULL)
+	  __builtin_abort();
+	get (&here);
+	if (strcmp (here, "sparta") == 0)
+	    goto next_line;
+	else if (strcmp (here, "sparta2") == 0)
+	  break;
+    }
+
+  return 0;
+}
diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c
index c7df237d57f..22261c15dc2 100644
--- a/gcc/tree-into-ssa.c
+++ b/gcc/tree-into-ssa.c
@@ -38,6 +38,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "tree-ssa.h"
 #include "domwalk.h"
 #include "statistics.h"
+#include "asan.h"
 
 #define PERCENT(x,y) ((float)(x) * 100.0 / (float)(y))
 
@@ -1807,6 +1808,26 @@ maybe_replace_use_in_debug_stmt (use_operand_p use_p)
 }
 
 
+/* If DEF has x_5 = ASAN_POISON () as its current def, add
+   ASAN_POISON_USE (x_5) stmt before GSI to denote the stmt writes into
+   a poisoned (out of scope) variable.  */
+
+static void
+maybe_add_asan_poison_write (tree def, gimple_stmt_iterator *gsi)
+{
+  tree cdef = get_current_def (def);
+  if (cdef != NULL
+      && TREE_CODE (cdef) == SSA_NAME
+      && gimple_call_internal_p (SSA_NAME_DEF_STMT (cdef), IFN_ASAN_POISON))
+    {
+      gcall *call
+	= gimple_build_call_internal (IFN_ASAN_POISON_USE, 1, cdef);
+      gimple_set_location (call, gimple_location (gsi_stmt (*gsi)));
+      gsi_insert_before (gsi, call, GSI_SAME_STMT);
+    }
+}
+
+
 /* If the operand pointed to by DEF_P is an SSA name in NEW_SSA_NAMES
    or OLD_SSA_NAMES, or if it is a symbol marked for renaming,
    register it as the current definition for the names replaced by
@@ -1837,7 +1858,11 @@ maybe_register_def (def_operand_p def_p, gimple *stmt,
 	      def = get_or_create_ssa_default_def (cfun, sym);
 	    }
 	  else
-	    def = make_ssa_name (def, stmt);
+	    {
+	      if (asan_sanitize_use_after_scope ())
+		maybe_add_asan_poison_write (def, &gsi);
+	      def = make_ssa_name (def, stmt);
+	    }
 	  SET_DEF (def_p, def);
 
 	  tree tracked_var = target_for_debug_bind (sym);
diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c
index 4e51e699d49..5ebe57b0983 100644
--- a/gcc/tree-ssa-dce.c
+++ b/gcc/tree-ssa-dce.c
@@ -1367,10 +1367,18 @@ eliminate_unnecessary_stmts (void)
 		  update_stmt (stmt);
 		  release_ssa_name (name);
 
-		  /* GOMP_SIMD_LANE without lhs is not needed.  */
-		  if (gimple_call_internal_p (stmt)
-		      && gimple_call_internal_fn (stmt) == IFN_GOMP_SIMD_LANE)
-		    remove_dead_stmt (&gsi, bb);
+		  /* GOMP_SIMD_LANE or ASAN_POISON without lhs is not
+		     needed.  */
+		  if (gimple_call_internal_p (stmt))
+		    switch (gimple_call_internal_fn (stmt))
+		      {
+		      case IFN_GOMP_SIMD_LANE:
+		      case IFN_ASAN_POISON:
+			remove_dead_stmt (&gsi, bb);
+			break;
+		      default:
+			break;
+		      }
 		}
 	      else if (gimple_call_internal_p (stmt))
 		switch (gimple_call_internal_fn (stmt))
diff --git a/gcc/tree-ssa.c b/gcc/tree-ssa.c
index f1826b2c9c4..10b77895508 100644
--- a/gcc/tree-ssa.c
+++ b/gcc/tree-ssa.c
@@ -1590,6 +1590,10 @@ is_asan_mark_p (gimple *stmt)
       && VAR_P (TREE_OPERAND (addr, 0)))
     {
       tree var = TREE_OPERAND (addr, 0);
+      if (lookup_attribute ("use_after_scope_memory",
+			    DECL_ATTRIBUTES (var)))
+	return false;
+
       unsigned addressable = TREE_ADDRESSABLE (var);
       TREE_ADDRESSABLE (var) = 0;
       bool r = is_gimple_reg (var);
@@ -1911,7 +1915,16 @@ execute_update_addresses_taken (void)
 			    gsi_replace (&gsi, call, GSI_SAME_STMT);
 			  }
 			else
-			  gsi_remove (&gsi, true);
+			  {
+			    /* In ASAN_MARK (UNPOISON, &b, ...) the variable
+			       is uninitialized.  Avoid dependencies on
+			       previous out of scope value.  */
+			    tree clobber
+			      = build_constructor (TREE_TYPE (var), NULL);
+			    TREE_THIS_VOLATILE (clobber) = 1;
+			    gimple *g = gimple_build_assign (var, clobber);
+			    gsi_replace (&gsi, g, GSI_SAME_STMT);
+			  }
 			continue;
 		      }
 		  }
-- 
2.11.0


[-- Attachment #4: use-after-scope-diff.patch --]
[-- Type: text/x-patch, Size: 1384 bytes --]

diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 2777a23eb93..1b076fdf45c 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -1206,8 +1206,19 @@ asan_poison_variables (hash_set<tree> *variables, bool poison, gimple_seq *seq_p
 
   sorted_variables.qsort (sort_by_decl_uid);
 
-  for (unsigned i = 0; i < sorted_variables.length (); i++)
-    asan_poison_variable (sorted_variables[i], poison, seq_p);
+  unsigned i;
+  tree var;
+  FOR_EACH_VEC_ELT (sorted_variables, i, var)
+    {
+      asan_poison_variable (var, poison, seq_p);
+
+      /* Add use_after_scope_memory attribute for the variable in order
+	 to prevent re-written into SSA.  */
+      DECL_ATTRIBUTES (var)
+	= tree_cons (get_identifier ("use_after_scope_memory"),
+		     build_int_cst (integer_type_node, 1),
+		     DECL_ATTRIBUTES (var));
+    }
 }
 
 /* Gimplify a BIND_EXPR.  Just voidify and recurse.  */
diff --git a/gcc/tree-ssa.c b/gcc/tree-ssa.c
index 5bd9004e715..2b33da93a23 100644
--- a/gcc/tree-ssa.c
+++ b/gcc/tree-ssa.c
@@ -1565,6 +1565,10 @@ is_asan_mark_p (gimple *stmt)
       && VAR_P (TREE_OPERAND (addr, 0)))
     {
       tree var = TREE_OPERAND (addr, 0);
+      if (lookup_attribute ("use_after_scope_memory",
+			    DECL_ATTRIBUTES (var)))
+	return false;
+
       unsigned addressable = TREE_ADDRESSABLE (var);
       TREE_ADDRESSABLE (var) = 0;
       bool r = is_gimple_reg (var);

  parent reply	other threads:[~2017-01-23  9:20 UTC|newest]

Thread overview: 111+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-06 11:04 [PATCH, RFC] Introduce -fsanitize=use-after-scope Martin Liška
2016-05-06 11:08 ` [PATCH] Introduce tests for -fsanitize=use-after-scope Martin Liška
2016-05-11 12:56   ` Martin Liška
2016-05-06 11:16 ` [PATCH, RFC] Introduce -fsanitize=use-after-scope Martin Liška
2016-05-06 11:48 ` Yury Gribov
2016-05-06 12:39   ` Jakub Jelinek
2016-05-06 13:07     ` Martin Liška
2016-05-06 14:22     ` Yury Gribov
2016-05-06 14:39       ` Jakub Jelinek
2016-05-10 15:03         ` Martin Liška
2016-05-10 15:15           ` Jakub Jelinek
2016-05-06 13:17   ` Martin Liška
2016-05-06 13:25     ` Jakub Jelinek
2016-05-06 14:41       ` Martin Liška
2016-05-06 14:46         ` Jakub Jelinek
2016-05-06 12:22 ` Jakub Jelinek
2016-05-11 12:54   ` Martin Liška
2016-05-12 10:42     ` Jakub Jelinek
2016-05-12 14:12       ` Martin Liška
2016-08-12 12:42         ` Martin Liška
2016-08-18 13:36         ` Jakub Jelinek
2016-10-03  9:27           ` [PATCH, RFC] Introduce -fsanitize=use-after-scope (v2) Martin Liška
2016-10-03  9:30             ` [PATCH, 02/N] Introduce tests for -fsanitize-address-use-after-scope Martin Liška
2016-11-07 10:04               ` [PATCH, 02/N] Introduce tests for -fsanitize-address-use-after-scope (v3) Martin Liška
2016-11-07 10:09                 ` Jakub Jelinek
2016-10-03  9:39             ` [PATCH, RFC] Introduce -fsanitize=use-after-scope (v2) Jakub Jelinek
2016-10-07 11:13             ` Jakub Jelinek
2016-10-12 14:08               ` Martin Liška
2016-10-21 14:26                 ` Jakub Jelinek
2016-10-25 13:18                   ` Martin Liška
2016-10-27 14:40                   ` Martin Liška
2016-10-27 17:24                     ` Jakub Jelinek
2016-11-01 14:48                       ` Martin Liška
2016-11-01 14:54                         ` Jakub Jelinek
2016-11-01 15:01                           ` Martin Liška
2016-11-02  9:36                           ` Martin Liška
2016-11-02  9:59                             ` Jakub Jelinek
2016-11-02 10:09                               ` Martin Liška
2016-11-02 10:11                               ` Jakub Jelinek
2016-11-02 14:20                                 ` Marek Polacek
2016-11-02 14:27                                   ` Martin Liška
2016-11-02 14:35                                     ` Jakub Jelinek
2016-11-04  9:17                                       ` Martin Liška
2016-11-04  9:33                                         ` Jakub Jelinek
2016-11-04 10:59                                           ` Martin Liška
2016-11-07 10:03                                             ` [PATCH, RFC] Introduce -fsanitize=use-after-scope (v3) Martin Liška
2016-11-07 10:08                                               ` Jakub Jelinek
2016-11-08  8:58                                                 ` Question about lambda function variables Martin Liška
2016-11-08  9:12                                                   ` Jakub Jelinek
2016-11-08  9:35                                                     ` Martin Liška
2016-11-07 16:07                                               ` Fix build of jit (was Re: [PATCH, RFC] Introduce -fsanitize=use-after-scope (v3)) David Malcolm
2016-11-07 16:17                                                 ` Jakub Jelinek
2016-11-08  9:38                                                   ` Martin Liška
2016-11-08  9:41                                                     ` Jakub Jelinek
2016-11-08 12:00                                                       ` [PATCH] use-after-scope fallout Martin Liška
2016-11-08 12:10                                                         ` Jakub Jelinek
2016-11-08 18:05                                                         ` David Malcolm
2016-11-01 14:54                       ` [PATCH, RFC] Introduce -fsanitize=use-after-scope (v2) Martin Liška
2016-11-01 15:12                         ` Jakub Jelinek
2016-11-02  9:40                           ` Richard Biener
2016-11-02  9:44                             ` Martin Liška
2016-11-02  9:52                             ` Jakub Jelinek
2016-11-02 12:36                               ` Richard Biener
2016-11-02 12:56                                 ` Jakub Jelinek
2016-11-02 12:59                                   ` Richard Biener
2016-11-02 13:06                                     ` Jakub Jelinek
2016-11-02 13:16                                       ` Richard Biener
2016-11-02 14:38                                         ` Martin Liška
2016-11-02 14:51                                           ` Jakub Jelinek
2016-11-02 15:25                                             ` Martin Liška
2016-11-03 13:34                                             ` Martin Liška
2016-11-03 13:44                                               ` Jakub Jelinek
2016-11-03 14:02                                                 ` Martin Liška
2016-11-03 14:04                                                   ` Jakub Jelinek
2016-11-03 14:18                                                     ` Martin Liška
2016-11-16 12:25                                         ` [RFC][PATCH] Speed-up use-after-scope (re-writing to SSA) Martin Liška
2016-11-16 12:53                                           ` Martin Liška
2016-11-16 13:07                                           ` Jakub Jelinek
2016-11-16 16:01                                             ` Martin Liška
2016-11-16 16:28                                               ` Jakub Jelinek
2016-11-22 11:55                                                 ` Martin Liška
2016-11-23 13:57                                                   ` Martin Liška
2016-11-23 14:14                                                     ` Jakub Jelinek
2016-12-01 16:30                                                       ` Martin Liška
2016-12-02 12:29                                                         ` Richard Biener
2016-12-08 12:51                                                           ` Martin Liška
2016-12-13 14:16                                                             ` Richard Biener
2016-12-20 11:34                                                 ` [PATCH] Speed-up use-after-scope (re-writing to SSA) (version 2) Martin Liška
2016-12-21  9:19                                                   ` Jakub Jelinek
2016-12-22 17:11                                                     ` Martin Liška
2016-12-22 17:28                                                       ` Jakub Jelinek
2017-01-09 14:58                                                         ` Martin Liška
2017-01-16 14:20                                                           ` Jakub Jelinek
2017-01-17 16:22                                                             ` Martin Liška
2017-01-17 16:55                                                               ` Jakub Jelinek
2017-01-18 15:37                                                                 ` Martin Liška
2017-01-19 16:43                                                                   ` Jakub Jelinek
2017-01-20 11:55                                                                     ` Martin Liška
2017-01-20 14:27                                                                       ` Martin Liška
2017-01-20 14:30                                                                         ` Jakub Jelinek
2017-01-20 14:42                                                                           ` Markus Trippelsdorf
2017-01-23  9:38                                                                           ` Martin Liška [this message]
2017-01-23  9:39                                                                             ` Jakub Jelinek
2017-01-23 12:07                                                                               ` Martin Liška
2017-01-26  9:04                                                                             ` Thomas Schwinge
2017-01-26 10:55                                                                               ` Jakub Jelinek
2017-01-26 20:45                                                                                 ` Thomas Schwinge
2017-01-26 20:52                                                                                   ` Jakub Jelinek
2016-11-16 16:09                                             ` [RFC][PATCH] Speed-up use-after-scope (re-writing to SSA) Martin Liška
2016-11-02  9:52                           ` [PATCH, RFC] Introduce -fsanitize=use-after-scope (v2) Martin Liška
2016-09-03 15:23         ` [PATCH, RFC] Introduce -fsanitize=use-after-scope Jakub Jelinek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4e0e7dbe-e770-bc77-b25f-6f38f5125e89@suse.cz \
    --to=mliska@suse.cz \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=richard.guenther@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).