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

[Sourabh Singh Tomar via Phabricator via llvm-commits]

SouraVX marked 3 inline comments as done.
SouraVX added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugMacro.cpp:166-171
+      uint64_t StrOffsetBase = GetStrOffsetBase();
+      E.Line = Data.getULEB128(&Offset);
+      uint64_t OffsetIndex = Data.getULEB128(&Offset);
+      uint64_t IndexOffset = StrOffsetBase + OffsetIndex * /*EntrySize=*/4;
+      uint64_t StrOffset = StringOffsetsExtractor.getValue().getRelocatedValue(
+          /*OffsetSize=*/4, &IndexOffset);
----------------
ikudrin wrote:
> `DWARFUnit` provides a method for that, `getStringOffsetSectionItem()`. Please, use it.
> 
> By the way, the string extractor can also be obtained from `DWARFUnit` using the `getStringExtractor()` method.
> By the way, the string extractor can also be obtained from DWARFUnit using the getStringExtractor() method.

There's small caveat here, consider a case of `_strp` forms. For this case we don't need any CU information to parse/dump the associated macro content.
So we if try this way  --
```
E.MacroStr = GetContributionUnit()->getStringExtractor().getCStr(&StrOffset);
```
Then cases which doesn't have CU in first place will end up crashing the dwarfdump.
I tried this approach and ended with crashes for these test cases, as you may notice these test cases only contains the minimum needed information to dump `_strp` forms which is `debug_str` and `debug_macro` section --
```
Failing Tests (2):
  LLVM :: DebugInfo/X86/debug-macro-macinfo.s
  LLVM :: DebugInfo/X86/debug-macro-v5.s
```
Shall we continue with earlier approach(pass the String Extractor explicitly) just for the cases of `_strp` form ? 


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

https://reviews.llvm.org/D78736