[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

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

14

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.

125

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

125

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

14

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

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 entity size
 
 --- !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 entity size
 
-## 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 entity size
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_EXEC
   Machine: EM_X86_64
 Sections:

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
+# RUN: llvm-readobj --dynamic-table %t.bad-size 2>&1 | FileCheck %s --check-prefix WARN
+# RUN: llvm-readelf --dynamic-table %t.bad-size 2>&1 | FileCheck %s --check-prefix WARN
 
-# ERR-SIZE: error: Invalid entity size
+# WARN: warning: Invalid entity size
 
 --- !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:
Entries:
         Value: 1
       - Tag:   DT_USED
         Value: 1
       - Tag:   DT_SONAME
         Value: 1
       - Tag:   DT_RPATH
         Value: 1
       - Tag:   DT_RUNPATH
         Value: 1
       - Tag:   DT_NULL
         Value: 0
 ProgramHeaders:
   - Type: PT_LOAD
     VAddr: 0x1000
     Sections:
       - Section: .dynstr
       - Section: .dynamic

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: This warning needs to be more precise.
+      reportWarning("Invalid entity size");
+      return {Start, Start};
+    }
     return {Start, Start + (Size / EntSize)};
   }
 };
 
 template <typename ELFT> class ELFDumper : public ObjDumper {
 public:
   ELFDumper(const object::ELFObjectFile<ELFT> *ObjF, ScopedPrinter &Writer);