Ok, r149994.<div>thanks!<br><div><br><br><div class="gmail_quote">On Tue, Feb 7, 2012 at 10:18 AM, Timur Iskhodzhanov <span dir="ltr"><<a href="mailto:timurrrr@google.com">timurrrr@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On Tue, Feb 7, 2012 at 10:05 PM, Kostya Serebryany <<a href="mailto:kcc@google.com">kcc@google.com</a>> wrote:<br>
> Much better.<br>
> Now, how about using _AddressOfReturnAddress() for the frame?<br>
</div>This will be a separate patch, see the reasoning below.<br>
<br>
On Windows, the value of GET_CURRENT_FRAME is only used when printing<br>
ASan reports and it is named "bp=".<br>
(we use CaptureStackBackTrace to get stack traces now, it doesn't<br>
require to know bp)<br></blockquote><div><br></div><div>CaptureStackBackTrace may appear to be prohibitively slow in the long run. </div><div>But ok for now. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

_AddressOfReturnAddress returns "ebp+4" on x86, which isn't a problem;<br>
(but this might not be true for all the calling conventions? need to check)<br>
<br>
On x64 however rbp is not used making function calls (tested on<br>
helloworld-like app calling foo(), rbp==0!) and _AddressOfReturnAddress<br>
actually returns rsp+40 (which is likely to always stay true as<br>
there's only one calling convention on Win x64)<br>
<br>
Also, it's not clear if we need to print ebp at all on Windows in the<br>
ASan reports.<br></blockquote><div><br></div><div>bp= is useful when you have weird stack-buffer-overflow reports that are hard to reason about. </div><div><br></div><div>--kcc </div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div class="HOEnZb"><div class="h5"><br>
> --kcc<br>
><br>
><br>
> On Tue, Feb 7, 2012 at 9:56 AM, Timur Iskhodzhanov <<a href="mailto:timurrrr@google.com">timurrrr@google.com</a>><br>
> wrote:<br>
>><br>
>> PTAL,<br>
>> though personally I don't like how it looks now.<br>
>><br>
>> On Tue, Feb 7, 2012 at 9:36 PM, Kostya Serebryany <<a href="mailto:kcc@google.com">kcc@google.com</a>> wrote:<br>
>> ><br>
>> ><br>
>> > On Tue, Feb 7, 2012 at 4:51 AM, <<a href="mailto:timurrrr@google.com">timurrrr@google.com</a>> wrote:<br>
>> >><br>
>> >><br>
>> >> <a href="http://codereview.appspot.com/5630065/diff/1/lib/asan/asan_internal.h" target="_blank">http://codereview.appspot.com/5630065/diff/1/lib/asan/asan_internal.h</a><br>
>> >> File lib/asan/asan_internal.h (right):<br>
>> >><br>
>> >><br>
>> >><br>
>> >> <a href="http://codereview.appspot.com/5630065/diff/1/lib/asan/asan_internal.h#newcode24" target="_blank">http://codereview.appspot.com/5630065/diff/1/lib/asan/asan_internal.h#newcode24</a><br>
>> >> lib/asan/asan_internal.h:24: # include <intrin.h><br>
>> >> On 2012/02/06 19:18:31, kcc wrote:<br>
>> >>><br>
>> >>> Can we avoid this include in the header?<br>
>> >>> ( I am not sure we can, just asking)<br>
>> >><br>
>> >> I doubt so.<br>
>> >> Or we'll need to #include it in every file which uses GET_CURRENT_PC or<br>
>> >> GET_BP_PC_SP.<br>
>> ><br>
>> ><br>
>> > Maybe you can find how this thing is defined in intrin.h and use similar<br>
>> > definition?<br>
>> ><br>
>> ><br>
>> > --kcc<br>
><br>
><br>
</div></div></blockquote></div><br></div></div>