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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/test/tools/llvm-dwp/X86/info-v5.s:25
+
+	.section	.debug_abbrev,"", at progbits
+	.byte	1                       # Abbreviation Code
----------------
Remove all the non-dwo sections (so the text section, and the debug sections that aren't dwo sections, etc).


================
Comment at: llvm/test/tools/llvm-dwp/X86/info-v5.s:69
+	.short	12                      # DW_AT_language
+        .asciz  "int.c"               # DW_AT_name
+	.asciz  "int.dwo"               # DW_AT_dwo_name
----------------
Correct indentation. (elsewhere in this too - line up the assembly directives and the comments)


================
Comment at: llvm/tools/llvm-dwp/llvm-dwp.cpp:156
+      return make_error<DWPError>(
+          "unit type DW_UT_split_compile type not found in debug_info header");
+  }
----------------
Might be worth specifying the unit type that was found? At least even if it's just the integer value "unexpected unit type 0x3 found in debug_info".

This should also be tested. (the existing test case could have a second CU in it that just has an incorrect unit type in its header)


================
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);
----------------
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)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74425