Bug 360571

(In reply to Anton Kirilov from comment #0)

> The assembly language code that is emitted by ART at the beginning of each
> method, and that causes the warning, looks like this:
> 
> sub r12, sp, #8192
> ldr.w r12, [r12, #0]

My understanding is as follows.  The loaded value is not actually used.  The load is
only performed so as to see if it is actually possible, or whether it takes a segfault
on a no-access guard page placed at the lowest address in the stack area.  Is that
correct?

Yes, that is correct. ART also sets up a signal handler for SIGSEGV.

(In reply to Anton Kirilov from comment #0)
> Currently, the workaround that I have been using is manually patching the
> Valgrind source code (in particular, the VG_GCC296_BUG_STACK_SLOP constant
> in memcheck/mc_errors.c), but a better solution is to introduce a way to
> configure that value at runtime. [..]

That would work, but I think we can do something a bit better here.
Using a renamed and parameterised version of --workaround-gcc296-bugs
will have the effect that all accesses below SP, down to the relevant
offset -- in this case, 8192 -- will be ignored, and so any other
invalid accesses below SP due to real bugs will get ignored.  An
alternative is to introduce a new flag, something like this (eg)

  --ignore-accesses-below-sp-range=<firstoffset>-<lastoffset>

so that just the specified range is ignored.  This would work since
(AFAICS) your accesses are always at a fixed offset, -8192, from SP.
Hence you could use:

  --ignore-accesses-below-sp-range=8189,8192

Then --workaround-gcc296-bugs=yes can be deprecated and declared to be
a synonym for 

  --ignore-accesses-below-sp-range=0,1023

Mark: would that also work for your use case(s)?

(In reply to Julian Seward from comment #3)
> An alternative is to introduce a new flag, something like this (eg)
> 
>   --ignore-accesses-below-sp-range=<firstoffset>-<lastoffset>
> 
> so that just the specified range is ignored.
Yes, that will work as well.

Just a quick idea - do you think it is worth it supporting more than one range (e.g. for running code generated by a compiler that misbehaves in the same way as GCC 2.96 in an environment similar to Android)?

(In reply to Anton Kirilov from comment #4)
> Just a quick idea - do you think it is worth it supporting more than one
> range (e.g. for running code generated by a compiler that misbehaves in the
> same way as GCC 2.96 in an environment similar to Android)?

Well, not really.  The gcc-2.96 style read/write below SP errors are real bugs
and I'd prefer to make it hard to hide those :-).  In the worst case you could
simply use
  --ignore-accesses-below-sp-range=0,8191

and then they would all disappear.

Do you have an actual use case like this?  Or is it more of a theoretical question?

Just a theoretical question.

Created attachment 101476 [details]
Proposed fix (lacks documentation, but seems to work)

For example, to keep the test program (next attachment) happy:

  ./vg-in-place --ignore-range-below-sp=8192-8189 ./access_below_sp 

Note that the first specified offset must be larger than the second, so as to
imply a non-wraparound address range.  This is because they are really
negative offsets relative to the stack pointer.

Created attachment 101477 [details]
A simple test program.

Anton, can you perhaps try this on aarch64 ?  Would this work for you?
(Apologies .. there's one line in the test program you'll have to change.)

I tried the fix both on Aarch32 and Aarch64, and it worked. I just modified lines 35-36 of the test program to either:

 __asm__ __volatile__("sub r0, sp, #8192\n\tldr %0, [r0, #0]"
                                     : "=r"(res) : : "r0", "memory","cc");

for Aarch32, or:

 __asm__ __volatile__("sub x0, sp, #8192\n\tldr %w0, [x0, #0]"
                                     : "=r"(res) : : "r0", "memory","cc");

for Aarch64.

I also ran a simple test application with ART, and this particular issue disappeared (although Valgrind complained about other things).

Committed on trunk, r16073.