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

[Sourabh Singh Tomar via Phabricator via llvm-commits]

SouraVX added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:301-304
+            SectionType != MacroDwoSection ? getStringOffsetsExtractor()
+                                           : getStringOffsetsDWOExtractor(),
+            SectionType != MacroDwoSection ? getStringExtractor()
+                                           : getStringDWOExtractor(),
----------------
ikudrin wrote:
> As string offsets sections are only involved in parsing .debug_macro[.dwo] section, you should not pass the extractors when parsing .debug_macinfo[.dwo] sections.
Thanks, I'll mark/pass this parameter as `optional`. 


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugMacro.cpp:118
 
+    auto GetStrOffsetsBase = [&]() {
+      for (auto &CU : CUs)
----------------
ikudrin wrote:
> What if there are two or more CUs which use separate contributions to the macro section with different string offsets bases?
Not sure what you meant here, this patch provide support for this situation too. 
The test case `debug-macro-multi-cu-strx.s` contains multiple CUs(2 CU) having separated contributions to macro section with different string offsets base.



================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugMacro.cpp:131
+      // FIXME: Add support for DWARF64
+      return (uint64_t)8;
+    };
----------------
ikudrin wrote:
> You may want to support pre-v5 .debug_macro sections. They were implemented as a GNU extension in GCC 4.7. As they use the pre-standard format of .debug_str_offsets.dwo, there is no header.
> 
> See http://www.dwarfstd.org/ShowIssue.php?issue=110722.1 and https://gcc.gnu.org/wiki/DebugFission.
This should be taken separately IMO, we should first support spec.


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

https://reviews.llvm.org/D78500