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

[grimar]

Summary

I've splitted this from D64401.
It does not make sence to stop dumping the object if the broken
dynamic section was found. In this patch I changed the behavior from
"report an error" to "report a warning". This matches GNU.

[grimar]

• Improve one of the test cases a bit.

[MaskRay]

tools/llvm-readobj/ELFDumper.cpp

139 ↗

(On Diff #208903)

Include sh_entsize in the warning?

[grimar]

• Addressed review comment.

[jhenderson]

How about a test case that shows that more stuff can be printed after the aborted dynamic printing, perhaps using --all? Also, is there a risk that this will print the same warning more than once for the same section, if multiple options are used?

[grimar]

In D64472#1577996, @jhenderson wrote:

How about a test case that shows that more stuff can be printed after the aborted dynamic printing, perhaps using --all?

Working on that.

Also, is there a risk that this will print the same warning more than once for the same section, if multiple options are used?

Yes, it might do that. It can be fixed easily, for example DynRegionInfo can have a flag saying that region is not valid and so
we then can report a warning only once and set this flag. Though I am not sure this is a problem to report this warning in each place we were
unable to dump something too. I would not expect to see too many places and it makes the output very clear. I.e. it is visible
that there was no information dumped because of a particular error and not something else.

[grimar]

• Addressed review comments.
• Added flush calls to reportWarning

[jhenderson]

test/tools/llvm-readobj/elf-dynamic-malformed.test

16 ↗

(On Diff #209130)

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.

127 ↗

(On Diff #209130)

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.

tools/llvm-readobj/llvm-readobj.cpp

383 ↗

(On Diff #209130)

I'm not sure this flush is needed, since errs() is unbuffered.

[grimar]

• Addressed review comments.

test/tools/llvm-readobj/elf-dynamic-malformed.test

127 ↗

(On Diff #209130)

Yes, it is because of that. Added a comment.

tools/llvm-readobj/llvm-readobj.cpp

383 ↗

(On Diff #209130)

You're right. I forgot or didn't know that.

[jhenderson]

LGTM, with one suggestion.

test/tools/llvm-readobj/elf-dynamic-malformed.test

12 ↗

(On Diff #209170)

I'd be inclined to add WARN-NEXT: File: ... here to show the context of this warning.

llvm/trunk/test/Object/invalid.test

FileHeader:
   Machine: EM_X86_64
 Sections:
   - Name: .symtab
     Type: SHT_SYMTAB
     EntSize: 32
 Symbols:
   - Name: foo
 
-## Check that llvm-readobj reports an error if .dynsym has an invalid sh_entsize.
+## Check that llvm-readobj reports a warning if .dynsym has an invalid sh_entsize.
 
 # RUN: yaml2obj %s --docnum=7 -o %t7
-# RUN: not llvm-readobj --dyn-symbols %t7 2>&1 | FileCheck --check-prefix=INVALID-DYNSYM-SIZE %s
+# RUN: llvm-readobj --dyn-symbols %t7 2>&1 | FileCheck --check-prefix=INVALID-DYNSYM-SIZE %s
 
-# INVALID-DYNSYM-SIZE: error: Invalid entity size
+# INVALID-DYNSYM-SIZE: warning: invalid section size (48) or entity size (32)
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_DYN
   Machine: EM_X86_64
 Sections:
Entries:
       - Tag:   DT_NULL
         Value: 0x0
 ProgramHeaders:
   - Type: PT_LOAD
     VAddr: 0x1000
     Sections:
       - Section: .dynamic
 
-## Check that llvm-readobj reports an error when a dynamic relocation section
+## Check that llvm-readobj reports a warning when a dynamic relocation section
 ## has sh_entsize field with size != sizeof(Elf_Rela).
 
-# RUN: not llvm-readobj --dyn-relocations \
+# RUN: llvm-readobj --dyn-relocations \
 # RUN:   %p/Inputs/corrupt-invalid-relocation-size.elf.x86-64 2>&1 \
 # RUN:    | FileCheck --check-prefix=RELOC-BROKEN-ENTSIZE %s
 
-# RELOC-BROKEN-ENTSIZE: error: Invalid entity size
+# RELOC-BROKEN-ENTSIZE: warning: invalid section size (24) or entity size (25)
 
-## Check that llvm-readobj reports an error when .dynamic section has an invalid
+## Check that llvm-readobj reports a warning when .dynamic section has an invalid
 ## size, which isn't a multiple of the dynamic entry size. 
 
 # RUN: yaml2obj %s --docnum=22 -o %t22
-# RUN: not llvm-readobj --dyn-relocations %t22 2>&1 | FileCheck --check-prefix=DYN-TABLE-SIZE %s
+# RUN: llvm-readobj --dyn-relocations %t22 2>&1 | FileCheck --check-prefix=DYN-TABLE-SIZE %s
 
-# DYN-TABLE-SIZE: error: Invalid entity size
+# DYN-TABLE-SIZE: warning: invalid section size (1) or entity size (16)
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_EXEC
   Machine: EM_X86_64
 Sections:

llvm/trunk/test/tools/llvm-readobj/elf-dynamic-malformed.test

-# Test handling of a dynamic section size which is not a multiple of its entry size.
-# RUN: yaml2obj %s --docnum=1 -o %t.bad-size
-# RUN: not llvm-readobj --dynamic-table %t.bad-size 2>&1 | FileCheck %s --check-prefix ERR-SIZE
-# RUN: not llvm-readelf --dynamic-table %t.bad-size 2>&1 | FileCheck %s --check-prefix ERR-SIZE
+## Test handling of a dynamic section size which is not a multiple of its entry size.
+## Test the full output to demonstrate how we print the warnings.
 
-# ERR-SIZE: error: Invalid entity size
+# RUN: yaml2obj %s --docnum=1 -o %t.bad-size
+# RUN: llvm-readobj --all %t.bad-size 2>&1 \
+# RUN:   | FileCheck %s --implicit-check-not=warning --check-prefix WARN
+# RUN: llvm-readelf --all %t.bad-size 2>&1 \
+# RUN:   | FileCheck %s --implicit-check-not=warning --check-prefix WARN-GNU
+
+# WARN-NOT: warning
+# WARN: warning: invalid section size (4) or entity size (16)
+# WARN-EMPTY:
+# WARN-NEXT: File:
+# WARN:      Symbols [
+# WARN:      ]
+# WARN-EMPTY:
+## A warning is printed at the place where a normal dynamic table should be. 
+# WARN-NEXT: warning: invalid section size (4) or entity size (16)
+# WARN-NEXT: ProgramHeaders [
+
+# WARN-GNU-NOT:  warning
+# WARN-GNU:      warning: invalid section size (4) or entity size (16)
+# WARN-GNU-NEXT: ELF Header:
+# WARN-GNU:      Symbol table '.symtab' contains 1 entries:
+# WARN-GNU:        0:
+# WARN-GNU-EMPTY:
+## A warning is printed at the place where a normal dynamic table should be. 
+# WARN-GNU:      warning: invalid section size (4) or entity size (16)
+# WARN-GNU-EMPTY:
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_EXEC
   Machine: EM_X86_64
 Sections:
   - Name:    .dynamic
     Type:    SHT_DYNAMIC
     Address: 0x1000
     Content: "01234567"
 ProgramHeaders:
   - Type: PT_LOAD
     VAddr: 0x1000
     Sections:
       - Section: .dynamic
   - Type: PT_DYNAMIC
     VAddr: 0x1000
     Sections:
       - Section: .dynamic
 
-# Test handling of a .dynamic section with an invalid entsize (i.e. not 2 * sizeof(Elf_Dyn)).
+## Test handling of a .dynamic section with an invalid entsize (i.e. not 2 * sizeof(Elf_Dyn)).
 # RUN: yaml2obj %s --docnum=2 -o %t.bad-entsize
 # RUN: llvm-readobj --dynamic-table %t.bad-entsize | FileCheck %s --check-prefix BAD-ENTSIZE-LLVM
 # RUN: llvm-readelf --dynamic-table %t.bad-entsize | FileCheck %s --check-prefix BAD-ENTSIZE-GNU
 
 # BAD-ENTSIZE-LLVM:      DynamicSection [ (2 entries)
 # BAD-ENTSIZE-LLVM-NEXT:   Tag                Type                 Name/Value
 # BAD-ENTSIZE-LLVM-NEXT:   0x0000000000000015 DEBUG                0x0
 # BAD-ENTSIZE-LLVM-NEXT:   0x0000000000000000 NULL                 0x0
- Type: PT_LOAD
     VAddr: 0x1000
     Sections:
       - Section: .dynamic
   - Type: PT_DYNAMIC
     VAddr: 0x1000
     Sections:
       - Section: .dynamic
 
-# Test handling of string references pointing past the end of the dynamic string table.
+## Test handling of string references pointing past the end of the dynamic string table.
 # RUN: yaml2obj %s --docnum=3 -o %t.bad-string
 # RUN: llvm-readobj --dynamic-table %t.bad-string | FileCheck %s --check-prefix BAD-STRING-LLVM
 # RUN: llvm-readelf --dynamic-table %t.bad-string | FileCheck %s --check-prefix BAD-STRING-GNU
 
 # BAD-STRING-LLVM: 0x000000000000000A STRSZ         1 (bytes)
 # BAD-STRING-LLVM: 0x0000000000000001 NEEDED        Shared library: <Invalid offset 0x1>
 # BAD-STRING-LLVM: 0x000000007FFFFFFF FILTER        Filter library: <Invalid offset 0x1>
 # BAD-STRING-LLVM: 0x000000007FFFFFFD AUXILIARY     Auxiliary library: <Invalid offset 0x1>
- Type: PT_LOAD
     Sections:
       - Section: .dynstr
       - Section: .dynamic
   - Type: PT_DYNAMIC
     VAddr: 0x1010
     Sections:
       - Section: .dynamic
 
-# Test handling of DT_STRTAB pointing outside the file's address space.
+## Test handling of DT_STRTAB pointing outside the file's address space.
 # RUN: yaml2obj %s --docnum=4 -o %t.bad-strtab
 
 # RUN: llvm-readobj --dynamic-table %t.bad-strtab 2>&1 >/dev/null | FileCheck %s --check-prefix BAD-STRTAB-ERR
 # RUN: llvm-readelf --dynamic-table %t.bad-strtab 2>&1 >/dev/null | FileCheck %s --check-prefix BAD-STRTAB-ERR
 # BAD-STRTAB-ERR: warning: Unable to parse DT_STRTAB: virtual address is not in any segment: 0x2000000
 
 # RUN: llvm-readobj --dynamic-table --needed-libs %t.bad-strtab | FileCheck %s --check-prefixes=BAD-STRTAB,BAD-STRTAB-LLVM
 # RUN: llvm-readelf --dynamic-table --needed-libs %t.bad-strtab | FileCheck %s --check-prefixes=BAD-STRTAB,BAD-STRTAB-GNU
- Type: PT_LOAD
     VAddr: 0x1000
     Sections:
       - Section: .dynamic
   - Type: PT_DYNAMIC
     VAddr: 0x1000
     Sections:
       - Section: .dynamic
 
-# Test handling of other d_ptr tags pointing outside the file's address space.
+## Test handling of other d_ptr tags pointing outside the file's address space.
 # RUN: yaml2obj %s --docnum=5 -o %t.bad-rela
 # RUN: llvm-readobj --dynamic-table %t.bad-rela 2>&1 | FileCheck %s --check-prefixes=CHECK,BAD-RELA
 # RUN: llvm-readelf --dynamic-table %t.bad-rela 2>&1 | FileCheck %s --check-prefixes=CHECK,BAD-RELA-GNU
 
 # CHECK: warning: Unable to parse DT_RELA: virtual address is not in any segment: 0x1000000
 
 # BAD-RELA:      DynamicSection [ (2 entries)
 # BAD-RELA-NEXT:   Tag                Type Name/Value

llvm/trunk/tools/llvm-readobj/ELFDumper.cpp

struct DynRegionInfo {
   uint64_t Size = 0;
   /// Size of each entity in the region.
   uint64_t EntSize = 0;
 
   template <typename Type> ArrayRef<Type> getAsArrayRef() const {
     const Type *Start = reinterpret_cast<const Type *>(Addr);
     if (!Start)
       return {Start, Start};
-    if (EntSize != sizeof(Type) || Size % EntSize)
-      reportError("Invalid entity size");
+    if (EntSize != sizeof(Type) || Size % EntSize) {
+      // TODO: Add a section index to this warning.
+      reportWarning("invalid section size (" + Twine(Size) +
+                    ") or entity size (" + Twine(EntSize) + ")");
+      return {Start, Start};
+    }
     return {Start, Start + (Size / EntSize)};
   }
 };
 
 template <typename ELFT> class ELFDumper : public ObjDumper {
 public:
   ELFDumper(const object::ELFObjectFile<ELFT> *ObjF, ScopedPrinter &Writer);
 

llvm/trunk/tools/llvm-readobj/llvm-readobj.cpp

 
 LLVM_ATTRIBUTE_NORETURN void reportError(Twine Msg) {
   errs() << "\n";
   WithColor::error(errs()) << Msg << "\n";
   exit(1);
 }
 
 void reportWarning(Twine Msg) {
+  fouts().flush();
   errs() << "\n";
   WithColor::warning(errs()) << Msg << "\n";
 }
 
 void warn(Error Err) {
   handleAllErrors(std::move(Err), [&](const ErrorInfoBase &EI) {
     reportWarning(EI.message());
   });