This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Invalid unpoisoning of stack redzones on ARM
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Yury Gribov <y dot gribov at samsung dot com>
- Cc: gcc-patches at gcc dot gnu dot org, dodji at gcc dot gnu dot org, dvyukov at gcc dot gnu dot org, kcc at gcc dot gnu dot org, GarbuzovViacheslav <v dot garbuzov at samsung dot com>, Evgeny Gavrin <e dot gavrin at samsung dot com>
- Date: Tue, 29 Oct 2013 10:37:30 +0100
- Subject: Re: [PATCH] Invalid unpoisoning of stack redzones on ARM
- Authentication-results: sourceware.org; auth=none
- References: <524591E1 dot 9060302 at samsung dot com> <20130927142529 dot GN30970 at tucnak dot zalov dot cz> <5248FBF4 dot 2050807 at samsung dot com> <525240F7 dot 1010409 at samsung dot com> <525E2599 dot 4020803 at samsung dot com>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
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 <jakub@redhat.com>
Yury Gribov <y.gribov@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