[yaml2obj] - Fix the issues with SectionHeaderTable when Sections key is empty.

[grimar]

Summary

We have an issue currently. The following YAML piece just ignores the Excluded key.

SectionHeaderTable:
  Sections: []
  Excluded:
    - Name: .foo

The code checks that the Sections key is empty and doesn't catch/check
invalid/duplicated/missed "Excluded" entries.

Also there is no way to exclude all sections except the first null section.
I suggest the simple fix that makes the next logic to work properly:

1. When there is a "Sections: []", but no "Excluded" key, we omit all section headers.
2. When there is a "Sections: []" and non-empty "Excluded", then the latter is validated properly and must list all sections. This can be used to create an objects which has no section headers except the null section.

[MaskRay]

Is it possible to use an omitted Sections (rather than an empty (Sections: [])) to represent that all section headers except Excluded should be present?
For Sections: [], a reader may expect that there is no section header.

[grimar]

In D81581#2085349, @MaskRay wrote:

Is it possible to use an omitted Sections (rather than an empty (Sections: [])) to represent that all section headers except Excluded should be present?

The current logic is that all sections must be included either in "Sections" or in "Excluded" list. We error out when them are not.
It was done because of implicit sections, like .strtab, .shstrtab which are easy to forget about. I think we should keep this rule.

< there was a suggestion here, but I removed it, because need to think more about it >

[grimar]

I'll suggest a different approach.

[grimar]

I suggest to go with D81655 instead.

llvm/lib/ObjectYAML/ELFEmitter.cpp

void ELFState<ELFT>::writeELFHeader(raw_ostream &OS, uint64_t SHOff) {
   Header.e_flags = Doc.Header.Flags;
   Header.e_ehsize = sizeof(Elf_Ehdr);
   Header.e_phentsize = Doc.ProgramHeaders.size() ? sizeof(Elf_Phdr) : 0;
   Header.e_phnum = Doc.ProgramHeaders.size();
 
   Header.e_shentsize =
       Doc.Header.SHEntSize ? (uint16_t)*Doc.Header.SHEntSize : sizeof(Elf_Shdr);
 
+  const bool NoShdrs = Doc.SectionHeaders &&
+                       Doc.SectionHeaders->Sections.empty() &&
+                       !Doc.SectionHeaders->Excluded;
+
   if (Doc.Header.SHOff)
     Header.e_shoff = *Doc.Header.SHOff;
-  else if (Doc.SectionHeaders && Doc.SectionHeaders->Sections.empty())
+  else if (NoShdrs)
     Header.e_shoff = 0;
   else
     Header.e_shoff = SHOff;
 
   if (Doc.Header.SHNum)
     Header.e_shnum = *Doc.Header.SHNum;
   else if (!Doc.SectionHeaders)
     Header.e_shnum = Doc.getSections().size();
-  else if (Doc.SectionHeaders->Sections.empty())
+  else if (NoShdrs)
     Header.e_shnum = 0;
   else
     Header.e_shnum = Doc.SectionHeaders->Sections.size() + /*Null section*/ 1;
 
   if (Doc.Header.SHStrNdx)
     Header.e_shstrndx = *Doc.Header.SHStrNdx;
-  else if ((!Doc.SectionHeaders || !Doc.SectionHeaders->Sections.empty()) &&
-           !ExcludedSectionHeaders.count(".shstrtab"))
-    Header.e_shstrndx = SN2I.get(".shstrtab");
-  else
+  else if (NoShdrs || ExcludedSectionHeaders.count(".shstrtab"))
     Header.e_shstrndx = 0;
+  else
+    Header.e_shstrndx = SN2I.get(".shstrtab");
 
   OS.write((const char *)&Header, sizeof(Header));
 }
 
 template <class ELFT>
 void ELFState<ELFT>::initProgramHeaders(std::vector<Elf_Phdr> &PHeaders) {
   DenseMap<StringRef, ELFYAML::Fill *> NameToFill;
   for (const std::unique_ptr<ELFYAML::Chunk> &D : Doc.Chunks)
void ELFState<ELFT>::writeFill(ELFYAML::Fill &Fill,
   uint64_t Written = 0;
   for (; Written + PatternSize <= Fill.Size; Written += PatternSize)
     CBA.writeAsBinary(*Fill.Pattern);
   CBA.writeAsBinary(*Fill.Pattern, Fill.Size - Written);
 }
 
 template <class ELFT>
 DenseMap<StringRef, size_t> ELFState<ELFT>::buildSectionHeaderReorderMap() {
-  if (!Doc.SectionHeaders || Doc.SectionHeaders->Sections.empty())
+  if (!Doc.SectionHeaders ||
+      (Doc.SectionHeaders->Sections.empty() && !Doc.SectionHeaders->Excluded))
     return DenseMap<StringRef, size_t>();
 
   DenseMap<StringRef, size_t> Ret;
   size_t SecNdx = 0;
   StringSet<> Seen;
 
   auto AddSection = [&](const ELFYAML::SectionHeader &Hdr) {
     if (!Ret.try_emplace(Hdr.Name, ++SecNdx).second)

llvm/test/tools/yaml2obj/ELF/section-headers-exclude.yaml

 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_X86_64
 SectionHeaderTable:
   Sections: []
-  Excluded:
-    - Name: .strtab
  
 ## Check how we handle cases when a section is excluded, but its section index is needed.
 ## The general rule is: when a section is explicitly linked with another section, which is
 ## excluded, then we report an error. In the case when it is linked implicitly with an excluded
 ## section, we use 0 as index value.
 
 ## Case A: check we report an error when a regular section has a Link field which
 ##         points to an excluded section.
 # RUN: not yaml2obj %s --docnum=4 -o /dev/null 2>&1 | \
FileHeader:
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_X86_64
 SectionHeaderTable:
   Sections:
     - Name: .strtab
   Excluded:
     - Name: .shstrtab
+
+## Check that we keep the null SHF_UNDEF section when the "Sections" key is empty,
+## but the "Excluded" key is not
+# RUN: yaml2obj --docnum=18 %s -o %t10
+# RUN: llvm-readelf --sections %t10 | FileCheck %s --check-prefix=KEEP-NULL
+
+# KEEP-NULL:      There are 1 section headers, starting at offset 0x48:
+# KEEP-NULL:      [Nr] Name         Type Address          Off    Size   ES Flg Lk Inf Al
+# KEEP-NULL-NEXT: [ 0] <no-strings> NULL 0000000000000000 000000 000000 00     0   0  0
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_X86_64
+SectionHeaderTable:
+  Sections: []
+  Excluded:
+    - Name: .strtab
+    - Name: .shstrtab
+
+## Check we are able to report all possible errors mentioned above even when the "Sections" key is empty.
+# RUN: not yaml2obj %s --docnum=19 -o /dev/null 2>&1 | FileCheck %s --check-prefix=INVALID-EXCLUDED
+
+# INVALID-EXCLUDED: error: repeated section name: '.strtab' in the section header description
+# INVALID-EXCLUDED: error: section '.shstrtab' should be present in the 'Sections' or 'Excluded' lists
+# INVALID-EXCLUDED: error: section header contains undefined section '.unknown'
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_X86_64
+SectionHeaderTable:
+  Sections: []
+  Excluded:
+    - Name: .strtab
+    - Name: .strtab
+    - Name: .unknown