[PATCH] D78500: [DWARF5]:Added support for .debug_macro.dwo section in llvm-dwarfdump

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:269
+                                                    : dwo_compile_units(),
                                                 getStringExtractor(), Data)
                             : Macro->parseMacinfo(Data)) {
----------------
dblaikie wrote:
> ikudrin wrote:
> > SouraVX wrote:
> > > ikudrin wrote:
> > > > Passing an extractor for a `.debug_str` section when we parse a `.debug_macro.dwo` does not seem correct. Please, fix and add a test with `DW_MACRO_*_strp` entries in a `.debug_macro.dwo` section.
> > > > Passing an extractor for a .debug_str section when we parse a .debug_macro.dwo does not seem correct.
> > > Yes, StringExtractor is not getting used anywhere except for parsing DW_MACRO_*_strp` form. but the current interface(`parseMacro`) does not distinguish whether we're parsing `.debug_macro` or `.debug_macro.dwo` (except the units part(that is being distiguished)).
> > > Do you think, it's a good idea to make this parameter optional ? Since it's only going to be used in parsing `.debug_macro` section.
> > > 
> > > > Please, fix and add a test with DW_MACRO_*_strp entries in a .debug_macro.dwo section.
> > > These forms are not allowed in a `DWO` object since these forms involves a direct relocation to the `debug_str` section. And in `LLVM` a relocation cannot be present in a `DWO` object.
> > > These forms are not allowed in a `DWO` object since these forms involves a direct relocation to the `debug_str` section. And in `LLVM` a relocation cannot be present in a `DWO` object.
> > 
> > There is no need for a relocation to reference a record in the `.debug_str.dwo` section; e.g. `.debug_str_offsets.dwo` references the strings somehow, right? Moreover, in figure `B.2` on page 278 you may find a link `(po)` between sections `.debug_macro.dwo` and `.debug_str.dwo`. The comment for the link is missing, but it should correspond to the similar link `(p)` in figure `B.1`.
> Oh, I was going to say that was just a spec bug, but seems to be explicitly supported:
> 
> > In a .dwo file, referring to a string using DW_FORM_strp is valid, but such use results in a file that cannot be incorporated into a package file (which involves string merging).
> 
> (DWARFv5, F.1, top of page 393)
> 
> Thanks for pointing that out, @ikudrin !
The standard definitely needs clarification on using `*_strp` forms in `dwo`/`dwp` files. A few pages after that it says:
> In a split DWARF object file, all references to strings go through this table (there
> are no other offsets to .debug_str.dwo in a split DWARF object file). That is,
> there is no use of DW_FORM_strp in a split DWARF object file.
(DWARFv5, p. 405, lines 4-6).

As it seems from now, the tool should be ready to handle `*_strp` records in `dwo` (and maybe even in `dwp`) files, because they are allowed at least in some parts of the standard.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78500/new/

https://reviews.llvm.org/D78500