<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">2013/12/3 Eric Christopher <span dir="ltr"><<a href="mailto:echristo@gmail.com" target="_blank" class="cremed">echristo@gmail.com</a>></span><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">>> ><br>
>><br>
>> Probably the same structure that we've been going down via<br>
>> lib/DebugInfo/. A set of files than handle reading and parsing and<br>
>> both some binary files and some files produced by the backend.<br>
><br>
><br>
> Ooph, that's a big one.<br>
> Any more tips to prioritize diving into this codebase?<br>
><br>
<br>
</div>It mostly uses libObject to do the initial read and parse of the<br>
object file. Past that I'd suggest adding a parallel set of<br>
context/structure like the dwarf one. Ultimately it's going to look<br>
somewhat similar.<br>
<div class="im"><br>
>> >> Please see the TODOs in the attached patch.<br>
>> >> You are very likely to come up with a better design/ideas given I'm<br>
>> >> new to this part of the codebase.<br>
>> >><br>
>><br>
>> Will do.<br>
><br>
><br>
> Ping?<br>
><br>
<br>
</div>Can you rebase after the other patches and I'll take a more careful look?</blockquote><div><br></div><div>Which ones?</div><div>It's already rebased on ToT.</div><div><br></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">
-eric<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
>><br>
>> >> One particular question I'd like to emphasize is getting a full<br>
>> >> filepath for a given MDNode.<br>
>> >> As far as I can tell, the metadata for scopes holds pairs of<br>
>> >> <filename, directory>, which reflects how DWARF stores them.<br>
>> >> However, CodeView stores full paths as entire strings (I admit that<br>
>> >> ain't efficient).<br>
>> >> Currently, I concat the directory and filename together, but it<br>
>> >> a) requires some extra memory management<br>
>> >> b) requires special tricks to handle filenames starting from "./",<br>
>> >> "../", etc.<br>
>> >> c) the slashes in the directory name and filename are not consistent on<br>
>> >> Windows<br>
>> >> and is ugly in general.<br>
>> >><br>
>> >> Do you think it's appropriate to change the scope metadata format to<br>
>> >> store <filename, directory, fullpath> instead?<br>
>> >> That'd require changing Clang, right?<br>
>> ><br>
>><br>
>> It would require changing clang. Since this is a single string per<br>
>> file I'm not against adding the full path to the source file in the<br>
>> metadata along side the basename/compilation dir pair.<br>
><br>
><br>
> OK, will go down this path.<br>
><br>
>><br>
>> -eric<br>
>><br>
>> > ping<br>
>> ><br>
>> >>> -eric<br>
>> >>><br>
>> >>><br>
>> >>> On Mon, Nov 18, 2013 at 9:14 AM, Timur Iskhodzhanov<br>
>> >>> <<a href="mailto:timurrrr@google.com" class="cremed">timurrrr@google.com</a>> wrote:<br>
>> >>>> I wrote some more lit tests for my patch and realized I was<br>
>> >>>> generating<br>
>> >>>> some redundant info. This is fixed now. Attached is a new version<br>
>> >>>> of the prototype patch with some more tests.<br>
>> >>>><br>
>> >>>> 2013/11/15 João Matos <<a href="mailto:ripzonetriton@gmail.com" class="cremed">ripzonetriton@gmail.com</a>>:<br>
>> >>>>> Hi Timur,<br>
>> >>>>><br>
>> >>>>> There's also a pending patch adding CodeView support in Phab:<br>
>> >>>>> <a href="http://llvm-reviews.chandlerc.com/D165" target="_blank" class="cremed">http://llvm-reviews.chandlerc.com/D165</a><br>
>> >>>><br>
>> >>>> I haven't looked at your patch yet, but based on the very low phab<br>
>> >>>> review number, I'm pretty sure there are good reasons this wasn't<br>
>> >>>> committed.<br>
>> >>>><br>
>> >>>>> Does your patch provide just a subset of the CodeView debug info<br>
>> >>>>> provided in<br>
>> >>>>> the other patch?<br>
>> >>>><br>
>> >>>> Yes. I prefer small incremental changes.<br>
>> >>>> Also, the file:line debug info is much much more important for me atm<br>
>> >>>> than the other types of debug info.<br>
>> >>>><br>
>> >>>>> Looking at the patch, I think the approach the other patch took of<br>
>> >>>>> abstracting the emission of debug information is a bit cleaner and<br>
>> >>>>> it will<br>
>> >>>>> probably make life easier when adding more debug formats in the<br>
>> >>>>> future.<br>
>> >>>><br>
>> >>>> Why wasn't the "abstract the emission of DI" part of that patch<br>
>> >>>> reviewed/committed separately?<br>
>> >>>><br>
>> >>>>> On Fri, Nov 15, 2013 at 4:39 PM, Timur Iskhodzhanov<br>
>> >>>>> <<a href="mailto:timurrrr@google.com" class="cremed">timurrrr@google.com</a>><br>
>> >>>>> wrote:<br>
>> >>>>>><br>
>> >>>>>> 2013/11/14 Timur Iskhodzhanov <<a href="mailto:timurrrr@google.com" class="cremed">timurrrr@google.com</a>>:<br>
>> >>>>>> > Hi David, Eric, LLVM devs,<br>
>> >>>>>> ><br>
>> >>>>>> > You've probably heard about AddressSanitizer (ASan) and other<br>
>> >>>>>> > sanitizers based on LLVM. One of the things that makes ASan<br>
>> >>>>>> > not as awesome on Windows as it is on Linux<br>
>> >>>>>> > is the symbolization of the stacks.<br>
>> >>>>>> ><br>
>> >>>>>> > Currently, ASan runtime on Windows uses<br>
>> >>>>>> > CaptureStackBackTrace/SymFromAddr/SymGetLineFromAddr64<br>
>> >>>>>> > to unwind and symbolize stacks. This works like a charm<br>
>> >>>>>> > in-process for stack frames built with CL, but yields<br>
>> >>>>>> > "function+0x0ff537" for frames built with Clang.<br>
>> >>>>>> ><br>
>> >>>>>> > I came up with a prototype which emits "old-style debug info"<br>
>> >>>>>> > COFF<br>
>> >>>>>> > sections that are sufficient to get function name / filename /<br>
>> >>>>>> > linenumber information. That's pretty much everything that's<br>
>> >>>>>> > required<br>
>> >>>>>> > for ASan to work beautifully in terms of the completeness of<br>
>> >>>>>> > error<br>
>> >>>>>> > reports.<br>
>> >>>>>> ><br>
>> >>>>>> > Attached is a prototype patch which I've tried on some simple<br>
>> >>>>>> > tests,<br>
>> >>>>>> > including some more complex ones with weird #line constructions.<br>
>> >>>>>> > It also works just great on ASan/Win tests without any<br>
>> >>>>>> > link/run-time<br>
>> >>>>>> > warnings (I had a bunch of those during development, so I can<br>
>> >>>>>> > tell it<br>
>> >>>>>> > works rather than fails silently).<br>
>> >>>>>> ><br>
>> >>>>>> > I didn't have time to work on threading the command-line flags<br>
>> >>>>>> > into<br>
>> >>>>>> > the AsmPrinter yet, so currently it just replaces DWARF entirely.<br>
>> >>>>>> > Of course, this should be fixed before this lands into trunk.<br>
>> >>>>>> > Currently, one can try this patch by using "clang-cl -Xclang -g<br>
>> >>>>>> > ...".<br>
>> >>>>>> > Eventually we should have some dedicated flag for clang-cl.<br>
>> >>>>>> ><br>
>> >>>>>> > Can you please take a look at the patch and suggest a good path<br>
>> >>>>>> > forward?<br>
>> >>>>>> ><br>
>> >>>>>> > I'm very unfamiliar with LLVM CodeGen/MC, so I'm pretty sure I<br>
>> >>>>>> > made a<br>
>> >>>>>> > few weird things in the code... I've also put a few TODOs with<br>
>> >>>>>> > questions and suggestions.<br>
>> >>>>>> ><br>
>> >>>>>> > Some general questions:<br>
>> >>>>>> > 1) Threading flags from the driver down to CodeGen.<br>
>> >>>>>> > How do we do that? Should we support all 4 combinations<br>
>> >>>>>> > of no-info/DWARF/CVLT/both?<br>
>> >>>>>> > How about "-Zmlt" as the clang-cl flag name? ("minimal line<br>
>> >>>>>> > tables")<br>
>> >>>>>> ><br>
>> >>>>>> > 2) Am I right that DWARF is pretty much the only debug info<br>
>> >>>>>> > format<br>
>> >>>>>> > supported by LLVM/AsmPrinter right now? Do we want to take<br>
>> >>>>>> > an effort to come up with a generic debuginfogenerator<br>
>> >>>>>> > interface<br>
>> >>>>>> > to share between DwarfDebug and WinCodeViewLineTables?<br>
>> >>>>>> > Then AsmPrinter should just hold a<br>
>> >>>>>> > SmallVector<DebugInfoEmitter*><br>
>> >>>>>> > rather than a pair of DD/DE pointers.<br>
>> >>>>>> ><br>
>> >>>>>> > 3) How would you suggest to write WinCodeViewLineTables tests<br>
>> >>>>>> > given that dumpbin is not available everywhere except Windows?<br>
>> >>>>>> > // Yeah, I should have looked at the DwarfDebug LIT tests and<br>
>> >>>>>> > // written some; but the prototype development went faster<br>
>> >>>>>> > // than I expected...<br>
>> >>>>>><br>
>> >>>>>> I found the MCAsmStreamer being used by llc which gives a decent<br>
>> >>>>>> text<br>
>> >>>>>> format.<br>
>> >>>>>> I wrote a simple x86+x86_64 .ll test and FileCheck expectations for<br>
>> >>>>>> the llc asm output.<br>
>> >>>>>> Is this the right approach to write tests?<br>
>> >>>>>> If so, I'll convert my remaining C program test cases into such an<br>
>> >>>>>> .ll+llc tests.<br>
>> >>>>>><br>
>> >>>>>> > Can you suggest ways to split this patch so it's easier<br>
>> >>>>>> > to review part-by-part before this hits trunk?<br>
>> >>>>>><br>
>> >>>>>> Attached is an updated patch with a new test and a few minor things<br>
>> >>>>>> improved.<br>
>> >>>>>> I also removed the "TODO: test on X64" as I did try it on x64 and<br>
>> >>>>>> no<br>
>> >>>>>> changes were required.<br>
>> >>>>>><br>
>> >>>>>> Looking forward to your feedback!<br>
>> >>>>>><br>
>> >>>>>> > Thanks!<br>
>> >>>>>> > --<br>
>> >>>>>> > Timur Iskhodzhanov,<br>
>> >>>>>> > Google<br>
>> >>>>>><br>
>> >>>>>> _______________________________________________<br>
>> >>>>>> LLVM Developers mailing list<br>
>> >>>>>> <a href="mailto:LLVMdev@cs.uiuc.edu" class="cremed">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu" target="_blank" class="cremed">http://llvm.cs.uiuc.edu</a><br>
>> >>>>>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank" class="cremed">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br>
>> >>>>>><br>
>> >>>>><br>
>> >>>>><br>
>> >>>>><br>
>> >>>>> --<br>
>> >>>>> João Matos<br>
><br>
><br>
</div></div></blockquote></div><br></div></div>