[yaml2obj] - Improve handling of SectionHeaderTable::NoHeaders flag.

[grimar]

Summary

When NoHeaders is set, we still have following issues:

1. We emit the .shstrtab implicit section of size 1 (empty string table).
2. We still align the start of the section header table, what affects the output size.
3. We still write section header table bytes.

This patch fixes all of these issues.

[grimar]

• Minor comment fix.

[jhenderson]

llvm/lib/ObjectYAML/ELFEmitter.cpp

1885

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

194–196

This might want a FIXME note. I think it corresponds to https://bugs.llvm.org/show_bug.cgi?id=40804 (might be worth putting that URL as a reference here too).

[grimar]

• Addressed review comment.

[jhenderson]

LGTM!

llvm/lib/ObjectYAML/ELFEmitter.cpp

template <class ELFT> class ELFState {
   void setProgramHeaderLayout(std::vector<Elf_Phdr> &PHeaders,
                               std::vector<Elf_Shdr> &SHeaders);
 
   std::vector<Fragment>
   getPhdrFragments(const ELFYAML::ProgramHeader &Phdr,
                    ArrayRef<typename ELFT::Shdr> SHeaders);
 
   void finalizeStrings();
-  void writeELFHeader(raw_ostream &OS, uint64_t SHOff);
+  void writeELFHeader(raw_ostream &OS, Optional<uint64_t> SHOff);
   void writeSectionContent(Elf_Shdr &SHeader,
                            const ELFYAML::NoBitsSection &Section,
                            ContiguousBlobAccumulator &CBA);
   void writeSectionContent(Elf_Shdr &SHeader,
                            const ELFYAML::RawContentSection &Section,
                            ContiguousBlobAccumulator &CBA);
   void writeSectionContent(Elf_Shdr &SHeader,
                            const ELFYAML::RelocationSection &Section,
if (Doc.DynamicSymbols)
     ImplicitSections.insert(ImplicitSections.end(), {".dynsym", ".dynstr"});
   if (Doc.Symbols)
     ImplicitSections.push_back(".symtab");
   if (Doc.DWARF)
     for (StringRef DebugSecName : Doc.DWARF->getNonEmptySectionNames()) {
       std::string SecName = ("." + DebugSecName).str();
       ImplicitSections.push_back(StringRef(SecName).copy(StringAlloc));
     }
-  ImplicitSections.insert(ImplicitSections.end(), {".strtab", ".shstrtab"});
+  ImplicitSections.insert(ImplicitSections.end(), {".strtab"});
+  if (!Doc.SectionHeaders || !Doc.SectionHeaders->NoHeaders.getValueOr(false))
+    ImplicitSections.insert(ImplicitSections.end(), {".shstrtab"});
 
   // Insert placeholders for implicit sections that are not
   // defined explicitly in YAML.
   for (StringRef SecName : ImplicitSections) {
     if (DocSections.count(SecName))
       continue;
 
     std::unique_ptr<ELFYAML::Chunk> Sec = std::make_unique<ELFYAML::Section>(
         ELFYAML::Chunk::ChunkKind::RawContent, true /*IsImplicit*/);
     Sec->Name = SecName;
     Doc.Chunks.push_back(std::move(Sec));
   }
 }
 
 template <class ELFT>
-void ELFState<ELFT>::writeELFHeader(raw_ostream &OS, uint64_t SHOff) {
+void ELFState<ELFT>::writeELFHeader(raw_ostream &OS, Optional<uint64_t> SHOff) {
   using namespace llvm::ELF;
 
   Elf_Ehdr Header;
   zero(Header);
   Header.e_ident[EI_MAG0] = 0x7f;
   Header.e_ident[EI_MAG1] = 'E';
   Header.e_ident[EI_MAG2] = 'L';
   Header.e_ident[EI_MAG3] = 'F';
void ELFState<ELFT>::writeELFHeader(raw_ostream &OS, Optional<uint64_t> SHOff) {
   else if (!Doc.ProgramHeaders.empty())
     Header.e_phnum = Doc.ProgramHeaders.size();
   else
     Header.e_phnum = 0;
 
   Header.e_shentsize = Doc.Header.EShEntSize ? (uint16_t)*Doc.Header.EShEntSize
                                              : sizeof(Elf_Shdr);
 
-  const bool NoShdrs =
-      Doc.SectionHeaders && Doc.SectionHeaders->NoHeaders.getValueOr(false);
-
   if (Doc.Header.EShOff)
     Header.e_shoff = *Doc.Header.EShOff;
-  else if (NoShdrs)
-    Header.e_shoff = 0;
+  else if (SHOff)
+    Header.e_shoff = *SHOff;
   else
-    Header.e_shoff = SHOff;
+    Header.e_shoff = 0;
 
   if (Doc.Header.EShNum)
     Header.e_shnum = *Doc.Header.EShNum;
   else if (!Doc.SectionHeaders ||
            (Doc.SectionHeaders->NoHeaders && !*Doc.SectionHeaders->NoHeaders))
     Header.e_shnum = Doc.getSections().size();
-  else if (NoShdrs)
+  else if (!SHOff)
     Header.e_shnum = 0;
   else
     Header.e_shnum =
         (Doc.SectionHeaders->Sections ? Doc.SectionHeaders->Sections->size()
                                       : 0) +
         /*Null section*/ 1;
 
   if (Doc.Header.EShStrNdx)
     Header.e_shstrndx = *Doc.Header.EShStrNdx;
-  else if (NoShdrs || ExcludedSectionHeaders.count(".shstrtab"))
-    Header.e_shstrndx = 0;
-  else
+  else if (SHOff && !ExcludedSectionHeaders.count(".shstrtab"))
     Header.e_shstrndx = SN2I.get(".shstrtab");
+  else
+    Header.e_shstrndx = 0;
 
   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)
bool ELFState<ELFT>::writeELF(raw_ostream &OS, ELFYAML::Object &Doc,
   ContiguousBlobAccumulator CBA(SectionContentBeginOffset, MaxSize);
 
   std::vector<Elf_Shdr> SHeaders;
   State.initSectionHeaders(SHeaders, CBA);
 
   // Now we can decide segment offsets.
   State.setProgramHeaderLayout(PHeaders, SHeaders);
 
-  // Align the start of the section header table, which is written after all
-  // section data.
-  uint64_t SHOff =
-      State.alignToOffset(CBA, sizeof(typename ELFT::uint), /*Offset=*/None);
-  bool ReachedLimit = SHOff + arrayDataSize(makeArrayRef(SHeaders)) > MaxSize;
+  // If needes, align the start of the section header table, which is written
+  // after all section data.
+  const bool HasSectionHeaders =
+      !Doc.SectionHeaders || !Doc.SectionHeaders->NoHeaders.getValueOr(false);
+  Optional<uint64_t> SHOff;
+  if (HasSectionHeaders)
+    SHOff = State.alignToOffset(CBA, sizeof(typename ELFT::uint),
+                                /*Offset=*/None);
+  bool ReachedLimit = SHOff.getValueOr(CBA.getOffset()) +
+                          arrayDataSize(makeArrayRef(SHeaders)) >
+                      MaxSize;
   if (Error E = CBA.takeLimitError()) {
     // We report a custom error message instead below.
     consumeError(std::move(E));
     ReachedLimit = true;
   }
 
   if (ReachedLimit)
     State.reportError(
         "the desired output size is greater than permitted. Use the "
         "--max-size option to change the limit");
 
   if (State.HasError)
     return false;
 
   State.writeELFHeader(OS, SHOff);
   writeArrayData(OS, makeArrayRef(PHeaders));
   CBA.writeBlobToStream(OS);
+  if (HasSectionHeaders)
     writeArrayData(OS, makeArrayRef(SHeaders));
   return true;
 }
 
 namespace llvm {
 namespace yaml {
 
 bool yaml2elf(llvm::ELFYAML::Object &Doc, raw_ostream &Out, ErrorHandler EH,
               uint64_t MaxSize) {

llvm/test/tools/llvm-readobj/ELF/malformed-pt-dynamic.test

 # WARN1-LLVM-NEXT:   0x0000000000000000 NULL 0x0
 # WARN1-LLVM-NEXT: ]
 
 # WARN1-GNU:       Dynamic section at offset 0x1000 contains 1 entries:
 # WARN1-GNU-NEXT:   Tag                Type   Name/Value
 # WARN1-GNU-NEXT:   0x0000000000000000 (NULL) 0x0
 
 ## Case A.2: in this case we drop section headers. The dynamic table is not dumped.
-# RUN: yaml2obj %s -DFILESIZE=0x119 -DNOHEADERS=true -o %t1.noheaders
+# RUN: yaml2obj %s -DFILESIZE=0x12 -DNOHEADERS=true -o %t1.noheaders
 # RUN: llvm-readobj %t1.noheaders --dynamic-table 2>&1 | FileCheck -DFILE=%t1.noheaders %s \
 # RUN:   --check-prefix=WARN1-NOHEADERS --implicit-check-not="DynamicSection ["
 # RUN: llvm-readelf %t1.noheaders --dynamic-table 2>&1 | FileCheck -DFILE=%t1.noheaders %s \
 # RUN:   --check-prefix=WARN1-NOHEADERS --implicit-check-not="Dynamic section"
 
-# WARN1-NOHEADERS: warning: '[[FILE]]': PT_DYNAMIC segment offset (0x1000) + file size (0x119) exceeds the size of the file (0x1118)
+# WARN1-NOHEADERS: warning: '[[FILE]]': PT_DYNAMIC segment offset (0x1000) + file size (0x12) exceeds the size of the file (0x1011)
 
 ## Case B: Test case where the offset of the PT_DYNAMIC header is too large to be in the file.
 
 ## Case B.1: the section header table is present in the object. Check that we report a warning about the
 ##           broken PT_DYNAMIC header. Check we also report a warning about broken fields of the SHT_DYNAMIC section.
 # RUN: yaml2obj %s -DOFFSET=0x1131 -o %t2
 # RUN: llvm-readobj %t2 --dynamic-table 2>&1 | FileCheck -DFILE=%t2 %s \
 # RUN:   --check-prefix=WARN2 --implicit-check-not=warning:
 # RUN: llvm-readelf %t2 --dynamic-table 2>&1 | FileCheck -DFILE=%t2 %s \
 # RUN:   --check-prefix=WARN2 --implicit-check-not=warning:
 
 # WARN2: warning: '[[FILE]]': PT_DYNAMIC segment offset (0x1131) + file size (0x10) exceeds the size of the file (0x1130)
 # WARN2: warning: '[[FILE]]': unable to read the dynamic table from SHT_DYNAMIC section with index 1: offset (0x1131) + size (0x10) is greater than the file size (0x1130)
 # WARN2: warning: '[[FILE]]': no valid dynamic table was found
 
 ## Case B.2: in this case we drop section headers. The dynamic table is not dumped.
-# RUN: yaml2obj %s -DOFFSET=0x1119 -DNOHEADERS=true -o %t2.noheaders
+# RUN: yaml2obj %s -DOFFSET=0x1112 -DNOHEADERS=true -o %t2.noheaders
 # RUN: llvm-readobj %t2.noheaders --dynamic-table 2>&1 | FileCheck -DFILE=%t2.noheaders %s \
 # RUN:   --check-prefix=WARN2-NOHEADERS --implicit-check-not="DynamicSection ["
 # RUN: llvm-readelf %t2.noheaders --dynamic-table 2>&1 | FileCheck -DFILE=%t2.noheaders %s \
 # RUN:   --check-prefix=WARN2-NOHEADERS --implicit-check-not="Dynamic section"
 
-# WARN2-NOHEADERS: warning: '[[FILE]]': PT_DYNAMIC segment offset (0x1119) + file size (0x10) exceeds the size of the file (0x1118)
+# WARN2-NOHEADERS: warning: '[[FILE]]': PT_DYNAMIC segment offset (0x1112) + file size (0x10) exceeds the size of the file (0x1011)
 
 ## Case C: test we report a warning when the offset + the file size of the PT_DYNAMIC is so large a
 ##         value that it overflows the platform address size type. Check we also report a warning about
 ##         broken fields of the SHT_DYNAMIC section.
 
 # RUN: yaml2obj %s -DOFFSET=0xffffffffffffffff -o %t3
 # RUN: llvm-readobj %t3 --dynamic-table 2>&1 | FileCheck -DFILE=%t3 %s \
 # RUN:   --check-prefix=WARN3 --implicit-check-not=warning:
 # RUN: llvm-readelf %t3 --dynamic-table 2>&1 | FileCheck -DFILE=%t3 %s \
 # RUN:   --check-prefix=WARN3 --implicit-check-not=warning:
 
 # WARN3: warning: '[[FILE]]': PT_DYNAMIC segment offset (0xffffffffffffffff) + file size (0x10) exceeds the size of the file (0x1130)
 # WARN3: warning: '[[FILE]]': unable to read the dynamic table from SHT_DYNAMIC section with index 1: offset (0xffffffffffffffff) + size (0x10) is greater than the file size (0x1130)
 # WARN3: warning: '[[FILE]]': no valid dynamic table was found
 
 # RUN: yaml2obj %s -DNOHEADERS=true -DOFFSET=0xffffffffffffffff -o %t3.noheaders
 # RUN: llvm-readobj %t3.noheaders --dynamic-table 2>&1 | \
 # RUN:   FileCheck -DFILE=%t3.noheaders %s --check-prefix=WARN3-NOHEADERS
 # RUN: llvm-readelf %t3.noheaders --dynamic-table 2>&1 | \
 # RUN:   FileCheck -DFILE=%t3.noheaders %s --check-prefix=WARN3-NOHEADERS
 
-# WARN3-NOHEADERS: warning: '[[FILE]]': PT_DYNAMIC segment offset (0xffffffffffffffff) + file size (0x10) exceeds the size of the file (0x1118)
+# WARN3-NOHEADERS: warning: '[[FILE]]': PT_DYNAMIC segment offset (0xffffffffffffffff) + file size (0x10) exceeds the size of the file (0x1011)
 
 # RUN: yaml2obj %s -DFILESIZE=0xffffffffffffffff -o %t4
 # RUN: llvm-readobj %t4 --dynamic-table 2>&1 | FileCheck -DFILE=%t4 %s --check-prefix=WARN4
 # RUN: llvm-readelf %t4 --dynamic-table 2>&1 | FileCheck -DFILE=%t4 %s --check-prefix=WARN4
 
 # WARN4: warning: '[[FILE]]': PT_DYNAMIC segment offset (0x1000) + file size (0xffffffffffffffff) exceeds the size of the file (0x1130)
 
 # RUN: yaml2obj %s -DNOHEADERS=true -DFILESIZE=0xffffffffffffffff -o %t4.noheaders
 # RUN: llvm-readobj %t4.noheaders --dynamic-table 2>&1 | \
 # RUN:   FileCheck -DFILE=%t4.noheaders %s --check-prefix=WARN4-NOHEADERS
 # RUN: llvm-readelf %t4.noheaders --dynamic-table 2>&1 | \
 # RUN:   FileCheck -DFILE=%t4.noheaders %s --check-prefix=WARN4-NOHEADERS
 
-# WARN4-NOHEADERS: warning: '[[FILE]]': PT_DYNAMIC segment offset (0x1000) + file size (0xffffffffffffffff) exceeds the size of the file (0x1118)
+# WARN4-NOHEADERS: warning: '[[FILE]]': PT_DYNAMIC segment offset (0x1000) + file size (0xffffffffffffffff) exceeds the size of the file (0x1011)
 
 ## Case D: the same as "Case C", but for a 32-bit object.
 
 # RUN: yaml2obj %s -DBITS=32 -DOFFSET=0xffffffff -o %t5
 # RUN: llvm-readobj %t5 --dynamic-table 2>&1 | FileCheck -DFILE=%t5 %s \
 # RUN:   --check-prefix=WARN5 --implicit-check-not=warning:
 # RUN: llvm-readelf %t5 --dynamic-table 2>&1 | FileCheck -DFILE=%t5 %s \
 # RUN:   --check-prefix=WARN5 --implicit-check-not=warning:
 
 # WARN5: warning: '[[FILE]]': PT_DYNAMIC segment offset (0xffffffff) + file size (0x8) exceeds the size of the file (0x10c8)
 # WARN5: warning: '[[FILE]]': unable to read the dynamic table from SHT_DYNAMIC section with index 1: offset (0xffffffff) + size (0x8) is greater than the file size (0x10c8)
 # WARN5: warning: '[[FILE]]': no valid dynamic table was found
 
 # RUN: yaml2obj %s -DNOHEADERS=true -DBITS=32 -DOFFSET=0xffffffff -o %t5.noheaders
 # RUN: llvm-readobj %t5.noheaders --dynamic-table 2>&1 | \
 # RUN:   FileCheck -DFILE=%t5.noheaders %s --check-prefix=WARN5-NOHEADERS
 # RUN: llvm-readelf %t5.noheaders --dynamic-table 2>&1 | \
 # RUN:   FileCheck -DFILE=%t5.noheaders %s --check-prefix=WARN5-NOHEADERS
 
-# WARN5-NOHEADERS: warning: '[[FILE]]': PT_DYNAMIC segment offset (0xffffffff) + file size (0x8) exceeds the size of the file (0x10ac)
+# WARN5-NOHEADERS: warning: '[[FILE]]': PT_DYNAMIC segment offset (0xffffffff) + file size (0x8) exceeds the size of the file (0x1009)
 
 # RUN: yaml2obj %s -DBITS=32 -DFILESIZE=0xffffffff -o %t6
 # RUN: llvm-readobj %t6 --dynamic-table 2>&1 | FileCheck -DFILE=%t6 %s --check-prefix=WARN6
 # RUN: llvm-readelf %t6 --dynamic-table 2>&1 | FileCheck -DFILE=%t6 %s --check-prefix=WARN6
 
 # WARN6: warning: '[[FILE]]': PT_DYNAMIC segment offset (0x1000) + file size (0xffffffff) exceeds the size of the file (0x10c8)
 
 # RUN: yaml2obj %s -DNOHEADERS=true -DBITS=32 -DFILESIZE=0xffffffff -o %t6.noheaders
 # RUN: llvm-readobj %t6.noheaders --dynamic-table 2>&1 | \
 # RUN:   FileCheck -DFILE=%t6.noheaders %s --check-prefix=WARN6-NOHEADERS
 # RUN: llvm-readelf %t6.noheaders --dynamic-table 2>&1 | \
 # RUN:   FileCheck -DFILE=%t6.noheaders %s --check-prefix=WARN6-NOHEADERS
 
-# WARN6-NOHEADERS: warning: '[[FILE]]': PT_DYNAMIC segment offset (0x1000) + file size (0xffffffff) exceeds the size of the file (0x10ac)
+# WARN6-NOHEADERS: warning: '[[FILE]]': PT_DYNAMIC segment offset (0x1000) + file size (0xffffffff) exceeds the size of the file (0x1009)
 
 --- !ELF
 FileHeader:
   Class: ELFCLASS[[BITS=64]]
   Data:  ELFDATA2LSB
   Type:  ET_EXEC
 Sections:
   - Name:     .dynamic

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

FileHeader:
   Data:      ELFDATA2LSB
   Type:      ET_REL
   EShOff:    0x2
   EShNum:    0x3
   EShStrNdx: 0x4
 Sections:
   - Name: .foo
     Type: SHT_PROGBITS
+## An arbitrary size to create a piece of dummy
+## data to make llvm-readelf happy.
+    Size: 0x100
 SectionHeaderTable:
   NoHeaders: true
 
 ## Check that section indices are updated properly in other places when we
 ## reorder sections in the section header table.
 # RUN: yaml2obj %s --docnum=7 -o %t5 -DSEC1=".foo" -DSEC2=".bar"
 # RUN: llvm-readelf --section-headers --symbols %t5 | FileCheck %s --check-prefix=INDICES-A
 # RUN: yaml2obj %s --docnum=7 -o %t6 -DSEC2=".foo" -DSEC1=".bar"
Sections:
     - Name: .symtab
     - Name: .strtab
     - Name: .shstrtab
 Symbols:
   - Name:    foo
     Section: .foo
   - Name:    bar
     Section: .bar
+
+## Check that when "NoHeaders" is set to "true" then we don't emit
+## the .shstrtab section implicitly and don't write the data of the
+## section header table to the file.
+
+# RUN: yaml2obj %s --docnum=8 -o %t8
+# RUN: wc -c < %t8 | FileCheck %s --check-prefix=SIZE
+
+# SIZE: 511{{$}}
+
+--- !ELF
+FileHeader:
+  Class: ELFCLASS64
+  Data:  ELFDATA2LSB
+  Type:  ET_REL
+Sections:
+## We don't want any implicit sections to be added after the .foo section,
+## so add them here explicitly.
+  - Name: .strtab
+    Type: SHT_STRTAB
+## Nothing should be emitted after the following section.
+## So we know that the expected file size is 0x100 + 0xFF == 0x1FF == 511.
+  - Name:   .foo
+    Type:   SHT_PROGBITS
+## Unaligned size. Used to make sure that we don't try to align the file offset
+## for writing the section header table.
+    Size:   0xFF
+    Offset: 0x100
+SectionHeaderTable:
+  NoHeaders: true