[PATCH] D50005: [DWARF] Basic support for producing DWARFv5 .debug_addr section

[David Blaikie via Phabricator via llvm-commits]

dblaikie accepted this revision.
dblaikie added inline comments.
This revision is now accepted and ready to land.


================
Comment at: test/DebugInfo/X86/debug_addr.ll:9
+; void foo() {
+;   return;
+; }
----------------
vleschuk wrote:
> dblaikie wrote:
> > These return statements seem unnecessary? (in the text of this comment, and in the IR of the sample - not that it'd make much of a difference)
> Removed them from the comment, not sure that I understood what do you suggest to remove from IR (IR code would be the same with or without explicit return statements in C code).
> 
I meant update the IR to match the changed text - but really all it's going to do is change the DebugLoc of the ret IR instruction, which isn't a big deal. No worries.


================
Comment at: test/DebugInfo/X86/debug_addr.ll:24
+; DWARF4: .debug_addr contents:
+; DWARF4-NEXT: 0x00000000: Addr Section params: length = 0x00000000, version = 0x0004, addr_size = 0x04, seg_size = 0x00
+; DWARF4-NEXT: Addrs: [
----------------
vleschuk wrote:
> vleschuk wrote:
> > dblaikie wrote:
> > > There's no header in debug_addr sections in DWARFv4, is there - what's this then?
> > > 
> > > Also is this consistent with the way other section contributions are described (I don't recall text like "Addr Section params" in other output - like the way those attributes are printed out like "Compile Unit: ..." rather than "Unit Section params")?
> > Changed it in dependency revision: https://reviews.llvm.org/D49676.
> > There's no header in debug_addr sections in DWARFv4, is there - what's this then?
> 
> You are correct, there is no header, these values are taken from CU. In code they still are placed into Header structure and I think it is useful to dump this structure into output.
As for the name/header - maybe take a look at all the other sections to see about something consistent. For example it looks like other header printing (debug_info/types, pubnames/types, etc) doesn't use a comma between values. But otherwise looks like what you're showing is modeled fairly closely off the printing of compile units? Seems OK if you could unify them a bit more (skip the commas, for example).

I'd still skip the header parameters in the case where they are not present in the binary format - to make the dump more accurately communicate what data is being rendered/where it's coming from.

& I'd probably skip the "Addr: [" wrapper - most of the other dumping dumps the contents without a container/braces/etc, I think? (sometimes with a header for columns if there are multiple columns)


https://reviews.llvm.org/D50005