[yaml2obj] - Make .symtab to be not mandatory section for SHT_REL[A] section.

[grimar]

Summary

Before this change .symtab section was required for SHT_REL[A] section
declarations. yaml2obj automatically defined it in case when YAML document
did not have it.

With this change it is now possible to produce an object that
has a relocation section, but has no symbol table.

It simplifies the code and also it is inline with how we handle Link fields
for another special sections.

[grimar]

• Rebased.

[jhenderson]

I'm not sure I follow why in most cases adding Symbols: [] is necessary? Could you explain the detailed impact of this change, with regards to when .symtab will/won't be generated and when the Link: .symtab is/is not required for relocation sections now?

llvm/test/tools/llvm-objcopy/ELF/no-symbol-relocation.test

24 ↗

(On Diff #226630)

Could you file a bug for this issue, if it doesn't already exist, please?

[grimar]

I'm not sure I follow why in most cases adding Symbols: [] is necessary? Could you explain the detailed impact of this change,
with regards to when .symtab will/won't be generated

Before this change we generated .symtab when there was the Symbols: tag in the YAML document and/or when there was a
REL[A] section that has an empty "Link" (i.e. normally it happened when it just was not specified).

After this change we only generate '.symtab' when there is Symbols: tag. When there is no 'Symbols:`, we do not generate it.
I.e. this patch lefts no more special handling for REL[A] sections.

and when the Link: .symtab is/is not required for relocation sections now?

So, if Link: .symtab is specified, you must provide the Symbols:, otherwise it will not find the referenced symbol table section.
Just like if you provide 'Link: .foo', you have to provide '.foo' section.

llvm/test/tools/llvm-objcopy/ELF/no-symbol-relocation.test

24 ↗

(On Diff #226630)

It will be fixed by D69304.

[grimar]

I am going to add an inline comment on each change where I added/removed "Symbols: []" or" Link: .symtab" to make it more clear why it was done.

[jhenderson]

In D69260#1723432, @grimar wrote:

I am going to add an inline comment on each change where I added/removed "Symbols: []" or" Link: .symtab" to make it more clear why it was done.

Thanks, I think that will be a bit clearer.

[grimar]

Here are comments for changes in test cases.

llvm/test/Object/invalid.test

336 ↗

(On Diff #226630)

Here we test the following error message:
"unable to access section [index 2] data at 0x18000040: offset goes past the end of file"

Without Symbols: [] yaml2obj does not create a .symtab.
.rela.text has sh_link == 0 then and we get a different error message instead of one we check:

"section [index 0] has invalid sh_entsize: expected 24, but got 0"

llvm/test/Object/objdump-sectionheaders.test

42 ↗

(On Diff #226630)

One of the lines we check is:
"# CHECK: 5 .symtab 00000018 0000000000000000"

I added "Symbols: []" to create the .symtab. Previously it was created by default because SHT_RELA has no Link specified.
This patch removes this special logic, so we need to specify "Symbols: []" explicitly.

llvm/test/tools/llvm-readobj/all.test

113 ↗

(On Diff #226630)

I added it to create the ".symtab" to test the "Symbol table '.symtab' contains {{.*}} entries:" line.

Previously it was created automatically because .rela.data had an empty Link field:
(i.e. had no field at all)

- Name: .rela.data
  Type: SHT_REL
  Relocations:

llvm/test/tools/llvm-readobj/elf-reloc-negative-addend-no-sym.test

32 ↗

(On Diff #226630)

With it this test would fail, since there is no symbol table anymore. It was created implicitly before, because of .rela.dyn section, which does not have Link specified.

We do not need a symbol table for this test, so instead of adding a "Symbols: []" line, I've just removed the reference.

llvm/test/tools/obj2yaml/elf-ppc64-relocations.yaml

15 ↗

(On Diff #226630)

The same as above. We do not create the symbol table by default for .rela.text and we do not need to have this reference to test the relocations here.

llvm/test/tools/obj2yaml/no-symbol-reloc.test

27 ↗

(On Diff #226630)

This is a cleanup change. We do not need a "Link" or a symbol table for this test.

llvm/test/tools/obj2yaml/relocation-unsupported-machine.yaml

15 ↗

(On Diff #226630)

The yaml below does not have "Symbols: []", and we do not create a symbol table by default for .rela.text anymore.
Hence, we do not set the sh_link too.

[jhenderson]

LGTM.

lib/ObjectYAML/ELFEmitter.cpp

 }
 
 template <class T> static void zero(T &Obj) { memset(&Obj, 0, sizeof(Obj)); }
 
 template <class ELFT>
 ELFState<ELFT>::ELFState(ELFYAML::Object &D, yaml::ErrorHandler EH)
     : Doc(D), ErrHandler(EH) {
   StringSet<> DocSections;
-  for (std::unique_ptr<ELFYAML::Section> &D : Doc.Sections) {
+  for (std::unique_ptr<ELFYAML::Section> &D : Doc.Sections)
     if (!D->Name.empty())
       DocSections.insert(D->Name);
 
-    // Some sections wants to link to .symtab by default.
-    // That means we want to create the symbol table for them.
-    if (D->Type == llvm::ELF::SHT_REL || D->Type == llvm::ELF::SHT_RELA)
-      if (!Doc.Symbols && D->Link.empty())
-        Doc.Symbols.emplace();
-  }
-
   // Insert SHT_NULL section implicitly when it is not defined in YAML.
   if (Doc.Sections.empty() || Doc.Sections.front()->Type != ELF::SHT_NULL)
     Doc.Sections.insert(
         Doc.Sections.begin(),
         std::make_unique<ELFYAML::Section>(
             ELFYAML::Section::SectionKind::RawContent, /*IsImplicit=*/true));
 
   std::vector<StringRef> ImplicitSections;
assert((Section.Type == llvm::ELF::SHT_REL ||
           Section.Type == llvm::ELF::SHT_RELA) &&
          "Section type is not SHT_REL nor SHT_RELA");
 
   bool IsRela = Section.Type == llvm::ELF::SHT_RELA;
   SHeader.sh_entsize = IsRela ? sizeof(Elf_Rela) : sizeof(Elf_Rel);
   SHeader.sh_size = SHeader.sh_entsize * Section.Relocations.size();
 
   // For relocation section set link to .symtab by default.
-  if (Section.Link.empty())
-    SHeader.sh_link = SN2I.get(".symtab");
+  unsigned Link = 0;
+  if (Section.Link.empty() && SN2I.lookup(".symtab", Link))
+    SHeader.sh_link = Link;
 
   if (!Section.RelocatableSec.empty())
     SHeader.sh_info = toSectionIndex(Section.RelocatableSec, Section.Name);
 
   auto &OS = CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign);
   for (const auto &Rel : Section.Relocations) {
     unsigned SymIdx = Rel.Symbol ? toSymbolIndex(*Rel.Symbol, Section.Name,
                                                  Section.Link == ".dynsym")

test/Object/invalid.test

 Sections:
   - Name: .rela.text
     Type: SHT_RELA
     Info: 0
     Relocations:
       - Offset: 0x0
         Type:   R_X86_64_64
         Symbol: 0xFFFFFF
+Symbols: []
 
 ## Check llvm-readobj does not crash on a truncated ELF.
 
 ## Create a truncated ELF object with ELFCLASSNONE class using echo.
 ## 0x7f, 'E', 'L', 'F', ELFCLASS64(2), ELFDATA2LSB(1),
 ## EV_CURRENT(1), ELFOSABI_LINUX(3), <padding zero bytes>, ET_REL(1), EM_NONE(0)
 # RUN: echo -e -n "\x7f\x45\x4c\x46\x02\x01\x01\x03\x00\x00\x00\x00\x00\x00\x00\x00\x01\x00" > %t11
 # RUN: not llvm-readobj -r %t11 2>&1 | FileCheck --check-prefix=INVALID-BUFFER %s

test/Object/objdump-sectionheaders.test

Sections:
   - Name:    .note.GNU-stack
     Type:    SHT_PROGBITS
     Address: 0x0000000000000033
   - Name:    .rela.text
     Type:    SHT_RELA
     Address: 0x0000000000000038
     Info:    .text
     Relocations:
+Symbols: []

test/tools/llvm-objcopy/ELF/no-symbol-relocation.test

- Name:            .text
     AddressAlign:    0x0000000000000010
     Content:         "0000000000000000"
   - Name:            .rel.text
     Type:            SHT_REL
     Info:            .text
     Relocations:
       - Offset: 0x1000
         Type:   R_X86_64_RELATIVE
+## TODO: llvm-objcopy crashes without the following line.
+Symbols: []
 
 # CHECK:     Relocations [
 # CHECK-NEXT:  Section (2) .rel.text {
 # CHECK-NEXT:    0x1000 R_X86_64_RELATIVE - 0x0
 # CHECK-NEXT:  }
 # CHECK-NEXT:]

test/tools/llvm-readobj/all.test

- Type: PT_DYNAMIC
     Sections:
       - Section: .dynamic
   - Type: PT_GNU_EH_FRAME
     Sections:
       - Section: .eh_frame_hdr
   - Type: PT_NOTE
     Sections:
       - Section: .note.gnu.build-id
+Symbols: []

test/tools/llvm-readobj/elf-reloc-negative-addend-no-sym.test

FileHeader:
   Type:    ET_EXEC
   Machine: EM_X86_64
 Sections:
   - Name: .text
     Type: SHT_PROGBITS
     Size: 0x10
   - Name: .rela.text
     Type: SHT_RELA
-    Link: .symtab
     Info: .text
     Relocations:
       - Offset: 0
         Type: R_X86_64_NONE
         Addend: -1
   - Name: .dynamic
     Type: SHT_DYNAMIC
     Flags: [SHF_ALLOC]

test/tools/obj2yaml/elf-ppc64-relocations.yaml

 ## Check obj2yaml is able to decode all PPC64 relocations.
 
 # RUN: yaml2obj %s -o %t
 # RUN: obj2yaml %t | FileCheck %s
 
 # CHECK:      --- !ELF
 # CHECK-NEXT: FileHeader:
 # CHECK-NEXT:   Class:   ELFCLASS64
 # CHECK-NEXT:   Data:    ELFDATA2MSB
 # CHECK-NEXT:   Type:    ET_REL
 # CHECK-NEXT:   Machine: EM_PPC64
 # CHECK-NEXT: Sections:
 # CHECK-NEXT:   - Name:    .rela.text
 # CHECK-NEXT:     Type:    SHT_RELA
-# CHECK-NEXT:     Link:    .symtab
 # CHECK-NEXT:     EntSize: 0x0000000000000018
 # CHECK-NEXT:     Relocations:
 # CHECK-NEXT:       - Offset: 0x0000000000000000
 # CHECK-NEXT:         Type:   R_PPC64_NONE
 # CHECK-NEXT:       - Offset: 0x0000000000000000
 # CHECK-NEXT:         Type:   R_PPC64_ADDR32
 # CHECK-NEXT:       - Offset: 0x0000000000000000
 # CHECK-NEXT:         Type:   R_PPC64_ADDR24

test/tools/obj2yaml/no-symbol-reloc.test

 # CHECK-NEXT:   Type:    ET_REL
 # CHECK-NEXT:   Machine: EM_X86_64
 # CHECK-NEXT: Sections:
 # CHECK-NEXT:   - Name:    .text
 # CHECK-NEXT:     Type:    SHT_PROGBITS
 # CHECK-NEXT:     Flags:   [ SHF_ALLOC, SHF_EXECINSTR ]
 # CHECK-NEXT:   - Name:    .rela.text
 # CHECK-NEXT:     Type:    SHT_RELA
-# CHECK-NEXT:     Link:    .symtab
 # CHECK-NEXT:     EntSize: 0x0000000000000018
 # CHECK-NEXT:     Info:    .text
 # CHECK-NEXT:     Relocations:
 # CHECK-NEXT:       - Offset: 0x0000000000000000
 # CHECK-NEXT:         Type:   R_X86_64_NONE
 # CHECK-NEXT:       - Offset: 0x0000000000000004
 # CHECK-NEXT:         Type:   R_X86_64_NONE
-# CHECK-NEXT: Symbols: []
 # CHECK-NEXT: ...
 
 --- !ELF
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_X86_64

test/tools/obj2yaml/relocation-unsupported-machine.yaml

 ## Show that obj2yaml is able to dump relocations when `e_machine` is unknown.
 
 # RUN: yaml2obj %s -o %t1
 # RUN: obj2yaml %t1 | FileCheck %s
 
 # CHECK:      --- !ELF
 # CHECK-NEXT: FileHeader:
 # CHECK-NEXT:   Class:   ELFCLASS64
 # CHECK-NEXT:   Data:    ELFDATA2MSB
 # CHECK-NEXT:   Type:    ET_REL
 # CHECK-NEXT:   Machine: 0x1234
 # CHECK-NEXT: Sections:
 # CHECK-NEXT:   - Name:    .rela.text
 # CHECK-NEXT:     Type:    SHT_RELA
-# CHECK-NEXT:     Link:    .symtab
 # CHECK-NEXT:     EntSize: 0x0000000000000018
 # CHECK-NEXT:     Relocations:
 # CHECK-NEXT:       - Offset: 0x0000000000000001
 # CHECK-NEXT:         Type:   0x00000002
 # CHECK-NEXT:       - Offset: 0x0000000000000003
 # CHECK-NEXT:         Type:   0x00000004
 
 --- !ELF

test/tools/yaml2obj/reloc-sec-info-link.yaml

Sections:
   - Name: .bar
     Type: SHT_RELA
     Info: .unknown2
     Relocations:
   - Name: .zed
     Type: SHT_RELA
     Info: 0xFF
     Relocations:
+
+## Check we set sh_link field of a relocation section to 0 when there is
+## no symbol table.
+
+# RUN: yaml2obj --docnum=3 %s -o %t3
+# RUN: llvm-readobj --sections %t3 | FileCheck %s --check-prefix=LINK-ZERO
+
+# LINK-ZERO: Name: .foo
+# LINK-ZERO: Link:
+# LINK-ZERO-SAME:  0
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_X86_64
+Sections:
+  - Name: .foo
+    Type: SHT_RELA
+
+## Check we link a relocation section with the .symtab section when one is exist.
+
+# RUN: yaml2obj --docnum=4 %s -o %t4
+# RUN: llvm-readobj --sections %t4 | FileCheck %s --check-prefix=LINK-SYMTAB
+
+# LINK-SYMTAB: Name: .foo
+# LINK-SYMTAB: Link:
+# LINK-SYMTAB-SAME:  2
+
+# LINK-SYMTAB:      Index: 3
+# LINK-SYMTAB-NEXT: Name: .strtab
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_X86_64
+Sections:
+  - Name: .foo
+    Type: SHT_RELA
+Symbols: []