[ELF] - Implemented version script hierarchies.

[grimar]

Summary

Patch implements hierarchies for version scripts.
This allows to handle script files with dependencies, like next one has:

LIBSAMPLE_1.0{
  global:
  a;
};

LIBSAMPLE_2.0
{
  global:
  b;
}LIBSAMPLE_1.0;

[grimar]

• Improved testcase.

[davide]

ELF/OutputSections.cpp

1510 ↗

(On Diff #61374)

It could be just me, but I'd rather prefer this -1 to be assigned to a variable or adding a comment to explain what this code does.

[ruiu]

ELF/OutputSections.cpp

1478 ↗

(On Diff #61374)

static size_t?

ELF/SymbolListFile.cpp

97 ↗

(On Diff #61374)

I'd do

if (!Version.empty() && peek() != ";")
  Config->SymbolVersions.back().Dependency = next();
expect(";");

[grimar]

• Addressed review comments.
• Renamed "dependency" to "parent"

ELF/OutputSections.cpp

1478 ↗

(On Diff #61374)

Right..

1510 ↗

(On Diff #61374)

Ok, added comment. It is in common in versioning relative methods around here, btw.

ELF/SymbolListFile.cpp

97 ↗

(On Diff #61374)

Done.

[ruiu]

LGTM with a few nits.

ELF/Config.h

39 ↗

(On Diff #61517)

Yup, I think this is a better name. I'd move this line after Name since Name is more important.

ELF/OutputSections.cpp

1510 ↗

(On Diff #61517)

I don't think this comment helps reader to understand this piece of code. Instead, we could avoid [-1] by doing this, but I'd think your code is easier to read. So I'd just do without comment.

Verdaux->vda_name = StrTabOffset;
if (HasParent) {
  Verdaux->vda_next = sizeof(Elf_Verdaux);
  ++Verdaux;
  Verdaux->vda_name = getVersionNameStrTabOffset(ParentName);
  Verdaux->vda_next = 0;
} else {
  Verdaux->vda_next = 0;
}
++Verdaux;

One thing I would do is to not do early return in this function. I believe this is easier to read in this case.

1524 ↗

(On Diff #61517)

Update comment.

[grimar]

Ok, thanks !
I`ll commit right after D21552 be landed as this one's testcase relies on it heavily.

[grimar]

ELF/OutputSections.cpp

1510 ↗

(On Diff #61517)

btw, may be ?

Verdaux->vda_name = StrTabOffset;
if (HasParent) {
  Verdaux->vda_next = sizeof(Elf_Verdaux);
  ++Verdaux;
  Verdaux->vda_name = getVersionNameStrTabOffset(ParentName);
} 

Verdaux->vda_next = 0;
++Verdaux;

[ruiu]

ELF/OutputSections.cpp

1510 ↗

(On Diff #61517)

It might be easier to read, but I'm okay with either one. It's up to you.

lld/trunk/ELF/Config.h

 
 enum class BuildIdKind { None, Fnv1, Md5, Sha1, Hexstring };
 
 // This struct contains symbols version definition that
 // can be found in version script if it is used for link.
 struct Version {
   Version(llvm::StringRef Name) : Name(Name) {}
   llvm::StringRef Name;
+  llvm::StringRef Parent;
   std::vector<llvm::StringRef> Globals;
   size_t NameOff; // Offset in string table.
 };
 
 // This struct contains the global configuration for the linker.
 // Most fields are direct mapping from the command line options
 // and such fields have the same name as the corresponding options.
 // Most fields are initialized by the driver.

lld/trunk/ELF/OutputSections.cpp

 
 template <class ELFT> void VersionDefinitionSection<ELFT>::finalize() {
   FileDefNameOff = Out<ELFT>::DynStrTab->addString(getFileDefName());
   for (Version &V : Config->SymbolVersions)
     V.NameOff = Out<ELFT>::DynStrTab->addString(V.Name);
 
   this->Header.sh_size =
       (sizeof(Elf_Verdef) + sizeof(Elf_Verdaux)) * getVerDefNum();
+  for (Version &V : Config->SymbolVersions)
+    if (!V.Parent.empty())
+      this->Header.sh_size += sizeof(Elf_Verdaux);
+
   this->Header.sh_link = Out<ELFT>::DynStrTab->SectionIndex;
   this->Header.sh_addralign = sizeof(uint32_t);
 
   // sh_info should be set to the number of definitions. This fact is missed in
   // documentation, but confirmed by binutils community:
   // https://sourceware.org/ml/binutils/2014-11/msg00355.html
   this->Header.sh_info = getVerDefNum();
 }
 
+static size_t getVersionNameStrTabOffset(StringRef Name) {
+  for (Version &V : Config->SymbolVersions)
+    if (V.Name == Name)
+      return V.NameOff;
+  error("unknown version name " + Name + " used as a dependency");
+  return 0;
+}
+
 template <class Elf_Verdef, class Elf_Verdaux>
 static void writeDefinition(Elf_Verdef *&Verdef, Elf_Verdaux *&Verdaux,
                             uint32_t Flags, uint32_t Index, StringRef Name,
-                            size_t StrTabOffset) {
+                            size_t StrTabOffset, StringRef ParentName) {
+  bool HasParent = !ParentName.empty();
+
   Verdef->vd_version = 1;
-  Verdef->vd_cnt = 1;
+  Verdef->vd_cnt = HasParent ? 2 : 1;
   Verdef->vd_aux =
       reinterpret_cast<char *>(Verdaux) - reinterpret_cast<char *>(Verdef);
   Verdef->vd_next = sizeof(Elf_Verdef);
 
   Verdef->vd_flags = Flags;
   Verdef->vd_ndx = Index;
   Verdef->vd_hash = hashSysv(Name);
   ++Verdef;
 
   Verdaux->vda_name = StrTabOffset;
+  if (HasParent) {
+    Verdaux->vda_next = sizeof(Elf_Verdaux);
+    ++Verdaux;
+    Verdaux->vda_name = getVersionNameStrTabOffset(ParentName);
+  }
+
   Verdaux->vda_next = 0;
   ++Verdaux;
 }
 
 template <class ELFT>
 void VersionDefinitionSection<ELFT>::writeTo(uint8_t *Buf) {
   Elf_Verdef *Verdef = reinterpret_cast<Elf_Verdef *>(Buf);
   Elf_Verdaux *Verdaux =
       reinterpret_cast<Elf_Verdaux *>(Verdef + getVerDefNum());
 
   writeDefinition(Verdef, Verdaux, VER_FLG_BASE, 1, getFileDefName(),
-                  FileDefNameOff);
+                  FileDefNameOff, "" /* Parent */);
 
   uint32_t I = 2;
   for (Version &V : Config->SymbolVersions)
-    writeDefinition(Verdef, Verdaux, 0 /* Flags */, I++, V.Name, V.NameOff);
+    writeDefinition(Verdef, Verdaux, 0 /* Flags */, I++, V.Name, V.NameOff,
+                    V.Parent);
 
   Verdef[-1].vd_next = 0;
 }
 
 template <class ELFT>
 VersionTableSection<ELFT>::VersionTableSection()
     : OutputSectionBase<ELFT>(".gnu.version", SHT_GNU_versym, SHF_ALLOC) {
   this->Header.sh_addralign = sizeof(uint16_t);

lld/trunk/ELF/SymbolListFile.cpp

if (peek() == "global:") {
     parseVersionSymbols(Version);
   }
   if (peek() == "local:")
     parseLocal();
   else
     parseVersionSymbols(Version);
 
   expect("}");
+  if (!Version.empty() && peek() != ";")
+    Config->SymbolVersions.back().Parent = next();
   expect(";");
 }
 
 void VersionScriptParser::parseLocal() {
   expect("local:");
   expect("*");
   expect(";");
   Config->VersionScriptGlobalByDefault = false;

lld/trunk/test/ELF/verdef-dependency.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+# RUN: echo "LIBSAMPLE_1.0{               \
+# RUN:          global: a;                \
+# RUN:          local: *; };              \
+# RUN:       LIBSAMPLE_2.0{               \
+# RUN:          global: b;                \
+# RUN:          local: *; }LIBSAMPLE_1.0; \
+# RUN:       LIBSAMPLE_3.0{               \
+# RUN:          global: c;                \
+# RUN:          local: *; }LIBSAMPLE_2.0;" > %t.script
+# RUN: ld.lld --version-script %t.script -shared -soname shared %t.o -o %t.so
+# RUN: llvm-readobj -V -dyn-symbols %t.so | FileCheck --check-prefix=DSO %s
+
+# DSO:      SHT_GNU_verdef {
+# DSO-NEXT:   Definition {
+# DSO-NEXT:     Version: 1
+# DSO-NEXT:     Flags: Base
+# DSO-NEXT:     Index: 1
+# DSO-NEXT:     Hash: 127830196
+# DSO-NEXT:     Name: shared
+# DSO-NEXT:   }
+# DSO-NEXT:   Definition {
+# DSO-NEXT:     Version: 1
+# DSO-NEXT:     Flags: 0x0
+# DSO-NEXT:     Index: 2
+# DSO-NEXT:     Hash: 98457184
+# DSO-NEXT:     Name: LIBSAMPLE_1.0
+# DSO-NEXT:   }
+# DSO-NEXT:   Definition {
+# DSO-NEXT:     Version: 1
+# DSO-NEXT:     Flags: 0x0
+# DSO-NEXT:     Index: 3
+# DSO-NEXT:     Hash: 98456416
+# DSO-NEXT:     Name: LIBSAMPLE_2.0
+# DSO-NEXT:     Predecessor: LIBSAMPLE_1.0
+# DSO-NEXT:   }
+# DSO-NEXT:   Definition {
+# DSO-NEXT:     Version: 1
+# DSO-NEXT:     Flags: 0x0
+# DSO-NEXT:     Index: 4
+# DSO-NEXT:     Hash: 98456672
+# DSO-NEXT:     Name: LIBSAMPLE_3.0
+# DSO-NEXT:     Predecessor: LIBSAMPLE_2.0
+# DSO-NEXT:   }
+# DSO-NEXT: }
+
+# RUN: echo "LIBSAMPLE_1.0{               \
+# RUN:          global: a;                \
+# RUN:          local: *; };              \
+# RUN:       LIBSAMPLE_2.0{               \
+# RUN:          global: b;                \
+# RUN:          local: *; }LIBSAMPLE_X.X; " > %t.script
+# RUN: not ld.lld --version-script %t.script -shared %t.o -o %t.so 2>&1 \
+# RUN:   | FileCheck -check-prefix=ERR %s
+# ERR: unknown version name LIBSAMPLE_X.X used as a dependency