[LLVMdev] Adding line table debug information to LLVM on Windows

Tue Dec 3 09:02:36 PST 2013

2013/11/21 Eric Christopher <echristo at gmail.com>

> On Wed, Nov 20, 2013 at 9:46 AM, Timur Iskhodzhanov <timurrrr at google.com>
> wrote:
> > Eric, David,
> >
> > 2013/11/19 Timur Iskhodzhanov <timurrrr at google.com>:
> >> Attached is a slightly updated patch.
> >> (it doesn't include D2222 yet).
> >
> > The new version of the patch stopped fitting into the llvmdev 100K limit,
> > so I've uploaded it to http://llvm-reviews.chandlerc.com/D2232
> > (you need to apply D2222 first if you'd like to give D2232 a try)
> >
>
> That's a big patch. :)
>

Mostly because of the tests, I hope :)

At least I already committed some obviously-independent parts of it...

> >> 2013/11/19 Eric Christopher <echristo at gmail.com>:
> >>> In general I do think we're going to need to abstract it out as much
> >>> as possible. I'm not sure what the previous patch looks like, but
> >>> abstracting the interface out would be general goodness for this. We
> >>> can talk about designs for that as we move on.
> >>
> >> How about http://llvm-reviews.chandlerc.com/D2222 ?
> >> // Ha! A lucky number!
> >>
> >>> As far as how to
> >>> migrate the decision down we can have it both as an option to code gen
> >>> maybe or, for now, make it dependent upon triple. The former is, I
> >>> think, the best option there.
> >>
> >> You mean LLVM CodeGen or Clang CodeGen?
> >
>
> LLVM... and we really need to change that directory in Clang. :)
>
> > On the second thought, "llc" seems to choose ELF vs COFF by using a
> triple.
> > I think we should make the DWARF vs CodeView choice in sync with the
> > object file format choice, at least to begin with.
> > WDYT?
> >
>
> Yep. Sounds good to me. I'd suggest triple, that way you can base it
> on "windows" versus "coff".
>
> >> Can you suggest a few places in the code where I can find the clues for
> that?
> >> I'm not yet familiar with this part of the project...
> >>
>
> lib/CodeGen/AsmPrinter/DwarfDebug.cpp has some checks for triple/os.

OK, I use the Triple().isOSWindows() now.

>  >
> > I tried my patch on Chromium and it hit the llvm_unreachable I wrote
> > in WinCOFFStreamer::EmitCOFFStaticOffset().
> > Now that it also supports using a fixup to calculate the offset (that
> > happens as the second pass, not supported by MCAsmStreamer, right?), I
> > think I do have to write at least a simple dumper...
> >
> > Any hints on how to do that? Should it be a separate app or built into
> > some COFF reader readily available? Should it have its own tests, i.e.
> > binary COFF files added to the repo?
> >
>
> Probably the same structure that we've been going down via
> lib/DebugInfo/. A set of files than handle reading and parsing and
> both some binary files and some files produced by the backend.

Ooph, that's a big one.
Any more tips to prioritize diving into this codebase?

>  > Good news - other than that, the emitter seems to work fine on some
> > medium-sized Chromium tests and generate symbolized ASan reports if I
> > manually introduce an error in the binary.
> >
>
> Awesome.
>
> >>> Any other questions I missed?
> >>
> >> Please see the TODOs in the attached patch.
> >> You are very likely to come up with a better design/ideas given I'm
> >> new to this part of the codebase.
> >>
>
> Will do.

Ping?

> >> One particular question I'd like to emphasize is getting a full
> >> filepath for a given MDNode.
> >> As far as I can tell, the metadata for scopes holds pairs of
> >> <filename, directory>, which reflects how DWARF stores them.
> >> However, CodeView stores full paths as entire strings (I admit that
> >> ain't efficient).
> >> Currently, I concat the directory and filename together, but it
> >> a) requires some extra memory management
> >> b) requires special tricks to handle filenames starting from "./",
> "../", etc.
> >> c) the slashes in the directory name and filename are not consistent on
> Windows
> >> and is ugly in general.
> >>
> >> Do you think it's appropriate to change the scope metadata format to
> >> store <filename, directory, fullpath> instead?
> >> That'd require changing Clang, right?
> >
>
> It would require changing clang. Since this is a single string per
> file I'm not against adding the full path to the source file in the
> metadata along side the basename/compilation dir pair.

OK, will go down this path.

>  -eric
>
> > ping
> >
> >>> -eric
> >>>
> >>>
> >>> On Mon, Nov 18, 2013 at 9:14 AM, Timur Iskhodzhanov <
> timurrrr at google.com> wrote:
> >>>> I wrote some more lit tests for my patch and realized I was generating
> >>>> some redundant info. This is fixed now. Attached is a new version
> >>>> of the prototype patch with some more tests.
> >>>>
> >>>> 2013/11/15 João Matos <ripzonetriton at gmail.com>:
> >>>>> Hi Timur,
> >>>>>
> >>>>> There's also a pending patch adding CodeView support in Phab:
> >>>>> http://llvm-reviews.chandlerc.com/D165
> >>>>
> >>>> I haven't looked at your patch yet, but based on the very low phab
> >>>> review number, I'm pretty sure there are good reasons this wasn't
> >>>> committed.
> >>>>
> >>>>> Does your patch provide just a subset of the CodeView debug info
> provided in
> >>>>> the other patch?
> >>>>
> >>>> Yes. I prefer small incremental changes.
> >>>> Also, the file:line debug info is much much more important for me atm
> >>>> than the other types of debug info.
> >>>>
> >>>>> Looking at the patch, I think the approach the other patch took of
> >>>>> abstracting the emission of debug information is a bit cleaner and
> it will
> >>>>> probably make life easier when adding more debug formats in the
> future.
> >>>>
> >>>> Why wasn't the "abstract the emission of DI" part of that patch
> >>>> reviewed/committed separately?
> >>>>
> >>>>> On Fri, Nov 15, 2013 at 4:39 PM, Timur Iskhodzhanov <
> timurrrr at google.com>
> >>>>> wrote:
> >>>>>>
> >>>>>> 2013/11/14 Timur Iskhodzhanov <timurrrr at google.com>:
> >>>>>> > Hi David, Eric, LLVM devs,
> >>>>>> >
> >>>>>> > You've probably heard about AddressSanitizer (ASan) and other
> >>>>>> > sanitizers based on LLVM. One of the things that makes ASan
> >>>>>> > not as awesome on Windows as it is on Linux
> >>>>>> > is the symbolization of the stacks.
> >>>>>> >
> >>>>>> > Currently, ASan runtime on Windows uses
> >>>>>> > CaptureStackBackTrace/SymFromAddr/SymGetLineFromAddr64
> >>>>>> > to unwind and symbolize stacks.  This works like a charm
> >>>>>> > in-process for stack frames built with CL, but yields
> >>>>>> > "function+0x0ff537" for frames built with Clang.
> >>>>>> >
> >>>>>> > I came up with a prototype which emits "old-style debug info" COFF
> >>>>>> > sections that are sufficient to get function name / filename /
> >>>>>> > linenumber information.  That's pretty much everything that's
> required
> >>>>>> > for ASan to work beautifully in terms of the completeness of error
> >>>>>> > reports.
> >>>>>> >
> >>>>>> > Attached is a prototype patch which I've tried on some simple
> tests,
> >>>>>> > including some more complex ones with weird #line constructions.
> >>>>>> > It also works just great on ASan/Win tests without any
> link/run-time
> >>>>>> > warnings (I had a bunch of those during development, so I can
> tell it
> >>>>>> > works rather than fails silently).
> >>>>>> >
> >>>>>> > I didn't have time to work on threading the command-line flags
> into
> >>>>>> > the AsmPrinter yet, so currently it just replaces DWARF entirely.
> >>>>>> > Of course, this should be fixed before this lands into trunk.
> >>>>>> > Currently, one can try this patch by using "clang-cl -Xclang -g
> ...".
> >>>>>> > Eventually we should have some dedicated flag for clang-cl.
> >>>>>> >
> >>>>>> > Can you please take a look at the patch and suggest a good path
> forward?
> >>>>>> >
> >>>>>> > I'm very unfamiliar with LLVM CodeGen/MC, so I'm pretty sure I
> made a
> >>>>>> > few weird things in the code...  I've also put a few TODOs with
> >>>>>> > questions and suggestions.
> >>>>>> >
> >>>>>> > Some general questions:
> >>>>>> > 1) Threading flags from the driver down to CodeGen.
> >>>>>> >   How do we do that? Should we support all 4 combinations
> >>>>>> >   of no-info/DWARF/CVLT/both?
> >>>>>> >   How about "-Zmlt" as the clang-cl flag name? ("minimal line
> tables")
> >>>>>> >
> >>>>>> > 2) Am I right that DWARF is pretty much the only debug info format
> >>>>>> >   supported by LLVM/AsmPrinter right now?  Do we want to take
> >>>>>> >   an effort to come up with a generic debuginfogenerator interface
> >>>>>> >   to share between DwarfDebug and WinCodeViewLineTables?
> >>>>>> >   Then AsmPrinter should just hold a
> SmallVector<DebugInfoEmitter*>
> >>>>>> >   rather than a pair of DD/DE pointers.
> >>>>>> >
> >>>>>> > 3) How would you suggest to write WinCodeViewLineTables tests
> >>>>>> >   given that dumpbin is not available everywhere except Windows?
> >>>>>> >   // Yeah, I should have looked at the DwarfDebug LIT tests and
> >>>>>> >   // written some; but the prototype development went faster
> >>>>>> >   // than I expected...
> >>>>>>
> >>>>>> I found the MCAsmStreamer being used by llc which gives a decent
> text
> >>>>>> format.
> >>>>>> I wrote a simple x86+x86_64 .ll test and FileCheck expectations for
> >>>>>> the llc asm output.
> >>>>>> Is this the right approach to write tests?
> >>>>>> If so, I'll convert my remaining C program test cases into such an
> >>>>>> .ll+llc tests.
> >>>>>>
> >>>>>> > Can you suggest ways to split this patch so it's easier
> >>>>>> > to review part-by-part before this hits trunk?
> >>>>>>
> >>>>>> Attached is an updated patch with a new test and a few minor things
> >>>>>> improved.
> >>>>>> I also removed the "TODO: test on X64" as I did try it on x64 and no
> >>>>>> changes were required.
> >>>>>>
> >>>>>> Looking forward to your feedback!
> >>>>>>
> >>>>>> > Thanks!
> >>>>>> > --
> >>>>>> > Timur Iskhodzhanov,
> >>>>>> > Google
> >>>>>>
> >>>>>> _______________________________________________
> >>>>>> LLVM Developers mailing list
> >>>>>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> >>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> >>>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>> --
> >>>>> João Matos
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20131203/1d9ab517/attachment.html>