[PATCH] D33680: [ELF] - Resolve references properly when using .symver directive

[George Rimar via Phabricator via llvm-commits]

grimar updated this revision to Diff 102527.
grimar added a comment.

- Updated.

I played with few variations of this patch and found combination that looks fine for me finally.
I think this way of solving initial issue is simple and short and that is why I prefer it in general.

isDefaultVersion() looks a code duplication at first look, but it is just 3 lines and
IMO makes logic be very straightforward and allows to simplify other things, what makes this patch be very short.


https://reviews.llvm.org/D33680

Files:
  ELF/SymbolTable.cpp
  ELF/Symbols.cpp
  test/ELF/version-script-symver-err.s
  test/ELF/version-script-symver.s


Index: test/ELF/version-script-symver.s
===================================================================
--- test/ELF/version-script-symver.s
+++ test/ELF/version-script-symver.s
@@ -0,0 +1,10 @@
+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+# RUN: ld.lld %t.o -o %t
+
+.global _start
+.global bar
+.symver _start, bar@@VERSION
+_start:
+  jmp bar
Index: test/ELF/version-script-symver-err.s
===================================================================
--- test/ELF/version-script-symver-err.s
+++ test/ELF/version-script-symver-err.s
@@ -0,0 +1,15 @@
+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+# RUN: not ld.lld %t.o -o %t 2>&1 | FileCheck %s
+# CHECK:      error: duplicate symbol: bar
+# CHECK-NEXT:   >>> defined at {{.*}}.o
+# CHECK-NEXT:   >>> defined at {{.*}}.o
+
+.global _start
+.global bar
+.symver _start, bar@@VERSION
+_start:
+  jmp bar
+
+bar:
Index: ELF/Symbols.cpp
===================================================================
--- ELF/Symbols.cpp
+++ ELF/Symbols.cpp
@@ -256,7 +256,9 @@
   }
 
   // It is an error if the specified version is not defined.
-  error(toString(File) + ": symbol " + S + " has undefined version " + Verstr);
+  if (Config->Shared)
+    error(toString(File) + ": symbol " + S + " has undefined version " +
+          Verstr);
 }
 
 Defined::Defined(Kind K, StringRefZ Name, bool IsLocal, uint8_t StOther,
Index: ELF/SymbolTable.cpp
===================================================================
--- ELF/SymbolTable.cpp
+++ ELF/SymbolTable.cpp
@@ -715,15 +715,33 @@
       B->symbol()->VersionId = VersionId;
 }
 
+static bool isDefaultVersion(SymbolBody *Body) {
+  if (!Body->isInCurrentDSO())
+    return false;
+  return Body->getName().find("@@") != StringRef::npos;
+}
+
 // This function processes version scripts by updating VersionId
 // member of symbols.
 template <class ELFT> void SymbolTable<ELFT>::scanVersionScript() {
   // Symbol themselves might know their versions because symbols
   // can contain versions in the form of <name>@<version>.
   // Let them parse their names.
-  if (!Config->VersionDefinitions.empty())
-    for (Symbol *Sym : SymVector)
-      Sym->body()->parseSymbolVersion();
+  // Below we resolve references to symbols that have default version. We use
+  // additional temp vector because addRegular() call may change SymVector.
+  std::vector<DefinedRegular *> DefaultV;
+  for (Symbol *Sym : SymVector) {
+    if (isDefaultVersion(Sym->body()))
+      DefaultV.push_back(cast<DefinedRegular>(Sym->body()));
+    Sym->body()->parseSymbolVersion();
+  }
+
+  // <name>@@<version> means symbol has 'default' version. We should use it to
+  // resolve references to <name>.
+  for (DefinedRegular *D : DefaultV)
+    if (!find(D->getName()))
+      addRegular(D->getName(), D->StOther, D->Type, D->Value, D->Size,
+                 D->symbol()->Binding, D->Section, D->File);
 
   // Handle edge cases first.
   handleAnonymousVersion();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D33680.102527.patch
Type: text/x-patch
Size: 3035 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170614/39c42b0b/attachment.bin>