[PATCH] D102312: [llvm-dwp] Skip type unit debug info sections

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

Maybe a bunch of the complexity in this patch comes from supporting multiple .debug_info.dwo sections in a .dwo file - could you check if that's needed? Could we fix that issue in LLVM instead of supporting it in llvm-dwp.



================
Comment at: llvm/tools/llvm-dwp/llvm-dwp.cpp:156-159
+      // Type offset.
+      InfoData.getU32(&Offset, &Err);
+      if (Err)
+        return make_error<DWPError>("Type unit is missing type offset.");
----------------
Why different error handling here than for the rest of the header field parsing?

I'd have thought the MinHeaderLength check above would be adequate/intended to mean that all the rest of the header fields could be parsed without error checking?

Ah, but you don't know the length anymore at that point - since you have to wait until you've read the unit kind. OK.

But this error checking would be incomplete - since it's not the offset field that would fail to parse - it's the addrSize later on where we'd run out of bytes to read and fail there instead of here. 

Test coverage would probably demonstrate this/help motivate changes to make this error checking correct.


================
Comment at: llvm/tools/llvm-dwp/llvm-dwp.cpp:603-604
     CurTUIndexSection = Contents;
+  else if (OutSection == InfoSection)
+    CurInfoSection.push_back(Contents);
   else {
----------------
The need to keep multiple info sections on input I think could reasonably be consider to be a bug in LLVM's output. At least as far as I'm concerned - we should produce a single .debug_info section with all the type and compile units in it. I don't think it's worth supporting multiple .debug_info sections in llvm-dwp - we should fix LLVM instead, if possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102312