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 Tue, Oct 29, 2013 at 12:25:33PM +0000, Richard Sandiford wrote:
>> >> 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?
>
> Why?

Well, that was the traditional rule:

   The caller must not alter the value in the register we return,
   since we mark it as a "constant" register.  */

rtx
force_reg (enum machine_mode mode, rtx x)
{

but maybe it doesn't matter now, since the constant register flag has
gone and since we seem to use REG_EQUAL rather than REG_EQUIV...

> If it is a pseudo, it is certainly a pseudo that isn't used for
> anything else, as it is the result of (base >> 3) + constant, if it isn't a
> pseudo, then supposedly it is better not to just keep adding the offsets to
> a base and thus not to use the end address from asan_clear_shadow.
> Your patch would force using the end address even if shadow_base
> is initially say some_reg + 32, I think it is better to use shadow_mem
> some_reg + 72 next time instead of some_other_reg + 40.

But I thought the register pressure thing was to avoid having the
original base live across the loop.  Doesn't that apply for reg + offset
as well as plain reg?  If we have to force some_reg + 32 into a
temporary and then loop on it, using the new base should avoid keeping
both it and some_reg live at the same time.  Plus smaller offsets are
often better on some targets.

Thanks,
Richard


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