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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added reviewers: dim, grimar, ruiu.
Herald added subscribers: llvm-commits, arichardson, emaste.
Herald added a reviewer: espindola.
Herald added a project: LLVM.

In lvm2, libdevmapper.so is linked with a version script with duplicate
version assignments:

  DM_1_02_138 { global: ... dm_bitset_parse_list; ... };
  DM_1_02_129 { global: ... dm_bitset_parse_list; ... };

ld.bfd silently accepts this while gold issues a warning. We currently
error, thus inhibit producing the executable. Change the error to
warning to allow this case, and improve the message.

There are some cases where ld.bfd error
`anonymous version tag cannot be combined with other version tags`
but we just warn. It is probably OK for now.

Changing the error to warning fixes PR38549, which is silently (and correctly) accepted by ld.bfd.
lld issued an error because
`assignWildcardVersion(ver, VER_NDX_LOCAL);` is processed before `assignExactVersion(ver, v.id, v.name);`.
It issues a warning instead with this change.
This is tested by `%t1.ver %t2w.ver` in version-script-reassign.s


Repository:
  rLLD LLVM Linker

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,26 @@
+# 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 /dev/null 2>&1 | FileCheck --check-prefix=LOCAL %s
+# RUN: ld.lld -shared %t.o --version-script %tg.ver --version-script %t1.ver \
+# RUN:   -o /dev/null 2>&1 | FileCheck --check-prefix=GLOBAL %s
+
+## Note, ld.bfd silently accepts the two cases.
+# RUN: ld.lld -shared %t.o --version-script %t1.ver --version-script %t2.ver \
+# RUN:   -o /dev/null 2>&1 | FileCheck --check-prefix=V1 %s
+# RUN: ld.lld -shared %t.o --version-script %t1.ver --version-script %t2w.ver \
+# RUN:   -o /dev/null 2>&1 | FileCheck --check-prefix=LOCAL %s
+
+# LOCAL: warning: symbol 'foo' was assigned as local
+# GLOBAL: warning: symbol 'foo' was assigned as global
+# V1: warning: symbol 'foo' was assigned to version 'V1'
+
+.globl foo
+foo:
Index: ELF/SymbolTable.cpp
===================================================================
--- ELF/SymbolTable.cpp
+++ ELF/SymbolTable.cpp
@@ -209,10 +209,18 @@
     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;
+    } else if (sym->versionId != versionId) {
+      std::string msg = ("symbol '" + ver.name + "' was assigned").str();
+      if (sym->versionId == VER_NDX_LOCAL)
+        warn(msg + " as local");
+      else if (sym->versionId == VER_NDX_GLOBAL)
+        warn(msg + " as global");
+      else
+        warn(msg + " to version '" +
+             config->versionDefinitions[sym->versionId - 2].name + "'");
+    }
   }
 }
 


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