This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, RFC] Introduce -fsanitize=use-after-scope (v2)


On 10/27/2016 07:23 PM, Jakub Jelinek wrote:
> On Thu, Oct 27, 2016 at 04:40:30PM +0200, Martin Liška wrote:
>> On 10/21/2016 04:26 PM, Jakub Jelinek wrote:
>>> On Wed, Oct 12, 2016 at 04:07:53PM +0200, Martin Liška wrote:
>>>>> Ok, first let me list some needed follow-ups that don't need to be handled
>>>>> right away:
>>>>> - r237814-like changes for ASAN_MARK
>>
>> I've spent quite some on that and that's what I begin (use-after-scope-addressable.patch).
>> Problem is that as I ignore all ASAN_MARK internal fns, the code does not detect having address
>> taken in:
>>
>> _2 = MEM[(char *)&my_char + 8B];
>>
>>   char *ptr;
>>   {
>>     char my_char[9];
>>     ptr = &my_char[0];
>>   }
>>
>>   return *(ptr+8);
>>
>> and thus the code in tree-ssa.c (maybe_optimize_var) sets TREE_ADDRESSABLE (var) = 0.
> 
> Perhaps we should do that only if the var's type is_gimple_reg_type,
> then we'd rewrite that into SSA at that time, right?  So, in theory if we
> turned the ASAN_MARK poisoning call into another internal function
> (var_5 = ASAN_POISON ()) and then after converting it into SSA looked at
> all the uses of such an lhs and perhaps at sanopt part or when marked all
> the use places with a library call that would complain at runtime?
> Or turn those back at sanopt time into addressable memory loads which would
> be poisoned or similar?  Or alternatively, immediately before turning
> variables addressable just because of ASAN_MARK into non-addressable use
> the same framework into-ssa uses to find out if there are any poisoned
> accesses, and just not optimize it in that case.
> Anyway, this can be done incrementally.

I've done a patch candidate (not tested yet) which is capable of ASAN_MARK removal
for local variables that can be rewritten into SSA. This is done by running execute_update_addresses_taken
after we create ASAN_CHECK internal fns and skipping all ASAN_MARK for having address taken considerations.
This removes significant number of ASAN_MARK fns in tramp3d (due to C++ temporaries).

> 
>> Second question I have is whether we want to handle just TREE_ADDRESSABLE stuff during gimplification?
>> Basically in a way that the current patch is doing?
> 
> How could variables that aren't TREE_ADDRESSABLE during gimplification be
> accessed out of scope?

Yep, TREE_ADDRESSABLE guard does what it should do.

I'm going to test the patch which can be installed incrementally.

Martin

> 
>> +/* Return true if DECL should be guarded on the stack.  */
>> +
>> +static inline bool
>> +asan_protect_stack_decl (tree decl)
>> +{
>> +  return DECL_P (decl)
>> +    && (!DECL_ARTIFICIAL (decl)
>> +	|| (asan_sanitize_use_after_scope () && TREE_ADDRESSABLE (decl)));
> 
> Bad formatting.  Should be:
> 
>   return (DECL_P (decl)
> 	  && (!DECL_ARTIFICIAL (decl)
> 	      || (asan_sanitize_use_after_scope ()
> 		  && TREE_ADDRESSABLE (decl))));
> 
> Ok for trunk with that change.
> 
> 	Jakub
> 

>From cf860324da41244745f04a16b184fabe343ac5d9 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Tue, 1 Nov 2016 11:21:20 +0100
Subject: [PATCH 4/4] Use-after-scope: do not mark variables that are no longer
 addressable

gcc/ChangeLog:

2016-11-01  Martin Liska  <mliska@suse.cz>

	* asan.c (asan_instrument): Call
	execute_update_addresses_taken_asan_sanitize right after
	sanitization.
	* tree-ssa.c (maybe_optimize_var): Mark all variables set to
	non-addressable.
	(is_asan_mark_p): New function.
	(execute_update_addresses_taken): Likewise.
	(execute_update_addresses_taken_asan_sanitize): Likewise.
	* tree-ssa.h (execute_update_addresses_taken_asan_sanitize):
	Declare new function.
---
 gcc/asan.c     |  4 +++
 gcc/tree-ssa.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++-----------
 gcc/tree-ssa.h |  1 +
 3 files changed, 84 insertions(+), 17 deletions(-)

diff --git a/gcc/asan.c b/gcc/asan.c
index 95495d2..5cb37c8 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -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-ssa.h"
 
 /* AddressSanitizer finds out-of-bounds and use-after-free bugs
    with <2x slowdown on average.
@@ -2973,6 +2974,9 @@ asan_instrument (void)
   if (shadow_ptr_types[0] == NULL_TREE)
     asan_init_shadow_ptr_types ();
   transform_statements ();
+
+  if (optimize)
+    execute_update_addresses_taken_asan_sanitize ();
   return 0;
 }
 
diff --git a/gcc/tree-ssa.c b/gcc/tree-ssa.c
index 135952b..0633b21 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;
@@ -1504,7 +1505,7 @@ non_rewritable_lvalue_p (tree lhs)
 
 static void
 maybe_optimize_var (tree var, bitmap addresses_taken, bitmap not_reg_needs,
-		    bitmap suitable_for_renaming)
+		    bitmap suitable_for_renaming, bitmap marked_nonaddressable)
 {
   /* Global Variables, result decls cannot be changed.  */
   if (is_global_var (var)
@@ -1522,6 +1523,7 @@ maybe_optimize_var (tree var, bitmap addresses_taken, bitmap not_reg_needs,
 	  || !bitmap_bit_p (not_reg_needs, DECL_UID (var))))
     {
       TREE_ADDRESSABLE (var) = 0;
+      bitmap_set_bit (marked_nonaddressable, DECL_UID (var));
       if (is_gimple_reg (var))
 	bitmap_set_bit (suitable_for_renaming, DECL_UID (var));
       if (dump_file)
@@ -1550,20 +1552,43 @@ maybe_optimize_var (tree var, bitmap addresses_taken, bitmap not_reg_needs,
     }
 }
 
-/* Compute TREE_ADDRESSABLE and DECL_GIMPLE_REG_P for local variables.  */
+/* Return true when STMT is ASAN mark where second argument is an address
+   of a local variable.  */
 
-void
-execute_update_addresses_taken (void)
+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
+      && TREE_CODE (TREE_OPERAND (addr, 0)) == VAR_DECL)
+    return true;
+
+  return false;
+}
+
+/* Compute TREE_ADDRESSABLE and DECL_GIMPLE_REG_P for local variables.
+   If SANITIZE_ASAN_MARK is set to true, sanitize also ASAN_MARK built-ins.  */
+
+
+static void
+execute_update_addresses_taken (bool sanitize_asan_mark = false)
 {
   basic_block bb;
   bitmap addresses_taken = BITMAP_ALLOC (NULL);
   bitmap not_reg_needs = BITMAP_ALLOC (NULL);
   bitmap suitable_for_renaming = BITMAP_ALLOC (NULL);
+  bitmap marked_nonaddressable = BITMAP_ALLOC (NULL);
   tree var;
   unsigned i;
 
   timevar_push (TV_ADDRESS_TAKEN);
 
+  if (dump_file)
+    fprintf (dump_file, "call execute_update_addresses_taken\n");
+
   /* Collect into ADDRESSES_TAKEN all variables whose address is taken within
      the function body.  */
   FOR_EACH_BB_FN (bb, cfun)
@@ -1575,17 +1600,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 (sanitize_asan_mark && is_asan_mark_p (stmt))
+		;
+	      else
+		gimple_ior_addresses_taken (addresses_taken, stmt);
 	    }
 	  else
 	    /* Note all addresses taken by the stmt.  */
@@ -1675,15 +1706,17 @@ execute_update_addresses_taken (void)
      for -g vs. -g0.  */
   for (var = DECL_ARGUMENTS (cfun->decl); var; var = DECL_CHAIN (var))
     maybe_optimize_var (var, addresses_taken, not_reg_needs,
-			suitable_for_renaming);
+			suitable_for_renaming, marked_nonaddressable);
 
   FOR_EACH_VEC_SAFE_ELT (cfun->local_decls, i, var)
     maybe_optimize_var (var, addresses_taken, not_reg_needs,
-			suitable_for_renaming);
+			suitable_for_renaming, marked_nonaddressable);
 
   /* Operand caches need to be recomputed for operands referencing the updated
      variables and operands need to be rewritten to expose bare symbols.  */
-  if (!bitmap_empty_p (suitable_for_renaming))
+  if (!bitmap_empty_p (suitable_for_renaming)
+      || (asan_sanitize_use_after_scope ()
+	  && !bitmap_empty_p (marked_nonaddressable)))
     {
       FOR_EACH_BB_FN (bb, cfun)
 	for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi);)
@@ -1841,6 +1874,17 @@ execute_update_addresses_taken (void)
 			continue;
 		      }
 		  }
+		else if (sanitize_asan_mark && 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))
+			|| bitmap_bit_p (marked_nonaddressable, DECL_UID (var)))
+		      {
+			unlink_stmt_vdef (stmt);
+			gsi_remove (&gsi, true);
+			continue;
+		      }
+		  }
 		for (i = 0; i < gimple_call_num_args (stmt); ++i)
 		  {
 		    tree *argp = gimple_call_arg_ptr (stmt, i);
@@ -1896,9 +1940,27 @@ execute_update_addresses_taken (void)
   BITMAP_FREE (not_reg_needs);
   BITMAP_FREE (addresses_taken);
   BITMAP_FREE (suitable_for_renaming);
+  BITMAP_FREE (marked_nonaddressable);
   timevar_pop (TV_ADDRESS_TAKEN);
 }
 
+/* Compute TREE_ADDRESSABLE and DECL_GIMPLE_REG_P for local variables.  */
+
+void
+execute_update_addresses_taken (void)
+{
+  execute_update_addresses_taken (false);
+}
+
+/* Compute TREE_ADDRESSABLE and DECL_GIMPLE_REG_P for local variables
+   and sanitize ASAN_MARK built-ins.  */
+
+void
+execute_update_addresses_taken_asan_sanitize (void)
+{
+  execute_update_addresses_taken (true);
+}
+
 namespace {
 
 const pass_data pass_data_update_address_taken =
diff --git a/gcc/tree-ssa.h b/gcc/tree-ssa.h
index 37ad7ae..912d144 100644
--- a/gcc/tree-ssa.h
+++ b/gcc/tree-ssa.h
@@ -53,6 +53,7 @@ extern tree tree_ssa_strip_useless_type_conversions (tree);
 extern bool ssa_undefined_value_p (tree, bool = true);
 extern bool gimple_uses_undefined_value_p (gimple *);
 extern void execute_update_addresses_taken (void);
+extern void execute_update_addresses_taken_asan_sanitize (void);
 
 /* Given an edge_var_map V, return the PHI arg definition.  */
 
-- 
2.10.1


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]