llvm dev - Nov 2013 - [LLVMdev] Adding line table debug information to LLVM on Windows

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)
>
> > 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?
>
> 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?

I think we can use the triple OS to choose a sensible default here (win32
-> CodeView, mingw -> DWARF), but eventually some users might want to
force
DWARF on win32 with a flag because it is more complete.

> > 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...
> >
> >> I'm not sure if you're going to want to support both debug
info at the
> >> same time, but it's a readonly format at debug emission so I
don't see
> >> it as being a problem.
> >
> > Well, if two debug formats share the same section name, they might
> > conflict with each other.
> > I don't think this is the case for Dwarf&CodeView [yet?].
> >
> >> -Zmlt makes sense as the clang-cl name, or just
> >> make it whatever the debug mode flag is for cl.exe - this is at
least
> >> a start down that path.
> >
> > 2013/11/19 Reid Kleckner <rnk at google.com>:
> >> I'd just use /Z7, since that's the cl.exe flag for the
compatible
> format.
> >> This will need a long-form clang -cc1 flag name, though.
> >
> > Z7 implies a much more complete debug info than what we'll emit
> short-term.
> > Since Z7 is not widely used, I don't think threading Z7 is any
simpler
> > than Zmlt.
> > I don't have a strong opinion though, WDYT?
>
> [I agreed to use Z7 on this thread]
>
> >> I think the best way for the tests will be to write a set of
parsers,
> >> etc that can dump the info. I realize this is likely a very large
> >> undertaking as well, but it seems like the only way to ensure
we're
> >> getting some decent testing out.
> >
> > As you can see from my patch, I actually succeeded in writing *some*
> > tests without a dumper - just by using the MCAsmStreamer.
> > Do you think we should really write a dumper too?
> > That's kinda hard and we don't plan to fully support the
CodeView format
> yet...
>
> 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?
>
> 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.
>
> >> 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.
> >
> > 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?
>
> ping

I think the problems you mention can be resolved without changing the DI
metadata format.  Debug info metadata already consumes too much memory.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20131120/dc175a28/attachment.html>

2013/11/20 Reid Kleckner <rnk at google.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)
>>
>> > 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?
>>
>> 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?

2013/11/20 Anton Korobeynikov <anton at
korobeynikov.info>:
> On Windows it's perfectly fine to have DWARF encoded into COFF
> objects. E.g. in case of mingw32/-w64 triple.
Yeah, I actually meant to say "in sync with the triple OS", as Reid
suggests.
> I think we can use the triple OS to choose a sensible default here (win32
->
> CodeView, mingw -> DWARF),
Yep
> but eventually some users might want to force
> DWARF on win32 with a flag because it is more complete.
Do you think this is worth doing immediately or this can wait?
>> > 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...
>> >
>> >> I'm not sure if you're going to want to support both
debug info at the
>> >> same time, but it's a readonly format at debug emission so
I don't see
>> >> it as being a problem.
>> >
>> > Well, if two debug formats share the same section name, they might
>> > conflict with each other.
>> > I don't think this is the case for Dwarf&CodeView [yet?].
>> >
>> >> -Zmlt makes sense as the clang-cl name, or just
>> >> make it whatever the debug mode flag is for cl.exe - this is
at least
>> >> a start down that path.
>> >
>> > 2013/11/19 Reid Kleckner <rnk at google.com>:
>> >> I'd just use /Z7, since that's the cl.exe flag for the
compatible
>> >> format.
>> >> This will need a long-form clang -cc1 flag name, though.
>> >
>> > Z7 implies a much more complete debug info than what we'll
emit
>> > short-term.
>> > Since Z7 is not widely used, I don't think threading Z7 is any
simpler
>> > than Zmlt.
>> > I don't have a strong opinion though, WDYT?
>>
>> [I agreed to use Z7 on this thread]
>>
>> >> I think the best way for the tests will be to write a set of
parsers,
>> >> etc that can dump the info. I realize this is likely a very
large
>> >> undertaking as well, but it seems like the only way to ensure
we're
>> >> getting some decent testing out.
>> >
>> > As you can see from my patch, I actually succeeded in writing
*some*
>> > tests without a dumper - just by using the MCAsmStreamer.
>> > Do you think we should really write a dumper too?
>> > That's kinda hard and we don't plan to fully support the
CodeView format
>> > yet...
>>
>> 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?
>>
>> 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.
>>
>> >> 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.
>> >
>> > 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?
>>
>> ping
>
>
> I think the problems you mention can be resolved without changing the DI
> metadata format.  Debug info metadata already consumes too much memory.

On Wed, Nov 20, 2013 at 10:22 AM, Timur Iskhodzhanov <timurrrr at
google.com>wrote:
> > but eventually some users might want to force
> > DWARF on win32 with a flag because it is more complete.
>
> Do you think this is worth doing immediately or this can wait?

Let's wait.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20131120/258c2fcd/attachment.html>