[llvm-readelf] Render messages similar to that of `GNU binutils readelf` when no sections and/or no headers.

[Prabhuk]

Summary

When there are no section headers section information printed by llvm-readelf is not useful and unnecessarily verbose. When there are no program headers there's a similar verbose output shown when section mapping is requested. Simplifying the message shown in these cases to match the behavior of GNU binuntils readelf.

[MaskRay]

LGTM.

[jhenderson]

Behaviour change is generally fine, but there are a couple of test changes/additions required.

llvm/test/tools/llvm-readobj/ELF/many-sections.s

45

Isn't this warning still expected to be emitted?

llvm/tools/llvm-readobj/ELFDumper.cpp

4085–4088

Seems like this case isn't tested?

[Prabhuk]

Added tests that will cover the printSectionDetails() function no section header use cases.
Rendering the warning e_shstrndx == SHN_XINDEX, but the section header table is empty along with the There is no sections in the file. message.

[Prabhuk]

llvm/test/tools/llvm-readobj/ELF/many-sections.s

45

Rendering the warning in this case and bringing back the warning message test to the test file.

llvm/tools/llvm-readobj/ELFDumper.cpp

4085–4088

Thank you. Added tests to cover this path.

[jhenderson]

llvm/test/tools/llvm-readobj/ELF/gnu-section-mapping-no-phdrs.test

3–6 ↗

(On Diff #448669)

As we're changing this test anyway, I think it would be good to show that no other output is printed before or after, not just after the checked line. The easiest way to do this is to add --implicit-check-not={{.}} to the FileCheck command and then just get rid of the existing CHECK-NOT line.

llvm/test/tools/llvm-readobj/ELF/many-sections.s

11

I'm not sure I follow what coverage this line is adding? We need a test case with --section-details but no sections, but you've added this to no-shdrs.test.

llvm/test/tools/llvm-readobj/ELF/no-phdrs.test

15

I think we need to show that the section segment mapping is not printed in this case. I'm thinking something like GNU-NOT: {{.}} or better yet, --implicit-check-not={{.}} should do the trick.

llvm/tools/llvm-readobj/ELFDumper.cpp

3697

3701–3702

We need a test case that shows this warning being emitted.

4085

4089–4090

Ditto: this warning needs checking, I believe.

4275–4276

I feel like this function could do with some early outs. I'm thinking something along the following lines would be much cleaner:

template <class ELFT>
void GNUELFDumper<ELFT>::printProgramHeaders(
    bool PrintProgramHeaders, cl::boolOrDefault PrintSectionMapping) {
  if (!PrintProgramHeaders && PrintSectionMapping == cl::BOU_FALSE)
    return;

  const Elf_Ehdr &Header = this->Obj.getHeader();
  if (Header.e_phnum == 0) {
    OS << "\nThere are no program headers in this file.\n";
    return;
  }

  if (PrintProgramHeaders)
    printProgramHeaders();
  if (PrintSectionMapping != cl::BOU_FALSE)
    printSectionMapping();
}

Thoughts?

[Prabhuk]

Program header rendering cleanup.

Rendering "no program header" message should not affect readelf invocations that request for section information only. Updated the PrintProgramHeader function and the corresponding tests.

[Prabhuk]

Ran clang-format to fix formatting mismatches.

[Prabhuk]

Updated the printProgramHeaders section to address the review comment and to fix logic flaws. Updated corresponding tests. Please take a look.

llvm/test/tools/llvm-readobj/ELF/gnu-section-mapping-no-phdrs.test

3–6 ↗

(On Diff #448669)

This change as it turns out is wrong. The code path that renders only "--section-mapping" is not originally intended to change.

llvm/test/tools/llvm-readobj/ELF/many-sections.s

11

You're right. This is a duplicate check of what is already handled in section-details.test. Removing this test.

llvm/test/tools/llvm-readobj/ELF/no-phdrs.test

15

Once again there was an error in this work flow. Clean up of "Program Headers" section was intended. But the Section to Segment mapping was not intended to be changed so bringing that part back.

llvm/tools/llvm-readobj/ELFDumper.cpp

3701–3702

test/tools/llvm-readobj/ELF/many-sections.s line 36,37 (shown below) exercises this code path.

1. RUN: llvm-readelf --file-headers --sections %t2 2>&1 | \
2. RUN: FileCheck %s -DFILE=%t2 --check-prefix=GNU2

4089–4090

Added a new test to cover this path:

test/tools/llvm-readobj/ELF/many-sections.s line 38,39 (shown below) :

1. RUN: llvm-readelf --file-headers --section-details %t2 2>&1 | \
2. RUN: FileCheck %s -DFILE=%t2 --check-prefix=GNU2

4275–4276

This comment helped me find a logical flaw in rendering. The control flow of invocations that request for segment details/mapping without the program header info should not change. I have updated this function to reflect this change and considered some of the cleanups suggested in your comment.

[emaste]

FWIW ELF Tool Chain readelf also emits There are no program headers in this file. or There are no sections in this file..

[jhenderson]

llvm/test/tools/llvm-readobj/ELF/no-phdrs.test

15

So just to confirm, GNU prints the (empty) section to segment mapping even if there are no program headers, when the program headers are dumped?

llvm/test/tools/llvm-readobj/ELF/no-shdrs.test

20

llvm/tools/llvm-readobj/ELFDumper.cpp

4292–4294

No need for braces in single-line if statements. Also, I'd rename IsPrintSectionMapping to ShouldPrintSectionMapping, as it is more grammatically correct.

4295

I'm not sure this and the equivalent comment below for the section mapping are really adding anything anymore, given the new code layout.

4305–4307

[Prabhuk]

Addressed review comments (var name, comment removal, code style, alignment).

[Prabhuk]

Removing braces for If block with single line code.

[Prabhuk]

Including clang-format fixes.

[Prabhuk]

llvm/test/tools/llvm-readobj/ELF/no-phdrs.test

15

That's correct! When we ask to dump program headers we dump section to segment mapping by default. Showing empty mapping offers users consistency on what to expect in the output printed to the console imo. But I could go either way (show empty mapping or not) to be honest.

[jhenderson]

Looks good to me.

[Prabhuk]

In D130670#3728080, @jhenderson wrote:

Looks good to me.

Thank you!

llvm/test/tools/llvm-readobj/ELF/many-sections.s

 ## Here we simulate an object with more than ~65k sections and check how we dump it.
 ## When an ELF object has SHN_LORESERVE (0xff00) or more sections, its e_shnum field
 ## should be zero and sh_size of the section header at index 0 is used
 ## to store the value. If the section name string table section index is
 ## greater than or equal to SHN_LORESERVE, then e_shstrndx field
 ## should have the value of SHN_XINDEX and sh_link of the section header
 ## at index 0 is used to store the value.
 
 # RUN: yaml2obj --docnum=1 %s -o %t1
 # RUN: llvm-readelf --file-headers -S %t1 | FileCheck %s --check-prefix=GNU1
 # GNU1: Number of section headers:         0 (3)
 # GNU1: Section header string table index: 65535 (2)
 # GNU1: There are 3 section headers, starting at offset 0x58
 
 # RUN: llvm-readobj --file-headers %t1 | FileCheck %s --check-prefix=LLVM1
 # LLVM1: SectionHeaderCount: 0 (3)
 # LLVM1: StringTableSectionIndex: 65535 (2)
 
 --- !ELF

 
 ## Another case is when e_shoff field set to zero, but not e_shstrndx.
 ## We want to show that this corrupt case is handled correctly.
 
 # RUN: yaml2obj --docnum=2 %s -o %t2
 
 # RUN: llvm-readelf --file-headers --sections %t2 2>&1 | \
 # RUN:   FileCheck %s -DFILE=%t2 --check-prefix=GNU2
+# RUN: llvm-readelf --file-headers --section-details %t2 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t2 --check-prefix=GNU2
 # GNU2: Number of section headers:         0
 # GNU2: Section header string table index: 65535 (corrupt: out of range)
 
-# GNU2:       There are 0 section headers, starting at offset 0x0:
 # GNU2-EMPTY:
-# GNU2-NEXT:  Section Headers:
-# GNU2-NEXT:   [Nr] Name Type Address Off Size ES Flg Lk Inf Al
+# GNU2:       There are no sections in this file.
 # GNU2-NEXT:  warning: '[[FILE]]': e_shstrndx == SHN_XINDEX, but the section header table is empty
-# GNU2-NEXT:  Key to Flags:
 
 # RUN: llvm-readobj --file-headers --sections %t2 | \
 # RUN:   FileCheck %s --check-prefix=LLVM2 --implicit-check-not="warning:"
 # LLVM2:       SectionHeaderCount: 0
 # LLVM2:       StringTableSectionIndex: 65535 (corrupt: out of range)
 # LLVM2-NEXT: }
 # LLVM2-NEXT: Sections [
 # LLVM2-NEXT: ]

llvm/test/tools/llvm-readobj/ELF/no-phdrs.test

 # Show that llvm-readobj can handle no program headers when --program-headers
 # is requested, both for LLVM and GNU output.
 
 # RUN: yaml2obj %s -o %t
 # RUN: llvm-readobj %t --program-headers | FileCheck %s --check-prefix=LLVM
 # RUN: llvm-readelf %t --program-headers | FileCheck %s --check-prefix=GNU
 
 # LLVM:      ProgramHeaders [
 # LLVM-NEXT: ]
 
-# GNU:       There are 0 program headers
-# GNU:       Program Headers:
-# GNU-NEXT:    Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
+# GNU: There are no program headers in this file.
 # GNU-EMPTY:
 # GNU-NEXT:  Section to Segment mapping:
 # GNU-NEXT:    Segment Sections...
 # GNU-NEXT:     None   .strtab .shstrtab
 
 --- !ELF
 FileHeader:
   Class: ELFCLASS64
   Data:  ELFDATA2LSB
   Type:  ET_REL

llvm/test/tools/llvm-readobj/ELF/no-shdrs.test

 # Show that llvm-readobj can handle ELF files where the section header table has
 # been stripped, both for GNU and LLVM style output.
 
 # RUN: yaml2obj %s -o %t.raw
 # RUN: llvm-objcopy --strip-sections %t.raw %t.o
 # RUN: llvm-readobj --file-headers --section-headers --symbols %t.o | FileCheck %s --check-prefix=LLVM
 # RUN: llvm-readelf --file-headers --section-headers --symbols %t.o | FileCheck %s --check-prefix=GNU --allow-empty
+# RUN: llvm-readelf --file-headers --section-details --symbols %t.o | FileCheck %s --check-prefix=GNU --allow-empty
 
 # LLVM:      SectionHeaderCount:      0
 # LLVM:      StringTableSectionIndex: 0
 # LLVM:      Sections [
 # LLVM-NEXT: ]
 # LLVM-NEXT: Symbols [
 # LLVM-NEXT: ]
 
 # GNU:     Number of section headers:         0
 # GNU:     Section header string table index: 0
-# GNU:     There are 0 section headers
+# GNU-EMPTY:
+# GNU:     There are no sections in this file.
 # GNU-NOT: Symbol table '{{.*}}' contains {{.*}} entries
 
 --- !ELF
 FileHeader:
   Class: ELFCLASS64
   Data:  ELFDATA2LSB
   Type:  ET_REL
 Sections:
   - Name: .text
     Type: SHT_PROGBITS
 Symbols:
   - Name:    foobar
     Section: .text
     Binding: STB_GLOBAL

llvm/tools/llvm-readobj/ELFDumper.cpp

if (EMachine == EM_X86_64)
     OS << ", l (large)";
   else if (EMachine == EM_ARM)
     OS << ", y (purecode)";
 
   OS << ", p (processor specific)\n";
 }
 
 template <class ELFT> void GNUELFDumper<ELFT>::printSectionHeaders() {
-  unsigned Bias = ELFT::Is64Bits ? 0 : 8;
   ArrayRef<Elf_Shdr> Sections = cantFail(this->Obj.sections());
+  if (Sections.empty()) {
+    OS << "\nThere are no sections in this file.\n";
+    Expected<StringRef> SecStrTableOrErr =
+        this->Obj.getSectionStringTable(Sections, this->WarningHandler);
+    if (!SecStrTableOrErr)
+      this->reportUniqueWarning(SecStrTableOrErr.takeError());
+    return;
+  }
+  unsigned Bias = ELFT::Is64Bits ? 0 : 8;
   OS << "There are " << to_string(Sections.size())
      << " section headers, starting at offset "
      << "0x" << utohexstr(this->Obj.getHeader().e_shoff, /*LowerCase=*/true) << ":\n\n";
   OS << "Section Headers:\n";
   Field Fields[11] = {
       {"[Nr]", 2},        {"Name", 7},        {"Type", 25},
       {"Address", 41},    {"Off", 58 - Bias}, {"Size", 65 - Bias},
       {"ES", 72 - Bias},  {"Flg", 75 - Bias}, {"Lk", 79 - Bias},
if (Error E = checkGNUHashTable<ELFT>(this->Obj, this->GnuHashTable))
       this->reportUniqueWarning(std::move(E));
     else
       printGnuHashTableSymbols(*this->GnuHashTable);
   }
 }
 
 template <class ELFT> void GNUELFDumper<ELFT>::printSectionDetails() {
   ArrayRef<Elf_Shdr> Sections = cantFail(this->Obj.sections());
+  if (Sections.empty()) {
+    OS << "\nThere are no sections in this file.\n";
+    Expected<StringRef> SecStrTableOrErr =
+        this->Obj.getSectionStringTable(Sections, this->WarningHandler);
+    if (!SecStrTableOrErr)
+      this->reportUniqueWarning(SecStrTableOrErr.takeError());
+    return;
+  }
   OS << "There are " << to_string(Sections.size())
      << " section headers, starting at offset "
      << "0x" << utohexstr(this->Obj.getHeader().e_shoff, /*LowerCase=*/true) << ":\n\n";
 
   OS << "Section Headers:\n";
 
   auto PrintFields = [&](ArrayRef<Field> V) {
     for (const Field &F : V)
bool CheckOffset = (Sec.sh_type == ELF::SHT_NOBITS) ||
                       Sec.sh_offset < Phdr.p_offset + Phdr.p_filesz);
   bool CheckVA = !(Sec.sh_flags & ELF::SHF_ALLOC) ||
                  (Sec.sh_addr > Phdr.p_vaddr && Sec.sh_addr < Phdr.p_memsz);
   return CheckOffset && CheckVA;
 }
 
 template <class ELFT>
 void GNUELFDumper<ELFT>::printProgramHeaders(
     bool PrintProgramHeaders, cl::boolOrDefault PrintSectionMapping) {
-  if (PrintProgramHeaders)
+  const bool ShouldPrintSectionMapping = (PrintSectionMapping != cl::BOU_FALSE);
+  // Exit early if no program header or section mapping details were requested.
+  if (!PrintProgramHeaders && !ShouldPrintSectionMapping)
+    return;
+
+  if (PrintProgramHeaders) {
+    const Elf_Ehdr &Header = this->Obj.getHeader();
+    if (Header.e_phnum == 0) {
+      OS << "\nThere are no program headers in this file.\n";
+    } else {
       printProgramHeaders();
+    }
+  }
 
-  // Display the section mapping along with the program headers, unless
-  // -section-mapping is explicitly set to false.
-  if (PrintSectionMapping != cl::BOU_FALSE)
+  if (ShouldPrintSectionMapping)
     printSectionMapping();
 }
 
 template <class ELFT> void GNUELFDumper<ELFT>::printProgramHeaders() {
   unsigned Bias = ELFT::Is64Bits ? 8 : 0;
   const Elf_Ehdr &Header = this->Obj.getHeader();
   Field Fields[8] = {2,         17,        26,        37 + Bias,
                      48 + Bias, 56 + Bias, 64 + Bias, 68 + Bias};
   OS << "\nElf file type is "
      << enumToString(Header.e_type, makeArrayRef(ElfObjectFileType)) << "\n"
      << "Entry point " << format_hex(Header.e_entry, 3) << "\n"
      << "There are " << Header.e_phnum << " program headers,"
      << " starting at offset " << Header.e_phoff << "\n\n"
      << "Program Headers:\n";
   if (ELFT::Is64Bits)
     OS << "  Type           Offset   VirtAddr           PhysAddr         "
        << "  FileSiz  MemSiz   Flg Align\n";
   else
     OS << "  Type           Offset   VirtAddr   PhysAddr   FileSiz "
        << "MemSiz  Flg Align\n";
 
   unsigned Width = ELFT::Is64Bits ? 18 : 10;
   unsigned SizeWidth = ELFT::Is64Bits ? 8 : 7;
 
   Expected<ArrayRef<Elf_Phdr>> PhdrsOrErr = this->Obj.program_headers();