[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/DWARFDebugMacro.cpp:154-155
+
+      // FIXME: Add support for DWARF64.
+      int StrOffsetBases = 8;
+
----------------
Is there an API you could call to provide this answer, rather than hardcoding it? (like a MacroHeader structure that tells you the size based on DWARF32/64 enum - you could hardcode passing the DWARF32 enum value to such an API for now, but at least calling the API rather than hardcoding it would be good)


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugMacro.cpp:163
+      uint64_t IndexOffset = StrOffsetBases + OffsetIndex * /*EntrySize=*/4;
+      uint64_t StrOffset = StringOffsetsExtractor.getU32(&IndexOffset);
+      E.MacroStr = StringExtractor.getCStr(&StrOffset);
----------------
Does the StringOffsetsExtractor already know which contribution it's reading from (if you have two objects linked together, with two string offsets contributions, would this code correctly read the desired one?)

Speaking of: these forms (_strx) aren't unique to debug_macro.dwo, they can be used in debug_macro - perhaps it'd be good to separate adding support for these new forms & testing them in debug_macro (non-dwo) support, then, separately, adding support for debug_macro.dwo?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78500