diff mbox

Remove dead code in asan.c

Message ID 72698527-7892-fa11-c34e-cbe1e325c124@suse.cz
State New
Headers show

Commit Message

Martin Liška June 30, 2017, 1:34 p.m. UTC 
On 06/30/2017 12:15 PM, Jakub Jelinek wrote:
> On Fri, Jun 30, 2017 at 12:00:36PM +0200, Martin Liška wrote:
>> Hi.
>>
>> Following crap code was added by me when I added use-after-scope.
>> Actually decl always points to LASANPC, so asan_handled_variables->contains (decl)
>> is always false.
>>
>> Well, originally the idea was to not clear content (place in shadow memory in between
>> red zoner) of auto variables, but as we emit 0xf5 in order to have working use-after-return,
>> it probably does not worth for doing an optimization?
> 
> use-after-return is only runtime conditional, defaults to off.
> And your patch doesn't bring the code to anything close to what we had
> before the -fsanitize-use-after-scope changes, just look what it did before
> - only cleared the shadow spots that weren't known to be 0, clearing the
> whole shadow might be too expensive.  Consider many KB large local
> variables.
> 
> You can find the right decl in decls[l / 2] or decls[l / 2 - 1] or so.
> 
> 	Jakub
> 

Huh, sorry, I overlooked that shadow stack used by use-after-return has it's own
code that does shadow memory modification.

Please take a look at v2 of the patch where I reverted the hunk to pre use-after-scope
and then I added condition to unpoison variables used in use-after-scope.

I'm going to test it.

Martin

Comments

Jakub Jelinek June 30, 2017, 2:26 p.m. UTC | #1 
> +      /* Unpoison shadow memory that corresponds to a variable that was
> +	 is subject of use-after-return sanitization.  */

was is ?
And shouldn't it be subject to instead of subject of?

Otherwise LGTM.

	Jakub
diff mbox

Patch

From 2e249424e28a0d66ffb1c9cf8f54c1160043a2cc Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Fri, 30 Jun 2017 15:08:55 +0200
Subject: [PATCH] Make stack epilogue more efficient

gcc/ChangeLog:

2017-06-30  Martin Liska  <mliska@suse.cz>

	* asan.c (asan_emit_stack_protection): Unpoison just red zones
	and shadow memory of auto variables which are subject of
	use-after-scope sanitization.
	(asan_expand_mark_ifn): Add do set only when is_poison.
---
 gcc/asan.c | 79 +++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 40 insertions(+), 39 deletions(-)

diff --git a/gcc/asan.c b/gcc/asan.c
index 2de16402c51..5cce2be9962 100644
--- a/gcc/asan.c
+++ b/gcc/asan.c
@@ -1062,7 +1062,7 @@  asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb,
   HOST_WIDE_INT base_offset = offsets[length - 1];
   HOST_WIDE_INT base_align_bias = 0, offset, prev_offset;
   HOST_WIDE_INT asan_frame_size = offsets[0] - base_offset;
-  HOST_WIDE_INT last_offset;
+  HOST_WIDE_INT last_offset, last_size;
   int l;
   unsigned char cur_shadow_byte = ASAN_STACK_MAGIC_LEFT;
   tree str_cst, decl, id;
@@ -1297,58 +1297,55 @@  asan_emit_stack_protection (rtx base, rtx pbase, unsigned int alignb,
   if (STRICT_ALIGNMENT)
     set_mem_align (shadow_mem, (GET_MODE_ALIGNMENT (SImode)));
 
-  /* Unpoison shadow memory of a stack at the very end of a function.
-     As we're poisoning stack variables at the end of their scope,
-     shadow memory must be properly unpoisoned here.  The easiest approach
-     would be to collect all variables that should not be unpoisoned and
-     we unpoison shadow memory of the whole stack except ranges
-     occupied by these variables.  */
+  prev_offset = base_offset;
   last_offset = base_offset;
-  HOST_WIDE_INT current_offset = last_offset;
-  if (length)
+  last_size = 0;
+  for (l = length; l; l -= 2)
     {
-      HOST_WIDE_INT var_end_offset = 0;
-      HOST_WIDE_INT stack_start = offsets[length - 1];
-      gcc_assert (last_offset == stack_start);
-
-      for (int l = length - 2; l > 0; l -= 2)
+      offset = base_offset + ((offsets[l - 1] - base_offset)
+			     & ~(ASAN_RED_ZONE_SIZE - HOST_WIDE_INT_1));
+      if (last_offset + last_size != offset)
 	{
-	  HOST_WIDE_INT var_offset = offsets[l];
-	  current_offset = var_offset;
-	  var_end_offset = offsets[l - 1];
-	  HOST_WIDE_INT rounded_size = ROUND_UP (var_end_offset - var_offset,
-					     BITS_PER_UNIT);
+	  shadow_mem = adjust_address (shadow_mem, VOIDmode,
+				       (last_offset - prev_offset)
+				       >> ASAN_SHADOW_SHIFT);
+	  prev_offset = last_offset;
+	  asan_clear_shadow (shadow_mem, last_size >> ASAN_SHADOW_SHIFT);
+	  last_offset = offset;
+	  last_size = 0;
+	}
+      last_size += base_offset + ((offsets[l - 2] - base_offset)
+				  & ~(ASAN_RED_ZONE_SIZE - HOST_WIDE_INT_1))
+		   - offset;
 
-	  /* Should we unpoison the variable?  */
+      /* Unpoison shadow memory that corresponds to a variable that was
+	 is subject of use-after-return sanitization.  */
+      if (l > 2)
+	{
+	  decl = decls[l / 2 - 2];
 	  if (asan_handled_variables != NULL
 	      && asan_handled_variables->contains (decl))
 	    {
+	      HOST_WIDE_INT size = offsets[l - 3] - offsets[l - 2];
 	      if (dump_file && (dump_flags & TDF_DETAILS))
 		{
 		  const char *n = (DECL_NAME (decl)
 				   ? IDENTIFIER_POINTER (DECL_NAME (decl))
 				   : "<unknown>");
 		  fprintf (dump_file, "Unpoisoning shadow stack for variable: "
-			   "%s (%" PRId64 "B)\n", n,
-			   var_end_offset - var_offset);
+			   "%s (%" PRId64 " B)\n", n, size);
 		}
 
-	      unsigned HOST_WIDE_INT s
-		= shadow_mem_size (current_offset - last_offset);
-	      asan_clear_shadow (shadow_mem, s);
-	      HOST_WIDE_INT shift
-		= shadow_mem_size (current_offset - last_offset + rounded_size);
-	      shadow_mem = adjust_address (shadow_mem, VOIDmode, shift);
-	      last_offset = var_offset + rounded_size;
-	      current_offset = last_offset;
+		last_size += size & ~(ASAN_RED_ZONE_SIZE - HOST_WIDE_INT_1);
 	    }
-
 	}
-
-      /* Handle last redzone.  */
-      current_offset = offsets[0];
-      asan_clear_shadow (shadow_mem,
-			 shadow_mem_size (current_offset - last_offset));
+    }
+  if (last_size)
+    {
+      shadow_mem = adjust_address (shadow_mem, VOIDmode,
+				   (last_offset - prev_offset)
+				   >> ASAN_SHADOW_SHIFT);
+      asan_clear_shadow (shadow_mem, last_size >> ASAN_SHADOW_SHIFT);
     }
 
   /* Clean-up set with instrumented stack variables.  */
@@ -2802,9 +2799,13 @@  asan_expand_mark_ifn (gimple_stmt_iterator *iter)
     decl = TREE_OPERAND (decl, 0);
 
   gcc_checking_assert (TREE_CODE (decl) == VAR_DECL);
-  if (asan_handled_variables == NULL)
-    asan_handled_variables = new hash_set<tree> (16);
-  asan_handled_variables->add (decl);
+
+  if (is_poison)
+    {
+      if (asan_handled_variables == NULL)
+	asan_handled_variables = new hash_set<tree> (16);
+      asan_handled_variables->add (decl);
+    }
   tree len = gimple_call_arg (g, 2);
 
   gcc_assert (tree_fits_shwi_p (len));
-- 
2.13.1