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

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. Differential revision: https://reviews.llvm.org/D90295
llvm · Oct 29, 2020 · fcf6287 · fcf6287
1 parent a4b6b1e
commit fcf6287
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 27 deletions.
diff --git a/llvm/lib/ObjectYAML/ELFEmitter.cpp b/llvm/lib/ObjectYAML/ELFEmitter.cpp
@@ -232,7 +232,7 @@ template <class ELFT> class ELFState {
                    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);
@@ -363,7 +363,9 @@ ELFState<ELFT>::ELFState(ELFYAML::Object &D, yaml::ErrorHandler EH)
       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.
@@ -379,7 +381,7 @@ ELFState<ELFT>::ELFState(ELFYAML::Object &D, yaml::ErrorHandler EH)
 }
 
 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;
@@ -429,22 +431,19 @@ void ELFState<ELFT>::writeELFHeader(raw_ostream &OS, uint64_t SHOff) {
   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 =
@@ -454,10 +453,10 @@ void ELFState<ELFT>::writeELFHeader(raw_ostream &OS, uint64_t SHOff) {
 
   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));
 }
@@ -1884,11 +1883,17 @@ bool ELFState<ELFT>::writeELF(raw_ostream &OS, ELFYAML::Object &Doc,
   // 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 needed, 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));
@@ -1906,7 +1911,8 @@ bool ELFState<ELFT>::writeELF(raw_ostream &OS, ELFYAML::Object &Doc,
   State.writeELFHeader(OS, SHOff);
   writeArrayData(OS, makeArrayRef(PHeaders));
   CBA.writeBlobToStream(OS);
-  writeArrayData(OS, makeArrayRef(SHeaders));
+  if (HasSectionHeaders)
+    writeArrayData(OS, makeArrayRef(SHeaders));
   return true;
 }
 

diff --git a/llvm/test/tools/llvm-readobj/ELF/malformed-pt-dynamic.test b/llvm/test/tools/llvm-readobj/ELF/malformed-pt-dynamic.test
@@ -22,13 +22,13 @@
 # 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.
 
@@ -45,13 +45,13 @@
 # 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
@@ -73,7 +73,7 @@
 # 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
@@ -87,7 +87,7 @@
 # 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.
 
@@ -107,7 +107,7 @@
 # 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
@@ -121,7 +121,7 @@
 # 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:

diff --git a/llvm/test/tools/yaml2obj/ELF/section-headers.yaml b/llvm/test/tools/yaml2obj/ELF/section-headers.yaml
@@ -191,6 +191,10 @@ FileHeader:
 Sections:
   - Name: .foo
     Type: SHT_PROGBITS
+## FIXME: we have to set an arbitrary size to create a
+## piece of dummy data to make llvm-readelf happy.
+## See: https://bugs.llvm.org/show_bug.cgi?id=40804
+    Size: 0x100
 SectionHeaderTable:
   NoHeaders: true
 
@@ -250,3 +254,33 @@ Symbols:
     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