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

[Sourabh Singh Tomar via Phabricator via llvm-commits]

SouraVX added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugMacro.cpp:154
       StrOffset = Data.getRelocatedValue(/*OffsetSize=*/4, &Offset);
-      E.MacroStr = StringExtractor.getCStr(&StrOffset);
+      assert(StringExtractor && "String Extractor not found");
+      E.MacroStr = StringExtractor->getCStr(&StrOffset);
----------------
ikudrin wrote:
> SouraVX wrote:
> > ikudrin wrote:
> > > `assert` should not be used for the cases when malicious user input may trigger it.
> > This `String Extractor` is innocuous in a sense, it is passed as parameters by functions up in the stack. So I left this ``assert` as it is. ? Are you okay with this.
> > Other asserts, I've replaced with error handling.
> What if a malformed `.debug_macinfo` section contains a `DW_MACRO_define_strp` entry? Does the code have any protection against that?
Ah, thanks for noting that. I think we should consider `assert` for these cases `DW_MACRO_define_strp` and `DW_MACRO_define_strx` since these forms belongs and should only be present when section is `debug_macro`. + Test cases to test these cases.
Does this sounds fair to you ? 


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

https://reviews.llvm.org/D78736