[yaml2obj] - Do not ignore explicit addresses for .dynsym and .dynstr

[grimar]

Summary

This fixes https://bugs.llvm.org/show_bug.cgi?id=40339

Previously if the addresses were set in YAML they were ignored for
.dynsym and .dynstr sections. The patch fixes that.

[grimar]

test/tools/yaml2obj/dynsym-dynstr-addr.yaml

4 ↗

(On Diff #186608)

I will change soes->does on next patch update/before the commit.

[jhenderson]

test/tools/yaml2obj/dynsym-dynstr-addr.yaml

5 ↗

(On Diff #186608)

Nit: .dynstr -> .dynsym in one case

tools/yaml2obj/yaml2elf.cpp

324–325 ↗

(On Diff #186608)

Nit: Let clang-format reflow your comments across multiple lines, rather than doing it by hand.

377 ↗

(On Diff #186608)

Do you think this should have a sanity assertion to show that Doc.Sections[SecNdx] is the right section? If I'm right, this only works because an auto-generated .dynstr happens to be added to the end of the sections, but it would not be unreasonable to change that behaviour.

[grimar]

tools/yaml2obj/yaml2elf.cpp

324–325 ↗

(On Diff #186608)

Clang format would produce the following:

// If .dynsym section is explicitly described in the YAML then we want to use
// its section address.

The first line is long, while the second is too short. It does not look nice. So you still want me to do that?
( I am often doing that by hand in such cases, btw).

377 ↗

(On Diff #186608)

I am using getDotDynStrSecNo() here to get the section index.
It implementation just maps a section name to index:
https://github.com/llvm-mirror/llvm/blob/master/tools/yaml2obj/yaml2elf.cpp#L178

We build the SN2I mapping in buildSectionIndex here:
https://github.com/llvm-mirror/llvm/blob/master/tools/yaml2obj/yaml2elf.cpp#L605
(i.e. before calling this place)

So ordering is not important I believe.

[grimar]

tools/yaml2obj/yaml2elf.cpp

324–325 ↗

(On Diff #186608)

"So you" -> "Do you"

[grimar]

• Fix comment in the test case.

[jhenderson]

LGTM.

tools/yaml2obj/yaml2elf.cpp

324–325 ↗

(On Diff #186608)

I guess it's a personal style thing. It just looks weird when I see short lines in comments. I don't feel strongly about it if you don't want to do it.

377 ↗

(On Diff #186608)

Never mind, I think I misread the code the first time.

llvm/trunk/test/tools/yaml2obj/dynsym-dynstr-addr.yaml

+# RUN: yaml2obj %s -o %t
+# RUN: llvm-readobj -sections %t | FileCheck %s
+
+## Check yaml2obj does not ignore the address of the
+## explicitly listed .dynstr and .dynsym sections.
+
+# CHECK:      Name: .dynstr
+# CHECK-NEXT:   Type: SHT_STRTAB
+# CHECK-NEXT:   Flags [
+# CHECK-NEXT:     SHF_ALLOC
+# CHECK-NEXT:   ]
+# CHECK-NEXT:   Address: 0x1000
+
+# CHECK:      Name: .dynsym
+# CHECK-NEXT:   Type: SHT_DYNSYM
+# CHECK-NEXT:   Flags [
+# CHECK-NEXT:     SHF_ALLOC
+# CHECK-NEXT:   ]
+# CHECK-NEXT:   Address: 0x2000
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_DYN
+  Machine:         EM_X86_64
+Sections:
+  - Name:         .dynstr
+    Type:         SHT_STRTAB
+    Flags:        [ SHF_ALLOC ]
+    Address:      0x1000
+    EntSize:      0x1
+  - Name:         .dynsym
+    Type:         SHT_DYNSYM
+    Flags:        [ SHF_ALLOC ]
+    Address:      0x2000
+    EntSize:      0x18
+DynamicSymbols:
+  Global:
+    - Name: foo

llvm/trunk/tools/yaml2obj/yaml2elf.cpp

void ELFState<ELFT>::initSymtabSectionHeader(Elf_Shdr &SHeader,
   SHeader.sh_link = IsStatic ? getDotStrTabSecNo() : getDotDynStrSecNo();
   const auto &Symbols = IsStatic ? Doc.Symbols : Doc.DynamicSymbols;
   auto &Strtab = IsStatic ? DotStrtab : DotDynstr;
   // One greater than symbol table index of the last local symbol.
   SHeader.sh_info = Symbols.Local.size() + 1;
   SHeader.sh_entsize = sizeof(Elf_Sym);
   SHeader.sh_addralign = 8;
 
+  // If .dynsym section is explicitly described in the YAML
+  // then we want to use its section address.
+  if (!IsStatic) {
+    // Take section index and ignore the SHT_NULL section.
+    unsigned SecNdx = getDotDynSymSecNo() - 1;
+    if (SecNdx < Doc.Sections.size())
+      SHeader.sh_addr = Doc.Sections[SecNdx]->Address;
+  }
+
   std::vector<Elf_Sym> Syms;
   {
     // Ensure STN_UNDEF is present
     Elf_Sym Sym;
     zero(Sym);
     Syms.push_back(Sym);
   }
 
void ELFState<ELFT>::initStrtabSectionHeader(Elf_Shdr &SHeader, StringRef Name,
                                              StringTableBuilder &STB,
                                              ContiguousBlobAccumulator &CBA) {
   zero(SHeader);
   SHeader.sh_name = DotShStrtab.getOffset(Name);
   SHeader.sh_type = ELF::SHT_STRTAB;
   STB.write(CBA.getOSAndAlignedOffset(SHeader.sh_offset, SHeader.sh_addralign));
   SHeader.sh_size = STB.getSize();
   SHeader.sh_addralign = 1;
+
+  // If .dynstr section is explicitly described in the YAML
+  // then we want to use its section address.
+  if (Name == ".dynstr") {
+    // Take section index and ignore the SHT_NULL section.
+    unsigned SecNdx = getDotDynStrSecNo() - 1;
+    if (SecNdx < Doc.Sections.size())
+      SHeader.sh_addr = Doc.Sections[SecNdx]->Address;
+  }
 }
 
 template <class ELFT>
 void ELFState<ELFT>::setProgramHeaderLayout(std::vector<Elf_Phdr> &PHeaders,
                                             std::vector<Elf_Shdr> &SHeaders) {
   uint32_t PhdrIdx = 0;
   for (auto &YamlPhdr : Doc.ProgramHeaders) {
     auto &PHeader = PHeaders[PhdrIdx++];