<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Oct 30, 2013 at 1:32 PM, Will Dietz <span dir="ltr"><<a href="mailto:w@wdtz.org" target="_blank">w@wdtz.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Done in r193711, thanks for the fix!<br></blockquote><div><br></div><div>Great, thank you.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
~Will<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
On Mon, Oct 28, 2013 at 6:21 PM, Alexey Samsonov <<a href="mailto:samsonov@google.com">samsonov@google.com</a>> wrote:<br>
><br>
> On Thu, Oct 17, 2013 at 11:56 AM, Will Dietz <<a href="mailto:w@wdtz.org">w@wdtz.org</a>> wrote:<br>
>><br>
>> Ah, well after your DebugInfo change re:form classes lands,<br>
>> this patch is almost certainly going about this the wrong way.<br>
>><br>
>> If you'd like I can update the patch, but since you just refactored<br>
>> things to support doing this properly it seems you should just<br>
>> go ahead and fix this yourself (compared to overhead of managing me doing<br>
>> it).<br>
>><br>
>> The testcase is probably still useful regardless :).<br>
><br>
><br>
> Hi Will, I think I've fixed the problem in r193555.<br>
> Could you please commit your testcase with the following improvements:<br>
> * Add the original source code with descriptions of how to build the object<br>
> file (which compiler was used etc.) See<br>
> test/DebugInfo/Inputs/dwarfdump-test.cc for inspiration.<br>
> * Rewrite the test to use llvm-symbolizer (it will probably be shorter), or<br>
> just extend test/DebugInfo/llvm-symbolizer.test<br>
><br>
> Thanks!<br>
><br>
>><br>
>><br>
>> No worries, would rather see this done properly for sure.<br>
>> Thanks!<br>
>><br>
>> ~Will<br>
>><br>
>><br>
>> On Thu, Oct 17, 2013 at 1:20 PM, Will Dietz <<a href="mailto:w@wdtz.org">w@wdtz.org</a>> wrote:<br>
>> > Great, thanks! You were absolutely right regarding the high_pc thing,<br>
>> > sorry for misdiagnosing the issue (and thanks for figuring this out!).<br>
>> ><br>
>> > Attached is a patch that attempts to address this issue,<br>
>> > with included test case (by far the larger time-sink after your<br>
>> > spot-on analysis).<br>
>> ><br>
>> > Thanks for taking a look and letting me take a crack at fixing this<br>
>> > properly :).<br>
>> ><br>
>> > ~Will<br>
>> ><br>
>> ><br>
>> > On Thu, Oct 17, 2013 at 5:49 AM, Alexey Samsonov <<a href="mailto:samsonov@google.com">samsonov@google.com</a>><br>
>> > wrote:<br>
>> >> Hi Will,<br>
>> >><br>
>> >> On Thu, Oct 17, 2013 at 12:51 AM, Will Dietz <<a href="mailto:w@wdtz.org">w@wdtz.org</a>> wrote:<br>
>> >>><br>
>> >>> Fixes bug exposed by tsan lit tests where addresses fail to be<br>
>> >>> symbolized on my system (x86_64 linux).<br>
>> >>><br>
>> >>> An example of this failure can be seen with this binary[1]:<br>
>> >>><br>
>> >>> $ unxz ./thread_leak.3.c.tmp.xz<br>
>> >>><br>
>> >>> Using llvm-symbolizer from ToT:<br>
>> >>><br>
>> >>> $ echo ./thread_leak3.c.tmp 0x020560 |<br>
>> >>> ~/llvm/build/bin/llvm-symbolizer<br>
>> >>> __interceptor_pthread_create<br>
>> >>> ??:0:0<br>
>> >>><br>
>> >>> And after:<br>
>> >>><br>
>> >>> $ echo ./thread_leak3.c.tmp 0x020560|~/llvm/build/bin/llvm-symbolizer<br>
>> >>> __interceptor_pthread_create<br>
>> >>><br>
>> >>><br>
>> >>> /home/will/llvm/latest/projects/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:876:0<br>
>> >>><br>
>> >>> Proposed patch attached.<br>
>> >><br>
>> >><br>
>> >> Interesting. I agree that llvm-symbolizer should perform at least as<br>
>> >> good<br>
>> >> with --inlining=true<br>
>> >> than without it, but the fix should probably go the DWARF parser in<br>
>> >> DebugInfo library.<br>
>> >><br>
>> >> Here's the DWARF entry for __interceptor_pthread_create in your binary:<br>
>> >><br>
>> >> 0x0002c37a:   DW_TAG_subprogram [182] *<br>
>> >>                 DW_AT_external [DW_FORM_flag_present]   (true)<br>
>> >>                 DW_AT_name [DW_FORM_strp]       (<br>
>> >> .debug_str[0x00012957] =<br>
>> >> "__interceptor_pthread_create")<br>
>> >>                 DW_AT_decl_file [DW_FORM_data1] (0x01)<br>
>> >>                 DW_AT_decl_line [DW_FORM_data2] (0x036b)<br>
>> >>                 DW_AT_type [DW_FORM_ref4]       (cu + 0x49a0 =><br>
>> >> {0x00014e6e})<br>
>> >>                 DW_AT_low_pc [DW_FORM_addr]     (0x0000000000020560)<br>
>> >>                 DW_AT_high_pc [DW_FORM_data8]   (0x000000000000015f)<br>
>> >> <-------------------- [1]<br>
>> >>                 DW_AT_frame_base [DW_FORM_exprloc]      (<0x1> 9c )<br>
>> >>                 DW_AT_Unknown_2117 [DW_FORM_flag_present]       (true)<br>
>> >>                 DW_AT_sibling [DW_FORM_ref4]    (cu + 0x1c267 =><br>
>> >> {0x0002c735})<br>
>> >><br>
>> >> [1] DW_AT_high_pc has different DW_FORM here and actually means the<br>
>> >> size of<br>
>> >> the functions, not its largest address.<br>
>> >> My guess is DWARF parser doesn't know about this and fails to build a<br>
>> >> correct address range for the function. I'm going<br>
>> >> to investigate this soon.<br>
>> >><br>
>> >> Thanks for the reproducer!<br>
>> >><br>
>> >>><br>
>> >>> ~Will<br>
>> >>><br>
>> >>> [1] <a href="http://wdtz.org/files/thread_leak3.c.tmp.xz" target="_blank">http://wdtz.org/files/thread_leak3.c.tmp.xz</a><br>
>> >>><br>
>> >>> _______________________________________________<br>
>> >>> llvm-commits mailing list<br>
>> >>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>> >>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
>> >>><br>
>> >><br>
>> >><br>
>> >><br>
>> >> --<br>
>> >> Alexey Samsonov, MSK<br>
><br>
><br>
><br>
><br>
> --<br>
> Alexey Samsonov, MSK<br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div>Alexey Samsonov, MSK</div>
</div></div>