[yaml2obj] - Make DynamicSymbols to be Optional<> too.

We already have Symbols property to list regular symbols and it is currently Optional<>. This patch makes DynamicSymbols to be optional too. With this there is no need to define a dummy symbol anymore to trigger creation of the .dynsym and it is now possible to define an empty .dynsym using just the following line: DynamicSymbols: [] (it is important to have when you do not want to have dynamic symbols, but want to have a .dynsym) Now the code is consistent and it helped to fix a bug: previously we did not report an error when both Content/Size and an empty Symbols/DynamicSymbols list were specified. Differential revision: https://reviews.llvm.org/D70956
llvm · Dec 4, 2019 · daff7b8 · daff7b8
1 parent 150c8dd
commit daff7b8
Show file tree

Hide file tree

Showing 9 changed files with 109 additions and 54 deletions.
diff --git a/llvm/include/llvm/ObjectYAML/ELFYAML.h b/llvm/include/llvm/ObjectYAML/ELFYAML.h
@@ -481,7 +481,7 @@ struct Object {
   // top-level key, which automatically ensures that invariants like there
   // being a single SHT_SYMTAB section are upheld.
   Optional<std::vector<Symbol>> Symbols;
-  std::vector<Symbol> DynamicSymbols;
+  Optional<std::vector<Symbol>> DynamicSymbols;
 
   std::vector<Section *> getSections() {
     std::vector<Section *> Ret;

diff --git a/llvm/lib/ObjectYAML/ELFEmitter.cpp b/llvm/lib/ObjectYAML/ELFEmitter.cpp
@@ -248,7 +248,7 @@ ELFState<ELFT>::ELFState(ELFYAML::Object &D, yaml::ErrorHandler EH)
     ImplicitSections.push_back(".symtab");
   ImplicitSections.insert(ImplicitSections.end(), {".strtab", ".shstrtab"});
 
-  if (!Doc.DynamicSymbols.empty())
+  if (Doc.DynamicSymbols)
     ImplicitSections.insert(ImplicitSections.end(), {".dynsym", ".dynstr"});
 
   // Insert placeholders for implicit sections that are not
@@ -562,21 +562,24 @@ void ELFState<ELFT>::initSymtabSectionHeader(Elf_Shdr &SHeader,
   ArrayRef<ELFYAML::Symbol> Symbols;
   if (IsStatic && Doc.Symbols)
     Symbols = *Doc.Symbols;
-  else if (!IsStatic)
-    Symbols = Doc.DynamicSymbols;
+  else if (!IsStatic && Doc.DynamicSymbols)
+    Symbols = *Doc.DynamicSymbols;
 
   ELFYAML::RawContentSection *RawSec =
       dyn_cast_or_null<ELFYAML::RawContentSection>(YAMLSec);
-  if (RawSec && !Symbols.empty() && (RawSec->Content || RawSec->Size)) {
-    if (RawSec->Content)
-      reportError("cannot specify both `Content` and " +
-                  (IsStatic ? Twine("`Symbols`") : Twine("`DynamicSymbols`")) +
-                  " for symbol table section '" + RawSec->Name + "'");
-    if (RawSec->Size)
-      reportError("cannot specify both `Size` and " +
-                  (IsStatic ? Twine("`Symbols`") : Twine("`DynamicSymbols`")) +
-                  " for symbol table section '" + RawSec->Name + "'");
-    return;
+  if (RawSec && (RawSec->Content || RawSec->Size)) {
+    bool HasSymbolsDescription =
+        (IsStatic && Doc.Symbols) || (!IsStatic && Doc.DynamicSymbols);
+    if (HasSymbolsDescription) {
+      StringRef Property = (IsStatic ? "`Symbols`" : "`DynamicSymbols`");
+      if (RawSec->Content)
+        reportError("cannot specify both `Content` and " + Property +
+                    " for symbol table section '" + RawSec->Name + "'");
+      if (RawSec->Size)
+        reportError("cannot specify both `Size` and " + Property +
+                    " for symbol table section '" + RawSec->Name + "'");
+      return;
+    }
   }
 
   zero(SHeader);
@@ -1334,7 +1337,8 @@ template <class ELFT> void ELFState<ELFT>::buildSymbolIndexes() {
 
   if (Doc.Symbols)
     Build(*Doc.Symbols, SymN2I);
-  Build(Doc.DynamicSymbols, DynSymN2I);
+  if (Doc.DynamicSymbols)
+    Build(*Doc.DynamicSymbols, DynSymN2I);
 }
 
 template <class ELFT> void ELFState<ELFT>::finalizeStrings() {
@@ -1345,8 +1349,9 @@ template <class ELFT> void ELFState<ELFT>::finalizeStrings() {
   DotStrtab.finalize();
 
   // Add the dynamic symbol names to .dynstr section.
-  for (const ELFYAML::Symbol &Sym : Doc.DynamicSymbols)
-    DotDynstr.add(ELFYAML::dropUniqueSuffix(Sym.Name));
+  if (Doc.DynamicSymbols)
+    for (const ELFYAML::Symbol &Sym : *Doc.DynamicSymbols)
+      DotDynstr.add(ELFYAML::dropUniqueSuffix(Sym.Name));
 
   // SHT_GNU_verdef and SHT_GNU_verneed sections might also
   // add strings to .dynstr section.

diff --git a/llvm/test/tools/yaml2obj/ELF/dynsymtab-implicit-sections-size-content.yaml b/llvm/test/tools/yaml2obj/ELF/dynsymtab-implicit-sections-size-content.yaml
@@ -30,6 +30,7 @@ DynamicSymbols:
 ## Specifying both `Size` and symbols at the same time is not allowed for .dynsym.
 
 # RUN: not yaml2obj --docnum=2 %s 2>&1 | FileCheck %s --implicit-check-not=error --check-prefix=CASE2
+# RUN: not yaml2obj --docnum=3 %s 2>&1 | FileCheck %s --implicit-check-not=error --check-prefix=CASE2
 
 # CASE2: yaml2obj: error: cannot specify both `Size` and `DynamicSymbols` for symbol table section '.dynsym'
 
@@ -50,9 +51,25 @@ DynamicSymbols:
   - Name:    foo
     Binding: STB_GLOBAL
 
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name: .dynsym
+    Type: SHT_DYNSYM
+    Size: 0x100
+  - Name: .dynsym2
+    Type: SHT_DYNSYM
+    Size: 0x100
+DynamicSymbols: []
+
 ## Specifying both `Content` and symbols at the same time is not allowed for .dynsym.
 
-# RUN: not yaml2obj --docnum=3 %s 2>&1 | FileCheck %s --implicit-check-not=error --check-prefix=CASE3
+# RUN: not yaml2obj --docnum=4 %s 2>&1 | FileCheck %s --implicit-check-not=error --check-prefix=CASE3
+# RUN: not yaml2obj --docnum=5 %s 2>&1 | FileCheck %s --implicit-check-not=error --check-prefix=CASE3
 
 # CASE3: yaml2obj: error: cannot specify both `Content` and `DynamicSymbols` for symbol table section '.dynsym'
 
@@ -63,18 +80,33 @@ FileHeader:
   Type:    ET_DYN
   Machine: EM_X86_64
 Sections:
-  - Name: .dynsym
-    Type: SHT_DYNSYM
+  - Name:    .dynsym
+    Type:    SHT_DYNSYM
     Content: "00"
-  - Name: .dynsym2
-    Type: SHT_DYNSYM
+  - Name:    .dynsym2
+    Type:    SHT_DYNSYM
     Content: "00"
 DynamicSymbols:
   - Name:    foo
     Binding: STB_GLOBAL
 
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:    .dynsym
+    Type:    SHT_DYNSYM
+    Content: "00"
+  - Name:    .dynsym2
+    Type:    SHT_DYNSYM
+    Content: "00"
+DynamicSymbols: []
+
 ## Check we can use just `Content` to emit custom data in the symbol table section.
-# RUN: yaml2obj --docnum=4 %s -o %t4
+# RUN: yaml2obj --docnum=6 %s -o %t4
 # RUN: llvm-readobj --section-data -S %t4 | FileCheck %s --check-prefix=CASE4
 
 # CASE4:      Name: .dynsym
@@ -106,7 +138,7 @@ Sections:
 
 ## Check we can use just `Size` to emit custom data filled with zeroes
 ## in the symbol table section.
-# RUN: yaml2obj --docnum=5 %s -o %t5
+# RUN: yaml2obj --docnum=7 %s -o %t5
 # RUN: llvm-readobj --section-data -S %t5 | FileCheck %s --check-prefix=CASE5
 
 # CASE5:      Name: .dynsym
@@ -140,7 +172,7 @@ Sections:
 ## than content size. In this case zeroes are added as padding
 ## after after the specified content.
 
-# RUN: yaml2obj --docnum=6 %s -o %t6
+# RUN: yaml2obj --docnum=8 %s -o %t6
 # RUN: llvm-readobj %t6 --section-data -S | FileCheck %s --check-prefix=CASE6
 
 # CASE6:      Name: .dynsym
@@ -174,7 +206,7 @@ Sections:
 ## Check we can specify both `Size` and `Content` when size is
 ## equal to content size.
 
-# RUN: yaml2obj --docnum=7 %s -o %t7
+# RUN: yaml2obj --docnum=9 %s -o %t7
 # RUN: llvm-readobj --section-data -S %t7 | FileCheck %s --check-prefix=CASE7
 
 # CASE7:       Name: .dynsym

diff --git a/llvm/test/tools/yaml2obj/ELF/gnu-hash-section.yaml b/llvm/test/tools/yaml2obj/ELF/gnu-hash-section.yaml
@@ -32,9 +32,7 @@ Sections:
     Type:  SHT_GNU_HASH
     Content: "001122"
 ## Used to trigger .dynsym creation.
-DynamicSymbols:
-  - Name:    foo
-    Binding: STB_GLOBAL
+DynamicSymbols: []
 
 ## Check we can use "Header", "BloomFilter", "HashBuckets" and "HashValues" keys to describe
 ## the hash section. Check we can set sh_link to any arbitrary value. Check both ELFCLASS32 and 64 bit output.

diff --git a/llvm/test/tools/yaml2obj/ELF/implicit-sections-addr.yaml b/llvm/test/tools/yaml2obj/ELF/implicit-sections-addr.yaml
@@ -52,6 +52,4 @@ Sections:
   - Name:    .symtab
     Type:    SHT_SYMTAB
     Address: 0x4000
-DynamicSymbols:
-  - Name:    foo
-    Binding: STB_GLOBAL
+DynamicSymbols: []
diff --git a/llvm/test/tools/yaml2obj/ELF/implicit-sections-types.yaml b/llvm/test/tools/yaml2obj/ELF/implicit-sections-types.yaml
@@ -27,9 +27,7 @@ FileHeader:
 ## Needed to force the creation of the .symtab.
 Symbols: []
 ## Needed to force the creation of the .dynsym and .dynstr.
-DynamicSymbols:
-  - Name:    foo
-  - Binding: STB_GLOBAL
+DynamicSymbols: []
 
 ## Check we can set any arbitrary types when describing sections
 ## that are usually implicit.

diff --git a/llvm/test/tools/yaml2obj/ELF/implicit-sections.yaml b/llvm/test/tools/yaml2obj/ELF/implicit-sections.yaml
@@ -11,12 +11,12 @@
 # CHECK:      Section Headers:
 # CHECK-NEXT:  [Nr] Name      Type     Address          Off    Size
 # CHECK-NEXT:  [ 0]           NULL     0000000000000000 000000 000000
-# CHECK-NEXT:  [ 1] .dynstr   STRTAB   0000000000000100 000040 000009
-# CHECK-NEXT:  [ 2] .dynsym   DYNSYM   0000000000000150 000049 000030
-# CHECK-NEXT:  [ 3] .symtab   SYMTAB   0000000000000000 000079 000018
-# CHECK-NEXT:  [ 4] .strtab   STRTAB   0000000000000000 000091 000001
-# CHECK-NEXT:  [ 5] .shstrtab STRTAB   0000000000000000 000092 000035
-# CHECK-NEXT:  [ 6] .text.foo PROGBITS 0000000000000200 0000c7 000000
+# CHECK-NEXT:  [ 1] .dynstr   STRTAB   0000000000000100 000040 000001
+# CHECK-NEXT:  [ 2] .dynsym   DYNSYM   0000000000000150 000041 000018
+# CHECK-NEXT:  [ 3] .symtab   SYMTAB   0000000000000000 000059 000018
+# CHECK-NEXT:  [ 4] .strtab   STRTAB   0000000000000000 000071 000001
+# CHECK-NEXT:  [ 5] .shstrtab STRTAB   0000000000000000 000072 000035
+# CHECK-NEXT:  [ 6] .text.foo PROGBITS 0000000000000200 0000a7 000000
 
 --- !ELF
 FileHeader:
@@ -40,10 +40,6 @@ Sections:
   - Name:    .text.foo
     Type:    SHT_PROGBITS
     Address: 0x200
-## Symbol is required for the .dynsym to be generated.
-DynamicSymbols:
-  - Name:    _Z3fooi
-    Binding: STB_GLOBAL
 
 ## Check that yaml2obj creates empty .dynstr and .dynsym sections for
 ## the case when no dynamic symbols were specified and Content wasn't set,

diff --git a/llvm/test/tools/yaml2obj/ELF/symtab-implicit-sections-size-content.yaml b/llvm/test/tools/yaml2obj/ELF/symtab-implicit-sections-size-content.yaml
@@ -27,6 +27,7 @@ Symbols:
 
 ## Specifying both `Size` and symbols at the same time is not allowed.
 # RUN: not yaml2obj --docnum=2 %s -o %t2 2>&1 | FileCheck %s --check-prefix=CASE2
+# RUN: not yaml2obj --docnum=3 %s -o %t2 2>&1 | FileCheck %s --check-prefix=CASE2
 
 # CASE2: error: cannot specify both `Size` and `Symbols` for symbol table section '.symtab'
 
@@ -43,8 +44,21 @@ Sections:
 Symbols:
   - Name: foo
 
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name: .symtab
+    Type: SHT_SYMTAB
+    Size: 0x100
+Symbols: []
+
 ## Specifying both `Content` and symbols at the same time is not allowed.
-# RUN: not yaml2obj --docnum=3 %s -o %t3 2>&1 | FileCheck %s --check-prefix=CASE3
+# RUN: not yaml2obj --docnum=4 %s -o %t3 2>&1 | FileCheck %s --check-prefix=CASE3
+# RUN: not yaml2obj --docnum=5 %s -o %t3 2>&1 | FileCheck %s --check-prefix=CASE3
 
 # CASE3: error: cannot specify both `Content` and `Symbols` for symbol table section '.symtab'
 
@@ -55,14 +69,26 @@ FileHeader:
   Type:    ET_DYN
   Machine: EM_X86_64
 Sections:
-  - Name: .symtab
-    Type: SHT_SYMTAB
+  - Name:    .symtab
+    Type:    SHT_SYMTAB
     Content: "00"
 Symbols:
   - Name: foo
 
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name:    .symtab
+    Type:    SHT_SYMTAB
+    Content: "00"
+Symbols: []
+
 ## Check we can use just `Content` to emit custom data in the symbol table section.
-# RUN: yaml2obj --docnum=4 %s -o %t4
+# RUN: yaml2obj --docnum=6 %s -o %t4
 # RUN: llvm-readobj --section-data -S %t4 | FileCheck %s --check-prefix=CASE4
 
 # CASE4:      Name: .symtab
@@ -93,7 +119,7 @@ Sections:
 
 ## Check we can use just `Size` to emit custom data filled with zeroes
 ## in the symbol table section.
-# RUN: yaml2obj --docnum=5 %s -o %t5
+# RUN: yaml2obj --docnum=7 %s -o %t5
 # RUN: llvm-readobj --section-data -S %t5 | FileCheck %s --check-prefix=CASE5
 
 # CASE5:      Name: .symtab (19)
@@ -126,7 +152,7 @@ Sections:
 ## than content size. In this case zeroes are added as padding
 ## after the specified content.
 
-# RUN: yaml2obj --docnum=6 %s -o %t6
+# RUN: yaml2obj --docnum=8 %s -o %t6
 # RUN: llvm-readobj %t6 --section-data -S | FileCheck %s --check-prefix=CASE6
 
 # CASE6:      Name: .symtab
@@ -159,7 +185,7 @@ Sections:
 ## Check we can specify both `Size` and `Content` when size is
 ## equal to content size.
 
-# RUN: yaml2obj --docnum=7 %s -o %t7
+# RUN: yaml2obj --docnum=9 %s -o %t7
 # RUN: llvm-readobj --section-data -S %t7 | FileCheck %s --check-prefix=CASE7
 
 # CASE7:       Name: .symtab

diff --git a/llvm/tools/obj2yaml/elf2yaml.cpp b/llvm/tools/obj2yaml/elf2yaml.cpp
@@ -215,9 +215,11 @@ template <class ELFT> Expected<ELFYAML::Object *> ELFDumper<ELFT>::dump() {
       return std::move(E);
   }
 
-  if (DynSymTab)
-    if (Error E = dumpSymbols(DynSymTab, Y->DynamicSymbols))
+  if (DynSymTab) {
+    Y->DynamicSymbols.emplace();
+    if (Error E = dumpSymbols(DynSymTab, *Y->DynamicSymbols))
       return std::move(E);
+  }
 
   for (const Elf_Shdr &Sec : Sections) {
     switch (Sec.sh_type) {