[PATCH] D82886: [DebugInfo] Fix a possible crash when reading a malformed .debug_*lists section.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/DebugInfo/X86/dwarfdump-rnglists-format-mix.s:1
+## The test checks that llvm-dwarfdump with enabled assertions can handle
+## a malformed input file that contains debugging info sections in different
----------------
I think you can get rid of the "with enabled assertions" comment here. The test also shows that llvm-dwarfdump without assertions doesn't crash!

As this is testing the library code, rather than something to do with llvm-dwarfdump specifically, perhaps you can just drop `dwarfdump-` from the test name.


================
Comment at: llvm/test/DebugInfo/X86/dwarfdump-rnglists-format-mix.s:5-7
+# RUN: llvm-mc -triple x86_64 %s -filetype=obj -o - \
+# RUN:   | not llvm-dwarfdump -debug-info - 2>&1 \
+# RUN:   | FileCheck %s
----------------
Two thoughts, and not asking you to change anything if you prefer the current approach:

1) I'm not a fan of piping llvm-mc output into llvm-dwarfdump directly, because it makes it harder to debug the test if there's a problem, since I then have to change this line to generate the intermediate object etc.
2) I've come to the conclusion that I personally prefer having the pipe operator at the end of the previous line, since it clearly shows that the next line is new command, and that you don't need to look at it to find more arguments for the command on the first line, i.e:
```
# RUN: llvm-mc -triple x86_64 %s -filetype=obj -o - | \
## Indentation shows this is a continuation; llvm-dwarfdump shows it's a new executable being called
# RUN:   not llvm-dwarfdump -debug-info - 2>&1 | \
# RUN:   FileCheck %s
```


================
Comment at: llvm/test/DebugInfo/X86/dwarfdump-rnglists-format-mix.s:6
+# RUN: llvm-mc -triple x86_64 %s -filetype=obj -o - \
+# RUN:   | not llvm-dwarfdump -debug-info - 2>&1 \
+# RUN:   | FileCheck %s
----------------
Do you actually need to dump the debug info, or could you just dump the .debug_rnglists section explicitly? That would allow you to simplify your test input accordingly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82886