[llvm-commits] [llvm] r160254 - in /llvm/trunk: lib/Transforms/Instrumentation/AddressSanitizer.cpp test/Instrumentation/AddressSanitizer/basic.ll

[Kostya Serebryany]

On Tue, Jul 17, 2012 at 4:57 PM, Joerg Sonnenberger <joerg at britannica.bec.de
> wrote:

> On Tue, Jul 17, 2012 at 01:14:39PM +0400, Kostya Serebryany wrote:
> > On Tue, Jul 17, 2012 at 1:08 PM, Joerg Sonnenberger <
> joerg at britannica.bec.de
> > > wrote:
> >
> > > On Mon, Jul 16, 2012 at 05:34:56PM +0400, Kostya Serebryany wrote:
> > > > First Chandler's suggestion was to replace this code with
> > > >
> > > > if (*shadow1) { arg = addr1; goto report_read4; }
> > > > if (*shadow2) { arg = addr2; goto report_read4; }
> > > > if (*shadow3) { arg = addr3; goto report_read4; }
> > > > return;
> > > > report_read4:
> > > >   __asan_report_read4(arg);
> > >
> > > What about not using call directly for __asan_report_read4, but
> building
> > > the stack frame with explicit push + jmp? At least for x86 that is
> > > trivial to implement and not that much larger in terms of code.
> > >
> >
> > Could you please explain more?
> > (And one thing I want to avoid is to add inline asm into the LLVM
> > instrumentation module)
>
> The problem for __builtin_return_address is that the code merging places
> the wrong return address on the stack, right?


Right.
Currently, we have unique call instruction per every instrumented memory
access and thus we encode the PC in this call instruction,
which is never executed and resides in cold region.
If the calls get merged, the information about PC is lost and asan starts
reporting errors with incorrect top frame.
If we pass PC as the second parameter to __asan_report_*, the code gets
bigger and slower.


> The other potential issue
> is that the dead attribute might allow the compiler to skip the call in
> first place and implement it like a tail call.


I don't think this may happen unless we explicitly add the tail call
attribute to the call (and we don't).


> So in terms of lowering,
> LLVM could replace the "call __asan_report_read4" instruction with
> "pushq %0; jmp __asan_report_read4" or so. Alternatively, just change
> the ABI to not depend on __builtin_return_address sounds fine as well?
>

How?

--kcc


>
> Joerg
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120717/7d21142e/attachment.html>