[PATCH] D104271: llvm-dwarfdump: Print warnings on invalid DWARF

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugInfoEntry.cpp:37-39
+                          "DWARF compile unit extends beyond its "
+                          "bounds cu 0x%8.8" PRIx64 " at 0x%8.8" PRIx64,
+                          U.getOffset(), Offset));
----------------
Could probably include the bounds in the message? Maybe something like:

"DWARF compile unit extends beyond its bounds [x, y) to z"?


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugInfoEntry.cpp:52-55
+        createStringError(errc::invalid_argument,
+                          "DWARF abbreviation 0x%" PRIx64 " is not valid "
+                          "in cu 0x%8.8" PRIx64 " at 0x%8.8" PRIx64,
+                          AbbrCode, U.getOffset(), Offset));
----------------
Maybe include some details about the offset to the debug_abbrev contribution, and the range of valid abbreviation values? (I forget if the abbrev table is necessarily contiguous - if it isn't, then maybe that's too complicated)


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp:387-406
         ++Depth;
+      else if (Depth == 0)
+        break; // This unit has a single DIE with no children.
     } else {
       // NULL DIE.
       if (Depth > 0)
         --Depth;
       if (Depth == 0)
         break;  // We are done with this compile unit!
----------------
What was the motivation to move this code and add the Depth check in? I guess this didn't actually work/was untested, maybe? Could you explain why it didn't work, etc?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104271