[PATCH] D64549: [ELF] Warn rather than error when duplicate version assignments occur

[Fangrui Song via Phabricator via llvm-commits]

MaskRay updated this revision to Diff 209143.
MaskRay edited the summary of this revision.
MaskRay added a comment.

Improve warning message.
Update tests.

Note this doesn't fix PR38549, but it is related. And I can fix it in another change


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64549/new/

https://reviews.llvm.org/D64549

Files:
  ELF/SymbolTable.cpp
  test/ELF/version-script-reassign.s
  test/ELF/version-script.s


Index: test/ELF/version-script.s
===================================================================
--- test/ELF/version-script.s
+++ test/ELF/version-script.s
@@ -32,12 +32,6 @@
 # RUN:   FileCheck -check-prefix=ERR2 %s
 # ERR2: EOF expected, but got VERSION_2.0
 
-# RUN: echo "VERSION_1.0 { global: foo1; local: *; };" > %t6.script
-# RUN: echo "VERSION_2.0 { global: foo1; local: *; };" >> %t6.script
-# RUN: not ld.lld --version-script %t6.script -shared %t.o %t2.so -o /dev/null 2>&1 | \
-# RUN:   FileCheck -check-prefix=ERR3 %s
-# ERR3: duplicate symbol 'foo1' in version script
-
 # RUN: echo "{ foo1; foo2; };" > %t.list
 # RUN: ld.lld --version-script %t.script --dynamic-list %t.list %t.o %t2.so -o %t2
 # RUN: llvm-readobj %t2 > /dev/null
Index: test/ELF/version-script-reassign.s
===================================================================
--- /dev/null
+++ test/ELF/version-script-reassign.s
@@ -0,0 +1,32 @@
+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %s -o %t.o
+# RUN: echo '{ local: foo; };' > %tl.ver
+# RUN: echo '{ global: foo; local: *; };' > %tg.ver
+# RUN: echo 'V1 { global: foo; };' > %t1.ver
+# RUN: echo 'V2 { global: foo; };' > %t2.ver
+# RUN: echo 'V2 { global: notexist; local: f*; };' > %t2w.ver
+
+## Note, ld.bfd errors on the two cases.
+# RUN: ld.lld -shared %t.o --version-script %tl.ver --version-script %t1.ver \
+# RUN:   -o %t.so 2>&1 | FileCheck --check-prefix=LOCAL %s
+# RUN: llvm-readelf --dyn-syms %t.so | FileCheck --check-prefix=LOCAL-SYM %s
+# RUN: ld.lld -shared %t.o --version-script %tg.ver --version-script %t1.ver \
+# RUN:   -o %t.so 2>&1 | FileCheck --check-prefix=GLOBAL %s
+# RUN: llvm-readelf --dyn-syms %t.so | FileCheck --check-prefix=GLOBAL-SYM %s
+
+## Note, ld.bfd silently accepts this case.
+# RUN: ld.lld -shared %t.o --version-script %t1.ver --version-script %t2.ver \
+# RUN:   -o %t.so 2>&1 | FileCheck --check-prefix=V1-WARN %s
+# RUN: llvm-readelf --dyn-syms %t.so | FileCheck --check-prefix=V1-SYM %s
+
+# LOCAL: warning: attempt to reassign symbol 'foo' of VER_NDX_LOCAL to version 'V1'
+# LOCAL-SYM-NOT: foo
+
+# GLOBAL: warning: attempt to reassign symbol 'foo' of VER_NDX_GLOBAL to version 'V1'
+# GLOBAL-SYM: foo{{$}}
+
+# V1-WARN: warning: attempt to reassign symbol 'foo' of version 'V1' to version 'V2'
+# V1-SYM: foo@@V1
+
+.globl foo
+foo:
Index: ELF/SymbolTable.cpp
===================================================================
--- ELF/SymbolTable.cpp
+++ ELF/SymbolTable.cpp
@@ -201,6 +201,14 @@
     return;
   }
 
+  auto getName = [&](uint16_t ver) -> std::string {
+    if (ver == VER_NDX_LOCAL)
+      return "VER_NDX_LOCAL";
+    if (ver == VER_NDX_GLOBAL)
+      return "VER_NDX_GLOBAL";
+    return ("version '" + config->versionDefinitions[ver - 2].name + "'").str();
+  };
+
   // Assign the version.
   for (Symbol *sym : syms) {
     // Skip symbols containing version info because symbol versions
@@ -209,10 +217,14 @@
     if (sym->getName().contains('@'))
       continue;
 
-    if (sym->versionId != config->defaultSymbolVersion &&
-        sym->versionId != versionId)
-      error("duplicate symbol '" + ver.name + "' in version script");
-    sym->versionId = versionId;
+    if (sym->versionId == config->defaultSymbolVersion)
+      sym->versionId = versionId;
+    if (sym->versionId == versionId)
+      continue;
+
+    warn(("attempt to reassign symbol '" + ver.name + "' of " +
+          getName(sym->versionId) + " to " + getName(versionId))
+             .str());
   }
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D64549.209143.patch
Type: text/x-patch
Size: 3523 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190711/fdd6d847/attachment.bin>