[PATCH] D78736: [DWARF5]: Added support for dumping strx forms in llvm-dwarfdump

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFDebugMacro.h:108
+  // FIXME: We should provide a better abstraction.
+  Error parse(DWARFUnitVector::iterator_range Units,
+              DataExtractor StringExtractor, DWARFDataExtractor MacroData,
----------------
The method looks weird. It would be better to add two separate methods to parse macro and macinfo sections respectively.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugMacro.cpp:173
+      if (MacroContributionOffset == MacroToUnits.end())
+        return createStringError(errc::invalid_argument,
+                                 "Macro contribution of the unit not found");
----------------
This probably should be a recoverable error. It would be good to report the offset of the macro table.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugMacro.cpp:175
+                                 "Macro contribution of the unit not found");
+      auto ContributionUnit = MacroToUnits.find(M->Offset)->second;
+      Optional<uint64_t> StrOffset =
----------------
You've already found the record, no need to search again.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugMacro.cpp:180
+      if (!StrOffset.hasValue())
+        return createStringError(
+            errc::invalid_argument,
----------------
This also should be a recoverable error. It would be good to report the problematic offset.


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

https://reviews.llvm.org/D78736