[PATCH] D74425: [DebugInfo]: Added support for DWARFv5 Info section header parsing in llvm-dwp utility.

[Sourabh Singh Tomar via Phabricator via llvm-commits]

SouraVX marked an inline comment as done.
SouraVX added inline comments.


================
Comment at: llvm/tools/llvm-dwp/llvm-dwp.cpp:159-162
   uint8_t AddrSize = InfoData.getU8(&Offset);
-
+  if (Version >= 5)
+    Signature = InfoData.getU64(&Offset);
   uint32_t AbbrCode = InfoData.getULEB128(&Offset);
----------------
dblaikie wrote:
> dblaikie wrote:
> > DWARFv5 reorders the abbreviation offset and the address size - so this code is /probably/ wrong/needs support for this part of DWARFv5. But that could wait for another patch (might be worth checking how the failure to parse that correctly is manifesting - if there's a good spot to demonstrate it in the test case by adding an extra CHECK line and a FIXME saying "hey, this is broken here/fix it soon" would be good)
> > DWARFv5 reorders the abbreviation offset and the address size - so this code is /probably/ wrong/needs support for this part of DWARFv5. But that could wait for another patch (might be worth checking how the failure to parse that correctly is manifesting - if there's a good spot to demonstrate it in the test case by adding an extra CHECK line and a FIXME saying "hey, this is broken here/fix it soon" would be good)
> Any thoughts on this ^ ?
Yes, I noticed this thing in Spec couple of times. But since llvm-dwarfdump is also not detecting while parsing, I took  it as an expected/planned behavior.


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

https://reviews.llvm.org/D74425