[yaml2obj] - Allow empty SectionHeaderTable definitions.

[grimar]

Summary

Currently we don't allow the following definition:

Sections:
  - Type: SectionHeaderTable
  - Name: .foo
    Type: SHT_PROGBITS

We report an error: "SectionHeaderTable can't be empty. Use 'NoHeaders' key to drop the section header table".

It was implemented in this way earlier, when SectionHeaderTable
was a dedicated key outside of the Sections list. And we did not
allow to select where the table is written.

Currently it makes sense to allow it, because a user might
want to place the default section header table at an arbitrary position,
e.g. before other sections. In this case it is not convenient and error prone
to require specifying all sections:

Sections:
  - Type: SectionHeaderTable
    Sections:
      - Name: .foo
      - Name: .strtab
      - Name: .shstrtab
  - Name: .foo
    Type: SHT_PROGBITS

This patch allows empty SectionHeaderTable definitions.

[grimar]

Hi guys.
I am sorry for pinging this too early, but I am planning a ~6 weeks vacation starting from 1st of february
and I wonder if there is something I can do for this to land it or if there are any concerns about this change?

[jhenderson]

LGTM.

In D95341#2524529, @grimar wrote:

Hi guys.
I am sorry for pinging this too early, but I am planning a ~6 weeks vacation starting from 1st of february
and I wonder if there is something I can do for this to land it or if there are any concerns about this change?

No problem. Somehow this slipped off my review pile, so I hadn't seen it. Let me know if there are any other reviews I haven't done by the end of the day.

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

155

I think this sounds better English to me, though I can't place specifically what was wrong before!

[grimar]

In D95341#2524716, @jhenderson wrote:

LGTM.

In D95341#2524529, @grimar wrote:

Hi guys.
I am sorry for pinging this too early, but I am planning a ~6 weeks vacation starting from 1st of february
and I wonder if there is something I can do for this to land it or if there are any concerns about this change?

No problem. Somehow this slipped off my review pile, so I hadn't seen it. Let me know if there are any other reviews I haven't done by the end of the day.

Nothing else. This is the only one. Thanks!

llvm/include/llvm/ObjectYAML/ELFYAML.h

static bool classof(const Chunk *S) {
     return S->Kind == ChunkKind::SectionHeaderTable;
   }
 
   Optional<std::vector<SectionHeader>> Sections;
   Optional<std::vector<SectionHeader>> Excluded;
   Optional<bool> NoHeaders;
 
   size_t getNumHeaders(size_t SectionsNum) const {
-    if (IsImplicit)
+    if (IsImplicit || isDefault())
       return SectionsNum;
     if (NoHeaders)
       return (*NoHeaders) ? 0 : SectionsNum;
     return (Sections ? Sections->size() : 0) + /*Null section*/ 1;
   }
 
+  bool isDefault() const { return !Sections && !Excluded && !NoHeaders; }
+
   static constexpr StringRef TypeStr = "SectionHeaderTable";
 };
 
 struct BBAddrMapSection : Section {
   Optional<std::vector<BBAddrMapEntry>> Entries;
 
   BBAddrMapSection() : Section(ChunkKind::BBAddrMap) {}
 

llvm/lib/ObjectYAML/ELFEmitter.cpp

else
       reportError("unknown section referenced: '" + S + "' by YAML section '" +
                   LocSec + "'");
     return 0;
   }
 
   const ELFYAML::SectionHeaderTable &SectionHeaders =
       Doc.getSectionHeaderTable();
   if (SectionHeaders.IsImplicit ||
-      (SectionHeaders.NoHeaders && !SectionHeaders.NoHeaders.getValue()))
+      (SectionHeaders.NoHeaders && !SectionHeaders.NoHeaders.getValue()) ||
+      SectionHeaders.isDefault())
     return Index;
 
   assert(!SectionHeaders.NoHeaders.getValueOr(false) ||
          !SectionHeaders.Sections);
   size_t FirstExcluded =
       SectionHeaders.Sections ? SectionHeaders.Sections->size() : 0;
   if (Index >= FirstExcluded) {
     if (LocSym.empty())
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() {
   const ELFYAML::SectionHeaderTable &SectionHeaders =
       Doc.getSectionHeaderTable();
-  if (SectionHeaders.IsImplicit || SectionHeaders.NoHeaders)
+  if (SectionHeaders.IsImplicit || SectionHeaders.NoHeaders ||
+      SectionHeaders.isDefault())
     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/lib/ObjectYAML/ELFYAML.cpp

if (const auto *F = dyn_cast<ELFYAML::Fill>(C.get())) {
     if (F->Pattern && F->Pattern->binary_size() != 0 && !F->Size)
       return "\"Size\" can't be 0 when \"Pattern\" is not empty";
     return "";
   }
 
   if (const auto *SHT = dyn_cast<ELFYAML::SectionHeaderTable>(C.get())) {
     if (SHT->NoHeaders && (SHT->Sections || SHT->Excluded || SHT->Offset))
       return "NoHeaders can't be used together with Offset/Sections/Excluded";
-    if (!SHT->NoHeaders && !SHT->Sections && !SHT->Excluded)
-      return "SectionHeaderTable can't be empty. Use 'NoHeaders' key to drop "
-             "the section header table";
     return "";
   }
 
   const ELFYAML::Section &Sec = *cast<ELFYAML::Section>(C.get());
   if (Sec.Size && Sec.Content &&
       (uint64_t)(*Sec.Size) < Sec.Content->binary_size())
     return "Section size must be greater than or equal to the content size";
 

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

FileHeader:
   Type:  ET_REL
 Sections:
   - Name: .foo
     Type: SHT_PROGBITS
   - Type: SectionHeaderTable
     Sections:  []
     NoHeaders: [[NOHEADERS]]
 
-## Check that we do not allow an empty SectionHeaderTable tag and suggest to use an explicit syntax instead.
-# RUN: not yaml2obj %s --docnum=5 -o /dev/null 2>&1 | FileCheck %s --check-prefix=NO-VALUE
+## Check that we allow using an empty SectionHeaderTable definition.
+## It can be used to emit the default section header table at an arbitrary position.
 
-# NO-VALUE: SectionHeaderTable can't be empty. Use 'NoHeaders' key to drop the section header table
+# RUN: yaml2obj %s --docnum=5 -o %t5.novalues
+# RUN: llvm-readelf --sections %t5.novalues | \
+# RUN:   FileCheck %s --check-prefix=NO-VALUES
+
+## Check we placed the section header table before the .foo section.
+
+# NO-VALUES: There are 4 section headers, starting at offset 0x40:
+# NO-VALUES: [Nr] Name Type     Address          Off    Size
+# NO-VALUES: [ 1] .foo PROGBITS 0000000000000000 000140 000000
 
 --- !ELF
 FileHeader:
   Class: ELFCLASS64
   Data:  ELFDATA2LSB
   Type:  ET_REL
 Sections:
+  - Type: SectionHeaderTable
   - Name: .foo
     Type: SHT_PROGBITS
-  - Type: SectionHeaderTable
 
 ## Test that we are still able to override e_shoff, e_shnum and e_shstrndx
 ## fields even when we do not produce section headers.
 # RUN: yaml2obj %s --docnum=6 -o %t4
 # RUN: not llvm-readelf --file-headers %t4 | FileCheck %s --check-prefix=NO-HEADERS-OVERRIDE
 
 # NO-HEADERS-OVERRIDE: Start of section headers:          2 (bytes into file)
 # NO-HEADERS-OVERRIDE: Number of section headers:         3