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] Invalid unpoisoning of stack redzones on ARM


Jakub Jelinek <jakub@redhat.com> writes:
> On Wed, Oct 16, 2013 at 09:35:21AM +0400, Yury Gribov wrote:
>> >>> I've recently submitted a bug report regarding invalid
>> unpoisoning of stack frame redzones
>> (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58543). Could someone
>> take a look at proposed patch (a simple one-liner) and check whether
>> it's ok for commit?
>> >>
>> >> Can you please be more verbose
>> >
>> > Do you think the proposed patch is ok or should I provide some
>> additional info?
>> 
>> Jakub, Dodji,
>> 
>> Any updates on this one? Note that this bug is a huge blocker for
>> using AddressSanitizer on ARM platforms.
>
> Sorry for the delay, I finally found time to look at it.
> While your patch fixes the issue, I wonder if for the case where
> shadow_mem before the asan_clear_shadow call is already of the form
> (mem (reg)) it isn't better to just adjust next asan_clear_shadow
> base from the end of the cleared region rather than from the start of it,
> because the end will be already in the right pseudo, while with your
> approach it needs to be done in a different register and potentially
> increase register pressure.  So here is (completely untested) alternate fix:

Hmm, it just seems wrong to be assigning to registers returned by force_reg
and expand_binop though.  Would this variation be OK?

Thanks,
Richard


Index: gcc/asan.c
===================================================================
--- gcc/asan.c	(revision 204124)
+++ gcc/asan.c	(working copy)
@@ -876,9 +876,10 @@
 }
 
 /* Clear shadow memory at SHADOW_MEM, LEN bytes.  Can't call a library call here
-   though.  */
+   though.  If the address of the next byte is in a known register, return
+   that register, otherwise return null.  */
 
-static void
+static rtx
 asan_clear_shadow (rtx shadow_mem, HOST_WIDE_INT len)
 {
   rtx insn, insns, top_label, end, addr, tmp, jump;
@@ -893,12 +894,12 @@
   if (insn == NULL_RTX)
     {
       emit_insn (insns);
-      return;
+      return 0;
     }
 
   gcc_assert ((len & 3) == 0);
   top_label = gen_label_rtx ();
-  addr = force_reg (Pmode, XEXP (shadow_mem, 0));
+  addr = copy_to_mode_reg (Pmode, XEXP (shadow_mem, 0));
   shadow_mem = adjust_automodify_address (shadow_mem, SImode, addr, 0);
   end = force_reg (Pmode, plus_constant (Pmode, addr, len));
   emit_label (top_label);
@@ -912,6 +913,7 @@
   jump = get_last_insn ();
   gcc_assert (JUMP_P (jump));
   add_int_reg_note (jump, REG_BR_PROB, REG_BR_PROB_BASE * 80 / 100);
+  return addr;
 }
 
 /* Insert code to protect stack vars.  The prologue sequence should be emitted
@@ -1048,7 +1050,15 @@
 				       (last_offset - prev_offset)
 				       >> ASAN_SHADOW_SHIFT);
 	  prev_offset = last_offset;
-	  asan_clear_shadow (shadow_mem, last_size >> ASAN_SHADOW_SHIFT);
+	  rtx end_addr = asan_clear_shadow (shadow_mem,
+					    last_size >> ASAN_SHADOW_SHIFT);
+	  if (end_addr)
+	    {
+	      shadow_mem
+		= adjust_automodify_address (shadow_mem, VOIDmode, end_addr,
+					     last_size >> ASAN_SHADOW_SHIFT);
+	      prev_offset += last_size;
+	    }
 	  last_offset = offset;
 	  last_size = 0;
 	}


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