[PATCH] D63001: [yaml2obj] - Do not assert when .dynsym is specified explicitly, but .dynstr is not present.

[George Rimar via Phabricator via llvm-commits]

grimar created this revision.
grimar added a reviewer: jhenderson.
Herald added a subscriber: jakehehrlich.

We have a code in `buildSectionIndex()` that adds implicit sections:

  // Add special sections after input sections, if necessary.
  for (StringRef Name : implicitSectionNames())
    if (SN2I.addName(Name, SecNo)) {
      // Account for this section, since it wasn't in the Doc
      ++SecNo;
      DotShStrtab.add(Name);
    }

The problem arises when `.dynsym` is specified explicitly and no
`DynamicSymbols` is used. In that case, we do not add
`.dynstr` implicitly and will assert later when will try to set `Link`
for `.dynsym`.

Seems, in this case, reasonable behavior is to allow `Link` field to be zero.
This is what this patch does.


https://reviews.llvm.org/D63001

Files:
  test/tools/yaml2obj/explicit-dynsym-no-dynstr.yaml
  tools/yaml2obj/yaml2elf.cpp


Index: tools/yaml2obj/yaml2elf.cpp
===================================================================
--- tools/yaml2obj/yaml2elf.cpp
+++ tools/yaml2obj/yaml2elf.cpp
@@ -367,7 +367,17 @@
   bool IsStatic = STType == SymtabType::Static;
   SHeader.sh_name = DotShStrtab.getOffset(IsStatic ? ".symtab" : ".dynsym");
   SHeader.sh_type = IsStatic ? ELF::SHT_SYMTAB : ELF::SHT_DYNSYM;
-  SHeader.sh_link = IsStatic ? SN2I.get(".strtab") : SN2I.get(".dynstr");
+
+  // When we describe the .dynsym section in document explicitly, it is allowed
+  // to omit "DynamicSymbols" tag. In this case, .dynstr is not added implicitly
+  // and we should be able to leave the Link zeroed if .dynstr is not described.
+  unsigned Link = 0;
+  if (IsStatic)
+    Link = SN2I.get(".strtab");
+  else
+    SN2I.lookup(".dynstr", Link);
+  SHeader.sh_link = Link;
+
   if (!IsStatic)
     SHeader.sh_flags |= ELF::SHF_ALLOC;
 
Index: test/tools/yaml2obj/explicit-dynsym-no-dynstr.yaml
===================================================================
--- /dev/null
+++ test/tools/yaml2obj/explicit-dynsym-no-dynstr.yaml
@@ -0,0 +1,21 @@
+## Check we do not crash/assert when .dynsym is specified
+## explicitly, but .dynstr is not present.
+
+# RUN: yaml2obj %s -o %t
+# RUN: not llvm-readelf --section-headers %t 2>&1 | FileCheck %s
+
+## TODO: Check that .dynsym has Link field set to 0.
+##       GNU readelf is able to dump sections headers,
+##       but llvm-readelf report an error below too early.
+
+# CHECK: error: invalid sh_type for string table, expected SHT_STRTAB
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_DYN
+  Machine: EM_X86_64
+Sections:
+  - Name: .dynsym
+    Type: SHT_SYMTAB


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D63001.203524.patch
Type: text/x-patch
Size: 1728 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190607/92be02e4/attachment.bin>