[llvm-objdump] --private-headers: change errors to warnings for dynamic section dumping

[MaskRay]

Summary

Fix #54456: objcopy --only-keep-debug produces a linked image with invalid
empty dynamic section. llvm-objdump -p currently reports an error which seems
excessive.

% llvm-readelf -l a.out
llvm-readelf: warning: 'a.out': no valid dynamic table was found
...

Follow the spirit of llvm-readelf -l (D64472) and report a warning instead.
This allows later files to be dumped despite warnings for an input file.
This improves objdump compatibility in that the exit code is now 0 instead of 1.

% llvm-objdump -p a.out  # new behavior
...
Program Header:
llvm-objdump: warning: 'a.out': invalid empty dynamic section
% objdump -p a.out
...
Dynamic Section:

[raj.khem]

I built this on top if 14.x branch and it fixes the problems, I was seeing and the warning is still emitted as expected.

[jhenderson]

Mostly looks good. Some small improvements to the testing suggested.

llvm/test/tools/llvm-objdump/ELF/dynamic-malformed.test

2–4

Generally, I think dumper tools should avoid emitting errors for individual files, because it prevents later files from being dumped. @grimar and I both in the past made significant improvements to llvm-readobj and other tools (in my case llvm-dwarfdump at least) to achieve this. I don't think the same attention has been applied to llvm-objdump.

I've therefore suggested a rewording of the comment.

llvm/test/tools/llvm-objdump/ELF/invalid-phdr.test

8–9

Might be worth a CHECK-EMPTY after this line now, to show nothing is actually printed.

llvm/test/tools/llvm-objdump/ELF/program-headers.test

1

Ditto.

1

Ditto.

[MaskRay]

Address comments

[jhenderson]

LGTM.

llvm/lib/Object/ELF.cpp

for (const Elf_Shdr &Sec : *SectionsOrError) {
       }
     }
 
     if (!Dyn.data())
       return ArrayRef<Elf_Dyn>();
   }
 
   if (Dyn.empty())
-    // TODO: this error is untested.
     return createError("invalid empty dynamic section");
 
   if (Dyn.back().d_tag != ELF::DT_NULL)
-    // TODO: this error is untested.
     return createError("dynamic sections must be DT_NULL terminated");
 
   return Dyn;
 }
 
 template <class ELFT>
 Expected<const uint8_t *>
 ELFFile<ELFT>::toMappedAddr(uint64_t VAddr, WarningHandler WarnHandler) const {

llvm/test/tools/llvm-objdump/ELF/dynamic-malformed.test

+## An empty dynamic section is invalid. Test we report a warning instead of an
+## error, so that dumping can continue with other objects.
+# RUN: yaml2obj %s --docnum=1 -o %t.empty
+# RUN: llvm-objdump -p %t.empty 2>&1 | FileCheck %s -DFILE=%t.empty --check-prefix=EMPTY
+
+# EMPTY:       Program Header:
+# EMPTY-NEXT:  warning: '[[FILE]]': invalid empty dynamic section
+# EMPTY-EMPTY:
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_X86_64
+Sections:
+  - Name: .dynamic
+    Type: SHT_DYNAMIC
+
+# RUN: yaml2obj %s --docnum=2 -o %t.nonull
+# RUN: llvm-objdump -p %t.nonull 2>&1 | FileCheck %s -DFILE=%t.nonull --check-prefix=NONULL
+
+# NONULL:       Program Header:
+# NONULL-NEXT:  warning: '[[FILE]]': dynamic sections must be DT_NULL terminated
+# NONULL-EMPTY:
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_X86_64
+Sections:
+  - Name: .dynamic
+    Type: SHT_DYNAMIC
+    Entries:
+      - Tag:   DT_SONAME
+        Value: 1

llvm/test/tools/llvm-objdump/ELF/invalid-phdr.test

 ## Test how we handle the case when the e_phoff field is invalid.
 # RUN: yaml2obj %s -o %t
-# RUN: not llvm-objdump --private-headers %t 2>&1 | \
+# RUN: llvm-objdump --private-headers %t 2>&1 | \
 # RUN:   FileCheck -DFILE=%t %s --check-prefix=INVALID-PHOFF
 
 # INVALID-PHOFF:      Program Header:
 # INVALID-PHOFF-NEXT: warning: '[[FILE]]': unable to read program headers: program headers are longer than binary of size 280: e_phoff = 0xffffff, e_phnum = 0, e_phentsize = 0
-# INVALID-PHOFF-NEXT: error: '[[FILE]]': program headers are longer than binary of size 280: e_phoff = 0xffffff, e_phnum = 0, e_phentsize = 0
+# INVALID-PHOFF-NEXT: warning: '[[FILE]]': program headers are longer than binary of size 280: e_phoff = 0xffffff, e_phnum = 0, e_phentsize = 0
+# INVALID-PHOFF-EMPTY:
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_EXEC
   Machine: EM_X86_64
   EPhOff:  0xffffff

llvm/test/tools/llvm-objdump/ELF/program-headers.test

 ## Check that program headers are output correctly
 
 # RUN: yaml2obj --docnum=1 -DBITS=32 -DMACHINE=EM_386 %s -o %t32.elf
 # RUN: llvm-objdump --private-headers %t32.elf | FileCheck %s --check-prefixes=ELF32 
 
 # ELF32:      Program Header:
 # ELF32-NEXT:    PHDR off    0x00000314 vaddr 0x00001000 paddr 0x00001000 align 2**0
 # ELF32-NEXT:         filesz 0x00000003 memsz 0x00000003 flags -w-
 # ELF32-NEXT:    PHDR off    0x00000317 vaddr 0x00002000 paddr 0x00002000 align 2**0
## Case 22: the PT_HIPROC segment.
   - Type:     0x7fffffff ## PT_HIPROC
     VAddr:    0x1000
     FirstSec: .foo.begin
     LastSec: .foo.end
 
 ## Check we report an error / warning when we are unable to read program headers.
 ## Case A: the e_phentsize field is invalid.
 # RUN: yaml2obj --docnum=2 -DPHENTSIZE=1 %s -o %t.phdr.err
-# RUN: not llvm-objdump --private-headers %t.phdr.err 2>&1 | \
+# RUN: llvm-objdump --private-headers %t.phdr.err 2>&1 | \
 # RUN:   FileCheck %s -DFILE=%t.phdr.err --check-prefix=PHENTSIZE
 
 # PHENTSIZE:      Program Header:
 # PHENTSIZE-NEXT: warning: '[[FILE]]': unable to read program headers: invalid e_phentsize: 1
-# PHENTSIZE-NEXT: error: '[[FILE]]': invalid e_phentsize: 1
+# PHENTSIZE-NEXT: warning: '[[FILE]]': invalid e_phentsize: 1
+# PHENTSIZE-EMPTY:
 
 --- !ELF
 FileHeader:
   Class:      ELFCLASS64
   Data:       ELFDATA2LSB
   Type:       ET_EXEC
   EPhEntSize: [[PHENTSIZE=<none>]]
   EPhOff:     [[PHOFF=<none>]]

 
 ## Check that we do not report a warning when the program header table ends right before the end of the file.
 ## 0x160 + size of headers (56) == file size.
 # RUN: yaml2obj --docnum=2 -DPHOFF=0x160 %s -o %t.phdr.no.err2
 # RUN: llvm-objdump %t.phdr.no.err2 --private-headers 2>&1 | FileCheck %s --implicit-check-not=warning:
 
 ## Check we report a warning / error when e_phoff goes 1 byte past the end of the file.
 # RUN: yaml2obj --docnum=2 -DPHOFF=0x161 %s -o %t.phdr.err2
-# RUN: not llvm-objdump --private-headers %t.phdr.err2 2>&1 | \
+# RUN: llvm-objdump --private-headers %t.phdr.err2 2>&1 | \
 # RUN:   FileCheck %s -DFILE=%t.phdr.err2 --check-prefix=PHOFF -DOFF=0x161
 
 # PHOFF:      Program Header:
 # PHOFF-NEXT: warning: '[[FILE]]': unable to read program headers: program headers are longer than binary of size 408: e_phoff = [[OFF]], e_phnum = 1, e_phentsize = 56
-# PHOFF-NEXT: error: '[[FILE]]': program headers are longer than binary of size 408: e_phoff = [[OFF]], e_phnum = 1, e_phentsize = 56
-
+# PHOFF-NEXT: warning: '[[FILE]]': program headers are longer than binary of size 408: e_phoff = [[OFF]], e_phnum = 1, e_phentsize = 56
+# PHOFF-EMPTY:
 
 ## Check we report a warning / error when the value of e_phoff is so large that
 ## e_phoff + e_phnum * e_phentsize > UINT64_MAX.
 # RUN: yaml2obj --docnum=2 -DPHOFF=0xffffffffffffffff %s -o %t.phdr.err3
-# RUN: not llvm-objdump --private-headers %t.phdr.err3 2>&1 | \
+# RUN: llvm-objdump --private-headers %t.phdr.err3 2>&1 | \
 # RUN:   FileCheck %s -DFILE=%t.phdr.err3 --check-prefix=PHOFF -DOFF=0xffffffffffffffff

llvm/tools/llvm-objdump/ELFDump.cpp

uint64_t objdump::getELFSectionLMA(const object::ELFSectionRef &Sec) {
   else if (const auto *ELFObj = dyn_cast<ELF64LEObjectFile>(Sec.getObject()))
     return getSectionLMA(ELFObj->getELFFile(), Sec);
   const auto *ELFObj = cast<ELF64BEObjectFile>(Sec.getObject());
   return getSectionLMA(ELFObj->getELFFile(), Sec);
 }
 
 template <class ELFT>
 static void printDynamicSection(const ELFFile<ELFT> &Elf, StringRef Filename) {
-  ArrayRef<typename ELFT::Dyn> DynamicEntries =
-      unwrapOrError(Elf.dynamicEntries(), Filename);
+  auto DynamicEntriesOrErr = Elf.dynamicEntries();
+  if (!DynamicEntriesOrErr) {
+    reportWarning(toString(DynamicEntriesOrErr.takeError()), Filename);
+    return;
+  }
+  ArrayRef<typename ELFT::Dyn> DynamicEntries = *DynamicEntriesOrErr;
 
   // Find the maximum tag name length to format the value column properly.
   size_t MaxLen = 0;
   for (const typename ELFT::Dyn &Dyn : DynamicEntries)
     MaxLen = std::max(MaxLen, Elf.getDynamicTagAsString(Dyn.d_tag).size());
   std::string TagFmt = "  %-" + std::to_string(MaxLen) + "s ";
 
   outs() << "\nDynamic Section:\n";