On Thu, Feb 20, 2014 at 04:19:12PM +0800, lin zuojian wrote:
> Without aligning the asan stack base,base will only 64-bit aligned in
> ARM machines.
> But asan require 256-bit aligned base because of this:
> 1.right shift take ASAN_SHADOW_SHIFT(which is 3) bits are zeros
> 2.store multiple/load multiple instructions require the other 2 bits are
> zeros
>
> that add up lowest 5 bits should be zeros.That means 32 bytes or 256
> bits aligned.
>
> * asan.c (asan_emit_stack_protection): Forcing the base to align to 256 bits
> * cfgexpand.c (expand_used_vars): Leaving a space in the stack frame for
> alignment
This is wrong.
You certainly don't want to do any of the extra aligning
on non-strict alignment targets, so all the changes must be
if (STRICT_ALIGNMENT) guarded, as they are quite expensive (you need one
extra register in that case).
Second thing is, I think you break --param use-after-return=0 this way
and also functions with very large stack frames, because emit_move_insn
to pbase is then done before you adjust the stack.
Also, I don't think you want to align for ASAN_RED_ZONE_SIZE,
but instead (GET_MODE_ALIGNMENT (SImode) << ASAN_SHADOW_SHIFT) / BITS_PER_UNIT,
and if you do (of course for STRICT_ALIGNMENT targets only), you should
set_mem_align on shadow_mem when it is created.
If you do the realignment, then it would be nice if the middle-end was told
about that, so do
base_align = crtl->max_used_stack_slot_alignment;
in both the if ((flag_sanitize & SANITIZE_ADDRESS) && ASAN_STACK && pred)
then body and else, and if sanitizing and STRICT_ALIGNMENT, set
base_align to MIN of crtl->max_used_stack_slot_alignment and the alignment
you actually guarantee.
Or, if ARM supports unaligned loads/stores using special instructions,
perhaps you should also benchmark the alternative of not realigning, but
instead making sure those unaligned instructions are used for the shadow
memory loads/stores in the asan prologue/epilogue.
Please next time post the patch from a sane MUA that doesn't eat tabs or
at least as an attachment. Comments should start with a capital letter, and
with a full stop followed by two spaces, in the ChangeLog also all entries
should end with a full stop.
Jakub
