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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:269
+                                                    : dwo_compile_units(),
                                                 getStringExtractor(), Data)
                             : Macro->parseMacinfo(Data)) {
----------------
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 !


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

https://reviews.llvm.org/D78500