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:
2013-10-29 Jakub Jelinek <ja...@redhat.com>
Yury Gribov <y.gri...@samsung.com>
PR sanitizer/58543
* asan.c (asan_clear_shadow): Return bool whether the emitted loop
(if any) updated shadow_mem's base.
(asan_emit_stack_protection): If asan_clear_shadow returned true,
adjust shadow_mem and prev_offset.
--- gcc/asan.c.jj 2013-10-23 14:43:15.000000000 +0200
+++ gcc/asan.c 2013-10-29 10:26:56.085406934 +0100
@@ -878,10 +878,11 @@ asan_shadow_cst (unsigned char shadow_by
/* Clear shadow memory at SHADOW_MEM, LEN bytes. Can't call a library call
here
though. */
-static void
+static bool
asan_clear_shadow (rtx shadow_mem, HOST_WIDE_INT len)
{
rtx insn, insns, top_label, end, addr, tmp, jump;
+ bool ret;
start_sequence ();
clear_storage (shadow_mem, GEN_INT (len), BLOCK_OP_NORMAL);
@@ -893,12 +894,13 @@ asan_clear_shadow (rtx shadow_mem, HOST_
if (insn == NULL_RTX)
{
emit_insn (insns);
- return;
+ return false;
}
gcc_assert ((len & 3) == 0);
top_label = gen_label_rtx ();
addr = force_reg (Pmode, XEXP (shadow_mem, 0));
+ ret = addr == 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 +914,7 @@ asan_clear_shadow (rtx shadow_mem, HOST_
jump = get_last_insn ();
gcc_assert (JUMP_P (jump));
add_int_reg_note (jump, REG_BR_PROB, REG_BR_PROB_BASE * 80 / 100);
+ return ret;
}
/* Insert code to protect stack vars. The prologue sequence should be emitted
@@ -1048,7 +1051,14 @@ asan_emit_stack_protection (rtx base, HO
(last_offset - prev_offset)
>> ASAN_SHADOW_SHIFT);
prev_offset = last_offset;
- asan_clear_shadow (shadow_mem, last_size >> ASAN_SHADOW_SHIFT);
+ if (asan_clear_shadow (shadow_mem, last_size >> ASAN_SHADOW_SHIFT))
+ {
+ shadow_mem
+ = adjust_automodify_address (shadow_mem, VOIDmode,
+ XEXP (shadow_mem, 0),
+ last_size >> ASAN_SHADOW_SHIFT);
+ prev_offset += last_size;
+ }
last_offset = offset;
last_size = 0;
}
Jakub
