[llvm-readobj] - Teach tool to dump objects with >= SHN_LORESERVE of sections.

[grimar]

Summary

http://www.sco.com/developers/gabi/2003-12-17/ch4.eheader.html

says that e_shnum and/or e_shstrndx may have special values if
"the number of sections is greater than or equal to SHN_LORESERVE" or
"the section name string table section index is greater than or equal to SHN_LORESERVE (0xff00)"

Previously llvm-readobj was unable to dump such files, patch changes that.

I had to add a precompiled test case because it does not seem possible to
prepare a test using yaml2obj or llvm-mc (not clear how to make .shstrtab
to have index >= SHN_LORESERVE).

[jhenderson]

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

1 ↗

(On Diff #155654)

Thanks for the descriptive comment! I've got a few grammar nits though.

that generated -> that was

2 ↗

(On Diff #155654)

When ELF -> When an ELF

3 ↗

(On Diff #155654)

more of sections -> more sections
it's -> its

4 ↗

(On Diff #155654)

used -> is used

5 ↗

(On Diff #155654)

If section... -> If the section

8 ↗

(On Diff #155654)

used -> is used

10–12 ↗

(On Diff #155654)

has little amount of sections -> has few sections
set in according to above -> set according to the above

tools/llvm-readobj/ELFDumper.cpp

2474 ↗

(On Diff #155654)

It may be worth giving a comment here and on the next function briefly explaining the rules, similar to what you did in the test.

2476 ↗

(On Diff #155654)

Personally, I'd be explicit here, and say ElfHeader->e_shnum > 0, but I don't mind that much, if you prefer it this way.

2478–2479 ↗

(On Diff #155654)

What about the situation where a file has no section headers? This will crash, I think. Please fix and add a test for this case. I think if e_shnum is zero, you need to check e_shoff to see if it has a non-zero value to decide if there are section headers.

2487 ↗

(On Diff #155654)

Again, what happens if there are no section headers here? In this case, the object is malformed, and we should probably report an error or warning, but still write the actual value in the field.

[grimar]

Addressed review comments. (Thanks, James!)

tools/llvm-readobj/ELFDumper.cpp

2478–2479 ↗

(On Diff #155654)

It would report "Error reading file: invalid section index" actually (because 'unwrapOrError' catches and reports an error and exit() then). It still was an issue though. Fixed, added test case.

2487 ↗

(On Diff #155654)

I am printing "corrupt: out of range" here, it what readelf do.

[jhenderson]

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

25 ↗

(On Diff #155847)

I'd use the term "zero" rather than null here, since it's just a value.

tools/llvm-readobj/ELFDumper.cpp

2492–2493 ↗

(On Diff #155847)

I'm not seeing this behaviour in readelf? If there are no section headers, I just get a value of 0 for the section header string table index, and no warning. A non-zero value on the other hand, when there are no sections (or indeed any value outside that range) does give me that error.

[grimar]

tools/llvm-readobj/ELFDumper.cpp

2492–2493 ↗

(On Diff #155847)

I tested many-sections-stripped.elf-x86_64
(shared it here: https://drive.google.com/open?id=1HgUSb15a6WQY_lwMIUCDXBxD25VX8I-S)
which has e_shoff set to zero. And it has e_shstrndx == SHN_XINDEX.

It reports the following for me:

readelf -v
GNU readelf (GNU Binutils for Ubuntu) 2.28
...

readelf -a many-sections-stripped.elf-x86_64 
ELF Header:
...
  Number of section headers:         0
  Section header string table index: 65535 <corrupt: out of range>

I think when we have e_shstrndx == SHN_XINDEX, we expect that "The actual number of section header table entries is contained in the sh_size field of the section header at index 0". And since actual number is always > 0 ("If the number of sections is greater than or equal to SHN_LORESERVE...").
(https://docs.oracle.com/cd/E19120-01/open.solaris/819-0690/chapter6-43405/index.html),
we are fine reporting "out of range" here when having no sections. Isn't that look valid behavior?

[jhenderson]

tools/llvm-readobj/ELFDumper.cpp

2492–2493 ↗

(On Diff #155847)

I think we might be misunderstanding each other. I think these are the cases we need to be aware of:

1. The regular case where 0 < e_shstrndx < SHN_XINDEX. Here, we should (and do already) print the correct number.
2. The regular large number of sections case where e_shrstndx == SHN_XINDEX. Here we should look up the number and print it along with SHN_XINDEX. This is what your patch implements, and is tested by LLVM1 etc.
3. The case where e_shstrndx is zero. This means that there is no section header string table. We should just print zero in this case.

In cases one and two, if the index is greater than the number of sections, we should print an error. In a stripped ELF, there is no reason for a section header string table, so the index should be 0, like case 3), and we should not print an error, even if there were many section headers previously. Currently you are printing "0 (corrupt: out of range)", even though the e_shstrndx field is non-zero. I would expect it to print "65535 (corrupt: out of range)" or similar. So I think the printing of '0' is the bad part, because it hides the actual value in the field.

Does that make sense?

[grimar]

tools/llvm-readobj/ELFDumper.cpp

2492–2493 ↗

(On Diff #155847)

So I think the printing of '0' is the bad part, because it hides the actual value in the field.

Ah, I missed that I am printing 0 (corrupt: out of range) now.
My intention sure was to print the value, so to print the 65535 (corrupt: out of range).

I'll update the patch.

[grimar]

• Addressed review comments.

[jhenderson]

LGTM, with a comment change in the test.

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

25 ↗

(On Diff #155884)

I suppose technically this is malformed, since if the section headers are stripped, the e_shstrndx should be zero. Maybe you should change the comment here to:

"... set to zero, but not e_shstrndx, to show that this corrupt case is handled correctly."

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

+## many-sections.elf-x86_64 is a file that was generated to simulate
+## an object with more than ~65k sections. 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.
+##
+## many-sections.elf-x86_64 has few sections to save disk
+## space, but its e_shnum, e_shstrndx, sh_size and sh_link fields are set
+## according to the above description, so that we can test the dumper.
+
+# RUN: llvm-readobj -file-headers -elf-output-style GNU \
+# RUN:   %p/Inputs/many-sections.elf-x86_64 | FileCheck %s --check-prefix=GNU1
+# GNU1: Number of section headers:         0 (5)
+# GNU1: Section header string table index: 65535 (3)
+
+# RUN: llvm-readobj -file-headers -elf-output-style LLVM \
+# RUN:   %p/Inputs/many-sections.elf-x86_64 | FileCheck %s --check-prefix=LLVM1
+# LLVM1: SectionHeaderCount: 0 (5)
+# LLVM1: StringTableSectionIndex: 65535 (3)
+
+## many-sections-stripped.elf-x86_64 is many-sections.elf-x86_64 with
+## e_shoff field set to zero, but not e_shstrndx, to show that
+## this corrupt case is handled correctly.
+
+# RUN: llvm-readobj -file-headers -elf-output-style GNU \
+# RUN:   %p/Inputs/many-sections-stripped.elf-x86_64 | FileCheck %s --check-prefix=GNU2
+# GNU2: Number of section headers:         0
+# GNU2: Section header string table index: 65535 (corrupt: out of range)
+
+# RUN: llvm-readobj -file-headers -elf-output-style LLVM \
+# RUN:   %p/Inputs/many-sections-stripped.elf-x86_64 | FileCheck %s --check-prefix=LLVM2
+# LLVM2: SectionHeaderCount: 0
+# LLVM2: StringTableSectionIndex: 65535 (corrupt: out of range)

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

static inline void printFields(formatted_raw_ostream &OS, StringRef Str1,
                                StringRef Str2) {
   OS.PadToColumn(2u);
   OS << Str1;
   OS.PadToColumn(37u);
   OS << Str2 << "\n";
   OS.flush();
 }
 
+template <class ELFT>
+static std::string getSectionHeadersNumString(const ELFFile<ELFT> *Obj) {
+  const typename ELFT::Ehdr *ElfHeader = Obj->getHeader();
+  if (ElfHeader->e_shnum != 0)
+    return to_string(ElfHeader->e_shnum);
+
+  ArrayRef<typename ELFT::Shdr> Arr = unwrapOrError(Obj->sections());
+  if (Arr.empty())
+    return "0";
+  return "0 (" + to_string(Arr[0].sh_size) + ")";
+}
+
+template <class ELFT>
+static std::string getSectionHeaderTableIndexString(const ELFFile<ELFT> *Obj) {
+  const typename ELFT::Ehdr *ElfHeader = Obj->getHeader();
+  if (ElfHeader->e_shstrndx != SHN_XINDEX)
+    return to_string(ElfHeader->e_shstrndx);
+
+  ArrayRef<typename ELFT::Shdr> Arr = unwrapOrError(Obj->sections());
+  if (Arr.empty())
+    return "65535 (corrupt: out of range)";
+  return to_string(ElfHeader->e_shstrndx) + " (" + to_string(Arr[0].sh_link) + ")";
+}
+
 template <class ELFT> void GNUStyle<ELFT>::printFileHeaders(const ELFO *Obj) {
   const Elf_Ehdr *e = Obj->getHeader();
   OS << "ELF Header:\n";
   OS << "  Magic:  ";
   std::string Str;
   for (int i = 0; i < ELF::EI_NIDENT; i++)
     OS << format(" %02x", static_cast<int>(e->e_ident[i]));
   OS << "\n";
template <class ELFT> void GNUStyle<ELFT>::printFileHeaders(const ELFO *Obj) {
   Str = to_string(e->e_ehsize) + " (bytes)";
   printFields(OS, "Size of this header:", Str);
   Str = to_string(e->e_phentsize) + " (bytes)";
   printFields(OS, "Size of program headers:", Str);
   Str = to_string(e->e_phnum);
   printFields(OS, "Number of program headers:", Str);
   Str = to_string(e->e_shentsize) + " (bytes)";
   printFields(OS, "Size of section headers:", Str);
-  Str = to_string(e->e_shnum);
+  Str = getSectionHeadersNumString(Obj);
   printFields(OS, "Number of section headers:", Str);
-  Str = to_string(e->e_shstrndx);
+  Str = getSectionHeaderTableIndexString(Obj);
   printFields(OS, "Section header string table index:", Str);
 }
 
 namespace {
 struct GroupMember {
   StringRef Name;
   uint64_t Index;
 };
const Elf_Ehdr *e = Obj->getHeader();
     else if (e->e_machine == EM_RISCV)
       W.printFlags("Flags", e->e_flags, makeArrayRef(ElfHeaderRISCVFlags));
     else
       W.printFlags("Flags", e->e_flags);
     W.printNumber("HeaderSize", e->e_ehsize);
     W.printNumber("ProgramHeaderEntrySize", e->e_phentsize);
     W.printNumber("ProgramHeaderCount", e->e_phnum);
     W.printNumber("SectionHeaderEntrySize", e->e_shentsize);
-    W.printNumber("SectionHeaderCount", e->e_shnum);
-    W.printNumber("StringTableSectionIndex", e->e_shstrndx);
+    W.printString("SectionHeaderCount", getSectionHeadersNumString(Obj));
+    W.printString("StringTableSectionIndex", getSectionHeaderTableIndexString(Obj));
   }
 }
 
 template <class ELFT>
 void LLVMStyle<ELFT>::printGroupSections(const ELFO *Obj) {
   DictScope Lists(W, "Groups");
   std::vector<GroupSection> V = getGroups<ELFT>(Obj);
   DenseMap<uint64_t, const GroupSection *> Map = mapSectionsToGroups(V);