[llvm-commits] Implement the GET_CALLER_PC macro, stub the GET_CURRENT_FRAME macro (issue 5630065)

[Kostya Serebryany]

Ok, r149994.
thanks!


On Tue, Feb 7, 2012 at 10:18 AM, Timur Iskhodzhanov <timurrrr at google.com>wrote:

> On Tue, Feb 7, 2012 at 10:05 PM, Kostya Serebryany <kcc at google.com> wrote:
> > Much better.
> > Now, how about using _AddressOfReturnAddress() for the frame?
> This will be a separate patch, see the reasoning below.
>
> On Windows, the value of GET_CURRENT_FRAME is only used when printing
> ASan reports and it is named "bp=".
> (we use CaptureStackBackTrace to get stack traces now, it doesn't
> require to know bp)
>

CaptureStackBackTrace may appear to be prohibitively slow in the long run.
But ok for now.


> _AddressOfReturnAddress returns "ebp+4" on x86, which isn't a problem;
> (but this might not be true for all the calling conventions? need to check)
>
> On x64 however rbp is not used making function calls (tested on
> helloworld-like app calling foo(), rbp==0!) and _AddressOfReturnAddress
> actually returns rsp+40 (which is likely to always stay true as
> there's only one calling convention on Win x64)
>
> Also, it's not clear if we need to print ebp at all on Windows in the
> ASan reports.
>

bp= is useful when you have weird stack-buffer-overflow reports that are
hard to reason about.

--kcc



>
> > --kcc
> >
> >
> > On Tue, Feb 7, 2012 at 9:56 AM, Timur Iskhodzhanov <timurrrr at google.com>
> > wrote:
> >>
> >> PTAL,
> >> though personally I don't like how it looks now.
> >>
> >> On Tue, Feb 7, 2012 at 9:36 PM, Kostya Serebryany <kcc at google.com>
> wrote:
> >> >
> >> >
> >> > On Tue, Feb 7, 2012 at 4:51 AM, <timurrrr at google.com> wrote:
> >> >>
> >> >>
> >> >>
> http://codereview.appspot.com/5630065/diff/1/lib/asan/asan_internal.h
> >> >> File lib/asan/asan_internal.h (right):
> >> >>
> >> >>
> >> >>
> >> >>
> http://codereview.appspot.com/5630065/diff/1/lib/asan/asan_internal.h#newcode24
> >> >> lib/asan/asan_internal.h:24: # include <intrin.h>
> >> >> On 2012/02/06 19:18:31, kcc wrote:
> >> >>>
> >> >>> Can we avoid this include in the header?
> >> >>> ( I am not sure we can, just asking)
> >> >>
> >> >> I doubt so.
> >> >> Or we'll need to #include it in every file which uses GET_CURRENT_PC
> or
> >> >> GET_BP_PC_SP.
> >> >
> >> >
> >> > Maybe you can find how this thing is defined in intrin.h and use
> similar
> >> > definition?
> >> >
> >> >
> >> > --kcc
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120207/75f7d3b7/attachment.html>