[ELF] - Depricate version references.

[grimar]

Summary

This is PR28358

According to
https://www.akkadia.org/drepper/dsohowto.pdf

"The fourth point, the VERS 1.0 version being referred to in the VERS 2.0 definition, is not really important in symbol versioning. It marks the predecessor relationship of the two versions and it is done to maintain the similar- ities with Solaris’ internal versioning. It does not cause any problem it might in fact be useful to a human reader so predecessors should always be mentioned."

Patch partially reverts 273423 "[ELF] - Implemented version script hierarchies.",
version references are just ignored now.

[ruiu]

Is this really what we want? Let's say we have two version definitions as shown below.

VER1 { global: foo; local: *; };
VER2 { global: bar; } VER1;

In this case, I think VER1's catch-all local: * hides all symbols but foo and bar from VER2. Your patch seems to change this behavior.

[grimar]

In D21888#471119, @ruiu wrote:

Is this really what we want? Let's say we have two version definitions as shown below.

VER1 { global: foo; local: *; };
VER2 { global: bar; } VER1;

In this case, I think VER1's catch-all local: * hides all symbols but foo and bar from VER2. Your patch seems to change this behavior.

There is no difference. Just re-checked with gold.

VER1 { global: foo; local: *; };
VER2 { global: bar; } VER1;

and

VER1 { global: foo; local: *; };
VER2 { global: bar; };

has the same output. Actually I noticed that much earlier when worked on all of that, but I was sure that hierarchies are required
for correctnes to comform specification.
But if Drepper writes that there is no point to have it then why not remove it.

[ruiu]

Really? At least, gold seems to set vda_next field in a DSO if a predecessor is given. Do you mean that vda_next is ignored at runtime?

[grimar]

In D21888#471126, @ruiu wrote:

Really? At least, gold seems to set vda_next field in a DSO if a predecessor is given. Do you mean that vda_next is ignored at runtime?

Yes, sure, vda_next is set and that is what D21556 implemented. I mean that there is no affect on local tag, symbols versions and so on.
So on everything that can do some effect. So yes, vda_next looks to be just ignored in runtime.

[ruiu]

What about non-GNU systems such as BSDs?

[grimar]

In D21888#471132, @ruiu wrote:

What about non-GNU systems such as BSDs?

I did not check. Generally this change is bit in opposite with documenation I saw before,
as everywhere (ex: https://docs.oracle.com/cd/E19120-01/open.solaris/819-0690/chapter6-80869/index.html) version
dependency or "predecessor" is mentioned as a part of verdef section.

So I guess we can only check to find how much is important to have for other systems.

[ruiu]

I still half-believe that the va_next is completely ignored, but if so, this patch makes sense. Let's land this to see if this will work.

ELF/SymbolListFile.cpp

98–99 ↗

(On Diff #62341)

Add a blank line before the comment. Also I'd mention that parent name is a hint for human that the runtime completely ignores.

// Each version may have a parent version. For example, "Ver2" defined as
// "Ver2 { global: foo; local: *; } Ver1;" has "Ver1" as a parent. This version
// hierarchy is, probably against your instinct, purely for human; the runtime
// doesn't care about them at all. In LLD, we simply skip the token.

[grimar]

In D21888#472146, @ruiu wrote:

I still half-believe that the va_next is completely ignored

I feel wierd about this. Looks like format of verdef section is half-baked.

lld/trunk/ELF/Config.h

 
 enum class UnresolvedPolicy { NoUndef, Error, Warn, Ignore };
 
 // 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, StringRef ParentName) {
-  bool HasParent = !ParentName.empty();
-
+                            size_t StrTabOffset) {
   Verdef->vd_version = 1;
-  Verdef->vd_cnt = HasParent ? 2 : 1;
+  Verdef->vd_cnt = 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, "" /* Parent */);
+                  FileDefNameOff);
 
   uint32_t I = 2;
   for (Version &V : Config->SymbolVersions)
-    writeDefinition(Verdef, Verdaux, 0 /* Flags */, I++, V.Name, V.NameOff,
-                    V.Parent);
+    writeDefinition(Verdef, Verdaux, 0 /* Flags */, I++, V.Name, V.NameOff);
 
   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 if (peek() != "}")
     parseVersionSymbols(Version);
 
   expect("}");
+
+  // Each version may have a parent version. For example, "Ver2" defined as
+  // "Ver2 { global: foo; local: *; } Ver1;" has "Ver1" as a parent. This
+  // version hierarchy is, probably against your instinct, purely for human; the
+  // runtime doesn't care about them at all. In LLD, we simply skip the token.
   if (!Version.empty() && peek() != ";")
-    Config->SymbolVersions.back().Parent = next();
+    next();
   expect(";");
 }
 
 void VersionScriptParser::parseLocal() {
   expect("local:");
   expect("*");
   expect(";");
   Config->VersionScriptGlobalByDefault = false;

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

 # 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:  }
 
 ## Check that we can link against DSO produced.
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t2
 # RUN: ld.lld %t2 %t.so -o %t3
 # RUN: llvm-readobj -V -dyn-symbols %t3 | FileCheck --check-prefix=EXE %s
 

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

 # 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