[PATCH] D64472: [llvm-readobj/llvm-readelf] - Report a warning instead of a error when dumping a broken dynamic section.

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 11 01:48:00 PDT 2019


jhenderson added inline comments.


================
Comment at: test/tools/llvm-readobj/elf-dynamic-malformed.test:16
+# WARN-NEXT: LoadName: <Not found>
+# WARN-NEXT: ElfHeader {
+# WARN-NEXT:   Ident {
----------------
I think you've gone a bit over the top with this test case now. I think it's sufficient to test the context either side of each warning, and add --implicit-check-not=warning to ensure that you've caught them all. Something like:
```
WARN: warning: invalid section size (4) or entity size (16)
WARN-EMPTY:
WARN-NEXT: File:
WARN: Symbols [
WARN: ]
WARN-EMPTY:
WARN-NEXT: warning: invalid section size (4) or entity size (16)
WARN-NEXT: ProgramHeaders
```
You can follow a similar process with the GNU output.


================
Comment at: test/tools/llvm-readobj/elf-dynamic-malformed.test:127
+# WARN-EMPTY: 
+# WARN-NEXT: warning: invalid section size (4) or entity size (16)
+# WARN-NEXT: ProgramHeaders [
----------------
Why is there a warning here? I think it's where the dynamic table should be printed, in which case, please add a comment stating as such.


================
Comment at: tools/llvm-readobj/llvm-readobj.cpp:383
   WithColor::warning(errs()) << Msg << "\n";
+  errs().flush();
 }
----------------
I'm not sure this flush is needed, since errs() is unbuffered.


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

https://reviews.llvm.org/D64472





More information about the llvm-commits mailing list