[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)) {
----------------
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`.


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

https://reviews.llvm.org/D78500