[obj2yaml] - Dump the `EShNum` key in some cases.

[grimar]

Summary

This patch starts emitting the EShNum key, when the e_shnum = 0
and the section header table exists.

e_shnum might be 0, when the the number of entries in the section header
table is larger than or equal to SHN_LORESERVE (0xff00).
In this case the real number of entries
in the section header table is held in the sh_size
member of the initial entry in section header table.

Currently, obj2yaml crashes when an object has e_shoff != 0 and the sh_size
member of the initial entry in section header table is 0.
This patch fixes it.

[MaskRay]

llvm/test/tools/obj2yaml/ELF/eshnum.yaml

28

The following may be slightly better:

--- !ELF
FileHeader:
  Class:  ELFCLASS64
  Data:   ELFDATA2LSB
  Type:   ET_REL
SectionHeaderTable:
  NoHeaders: true

[grimar]

llvm/test/tools/obj2yaml/ELF/eshnum.yaml

28

We currently don't emit the SectionHeaderTable. Though I guess it might be reasonable to do it when sections are out of order or when there is no section header.

[jhenderson]

llvm/test/tools/obj2yaml/ELF/eshnum.yaml

10

llvm/tools/obj2yaml/elf2yaml.cpp

238

I take it V doesn't normally contain a chunk for the null (index 0) section header?

[grimar]

• Addressed review comments.

llvm/tools/obj2yaml/elf2yaml.cpp

238

V contains all sections that are dumped from an object. The SHT_NULL section is
normally present, it dumped as a regular section, and so V usually contains it. This is assumed below:

for (const std::unique_ptr<ELFYAML::Chunk> &C :
     makeArrayRef(V).drop_front()) {

In the case revealed by this patch we have no section header and no sections are dumped,
that's why V is empty.

[grimar]

llvm/tools/obj2yaml/elf2yaml.cpp

238

"we have no section header" -> "we have 0 sections"

[grimar]

llvm/test/tools/obj2yaml/ELF/eshnum.yaml

28

BTW,

--- !ELF
FileHeader:
  Class:  ELFCLASS64
  Data:   ELFDATA2LSB
  Type:   ET_REL
SectionHeaderTable:
  NoHeaders: true

is not correct. In this case we have e_shnum = 0. So we should read the number of sections from
the sh_size member of the initial entry in section header table (see test case header comment).

I.e. I think the output here should be the same as YAML input.

[jhenderson]

llvm/test/tools/obj2yaml/ELF/eshnum.yaml

15–20

If you feed this output of obj2yaml back into yaml2obj, the final object doesn't really reflect what you had originally - it generates three section headers (.strtab and .shstrtab, plus the null one). Seems like we need to fix something, but not entirely sure what.

llvm/tools/obj2yaml/elf2yaml.cpp

238

You've lost me a little. In the case when e_shnum and sh_size of section table index 0 are both 0, obj2yaml doesn't populate V with anything? So there is a section header table, it's just that no sections are read, because we say there are none. Is that right?

[grimar]

llvm/test/tools/obj2yaml/ELF/eshnum.yaml

15–20

Ideally we might want to produce the same output as YAML input I think.
At the same time this is a corner case that I am not sure is worth to support.
I've tried to expain in the comment below.

llvm/tools/obj2yaml/elf2yaml.cpp

238

You've lost me a little. In the case when e_shnum and sh_size of section table index 0 are both 0, obj2yaml doesn't populate V with anything?

Yes.

So there is a section header table, it's just that no sections are read, because we say there are none. Is that right?

Yes.

The responsible code that returns us the sections list is:

template <class ELFT>
Expected<typename ELFT::ShdrRange> ELFFile<ELFT>::sections() const {
  const uintX_t SectionTableOffset = getHeader().e_shoff;
  if (SectionTableOffset == 0)
    return ArrayRef<Elf_Shdr>();

....

  const Elf_Shdr *First =
      reinterpret_cast<const Elf_Shdr *>(base() + SectionTableOffset);

  uintX_t NumSections = getHeader().e_shnum;
  if (NumSections == 0)
    NumSections = First->sh_size;

....
  return makeArrayRef(First, NumSections);
}

When e_shoff != 0 and e_shnum==0, it reads the number of sections from First->sh_size (which is 0) and returns us an empty array.
This is why no chunks are dumped.

To be able to support the case of this patch properly I think we should check the e_shnum manually. When it is 0, and the e_shoff != 0,
then we should dump the first section header table entry (manually). After that we should emit the EShNum = 0 key to preserve e_shnum.
With all of these steps done, the output of the test case will be the same as input.

But do we want all this complexity to support this particular corner case? I believe having all this conditions in the real life is unrealistic. I've only tried to fix the crash I've found accidentally.

[grimar]

I am experimenting with the patch which preserves the EShNum key when e_shnum == 0. It is usefull for the normal scenario when
the number of sections is greater than or equal to the SHN_LORESERVE .

I'll rebase this patch later. I think that after we start emitting the EShNum = 0, this patch will produce an equivalent output for the input given.

[grimar]

In D92098#2418299, @grimar wrote:

I am experimenting with the patch which preserves the EShNum key when e_shnum == 0. It is usefull for the normal scenario when
the number of sections is greater than or equal to the SHN_LORESERVE .

I'll rebase this patch later. I think that after we start emitting the EShNum = 0, this patch will produce an equivalent output for the input given.

It turns out this is impossible to prepare a different patch because of this crash.
So updating this one with the new functionality.

[grimar]

[jhenderson]

LGTM, with the suggested comment fixes.

llvm/test/tools/obj2yaml/ELF/eshnum.yaml

2

8

11

41–43

57

llvm/tools/obj2yaml/elf2yaml.cpp

297–299

I think you can make this more concise like this.

300

llvm/test/tools/obj2yaml/ELF/eshnum.yaml

+## Check cases related to the value of the e_shnum field of the ELF header live here.
+
+## Normally an object that does not have sections has e_shoff == 0.
+## Also, e_shoff might be 0, when the the number of entries in the section
+## header table is larger than or equal to SHN_LORESERVE (0xff00). In this case
+## the real number of entries in the section header table is held in the sh_size
+## member of the initial entry in section header table.
+## In the test case below we have an object with a non-zero section header table
+## file offset and no sections. Here we check that we are able to dump such
+## objects properly.
+
+# RUN: yaml2obj %s -o %t
+# RUN: obj2yaml %t | FileCheck %s
+
+# CHECK:      --- !ELF
+# CHECK-NEXT: FileHeader:
+# CHECK-NEXT:   Class: ELFCLASS64
+# CHECK-NEXT:   Data:  ELFDATA2LSB
+# CHECK-NEXT:   Type:  ET_REL
+# CHECK-NEXT: ...
+
+--- !ELF
+FileHeader:
+  Class:  ELFCLASS64
+  Data:   ELFDATA2LSB
+  Type:   ET_REL
+  EShNum: 0x0
+Sections:
+  - Type: SHT_NULL
+    Size: 0x0

llvm/tools/obj2yaml/elf2yaml.cpp

if (S.Type == ELF::SHT_STRTAB || S.Type == ELF::SHT_SYMTAB ||
     return S.Flags.getValueOr(ELFYAML::ELF_SHF(0)) & ELF::SHF_ALLOC;
 
   return true;
 }
 
 template <class ELFT>
 static void dumpSectionOffsets(const typename ELFT::Ehdr &Header,
                                ArrayRef<ELFYAML::ProgramHeader> Phdrs,
                                std::vector<std::unique_ptr<ELFYAML::Chunk>> &V,
                                ArrayRef<typename ELFT::Shdr> S) {
+  if (V.empty())
+    return;
+
   uint64_t ExpectedOffset;
   if (Header.e_phoff > 0)
     ExpectedOffset = Header.e_phoff + Header.e_phentsize * Header.e_phnum;
   else
     ExpectedOffset = sizeof(typename ELFT::Ehdr);
 
   for (const std::unique_ptr<ELFYAML::Chunk> &C :
        makeArrayRef(V).drop_front()) {
template <class ELFT> Expected<ELFYAML::Object *> ELFDumper<ELFT>::dump() {
   auto SectionsOrErr = Obj.sections();
   if (!SectionsOrErr)
     return SectionsOrErr.takeError();
   Sections = *SectionsOrErr;
   SectionNames.resize(Sections.size());
 
   // Dump symbols. We need to do this early because other sections might want
   // to access the deduplicated symbol names that we also create here.
   const Elf_Shdr *SymTab = nullptr;
   const Elf_Shdr *SymTabShndx = nullptr;
   const Elf_Shdr *DynSymTab = nullptr;
 
   for (const Elf_Shdr &Sec : Sections) {
     if (Sec.sh_type == ELF::SHT_SYMTAB) {
       SymTab = &Sec;
     } else if (Sec.sh_type == ELF::SHT_DYNSYM) {
       DynSymTab = &Sec;
     } else if (Sec.sh_type == ELF::SHT_SYMTAB_SHNDX) {
       // ABI allows us to have one SHT_SYMTAB_SHNDX for each symbol table.
       // We only support having the SHT_SYMTAB_SHNDX for SHT_SYMTAB now.