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

[Jan Kratochvil via Phabricator via llvm-commits]

jankratochvil marked 7 inline comments as done.
jankratochvil added a comment.

To satisfy all the required detailed reporting I had to extend the patch far more than I originally inteded. Is it OK this way?



================
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!
----------------
dblaikie wrote:
> 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?
That `DIE.extractFast` just exited on both errors and successful end of DIEs. As it did not report any errors the return code (`false`) was the same. So this one specific error was handled outside. But now when we do all the detailed error reporting we need to do it from inside `DIE.extractFast` as that has all the info available. Therefore this error has moved inside `DIE.extractFast` (`if (Offset >= UEndOffset)` there).


================
Comment at: llvm/test/tools/llvm-dwarfdump/X86/format-warnings.s:3
+# RUN:   | llvm-dwarfdump - 2>&1 | FileCheck --check-prefix=CHECK-CUEND %s
+# CHECK-CUEND: warning: DWARF compile unit extends beyond its bounds cu 0x00000000 at 0x0000001f
+
----------------
jhenderson wrote:
> It would probably be a good idea if you used a non-zero value for the cu index. That will help flag up any conversion issues (e.g. because you used a 32-bit print format for a 64-bit number).
> 
> Same goes for the remaining messages.
> 
> Here and below, you can drop the "CHECK-" bit of the check prefixes, to make them more concise.
> It would probably be a good idea if you used a non-zero value for the cu index.

Done.

> That will help flag up any conversion issues (e.g. because you used a 32-bit print format for a 64-bit number).

That would not show anything more as on 64-bit platforms variadic function extends all parameters to (at least) 64 bits. Therefore even 32-bit format will still read 64-bit variadic parameter.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104271