[PATCH] D43126: [LLD][ELF] Do not error for missing version when symbol has local version.

[Peter Smith via Phabricator via llvm-commits]

peter.smith created this revision.
peter.smith added reviewers: ruiu, rafael, grimar.
Herald added subscribers: kristof.beyls, arichardson, emaste.

If a symbol with an undefined version in a DSO is not going to be exported into the dynamic symbol table then do not give an error message for the missing version. This can happen with the --exclude-libs option which implicitly gives all symbols in a static library the local version. This matches the behavior of ld.gold and is exploited by the Bionic dynamic linker on Arm.

This was found when trying to link the bionic dynamic linker on Arm and got link errors with LLD. I've reported to Bionic under https://issuetracker.google.com/issues/73020933
as ld.bfd was also giving the same error message, but ld.gold was succeeding. I think on balance ld.gold is more sensible and we should match it.

Fixes PR36295


https://reviews.llvm.org/D43126

Files:
  ELF/Symbols.cpp
  test/ELF/Inputs/versiondef.s
  test/ELF/version-exclude-libs.s


Index: test/ELF/version-exclude-libs.s
===================================================================
--- /dev/null
+++ test/ELF/version-exclude-libs.s
@@ -0,0 +1,30 @@
+// REQUIRES: x86
+// RUN: llvm-mc %p/Inputs/versiondef.s -o %t.o -filetype=obj -triple=x86_64-pc-linux
+// RUN: llvm-ar -r %t.a %t.o
+// RUN: llvm-mc %s -o %t2.o -filetype=obj -triple=x86_64-pc-linux
+// RUN: ld.lld %t2.o %t.a --shared --exclude-libs ALL -o %t.so
+// RUN: llvm-readobj -symbols %t.so | FileCheck %s
+// RUN: llvm-readobj -dyn-symbols %t.so | FileCheck -check-prefix CHECK-DYN %s
+// RUN: not ld.lld %t2.o %t.a --shared -o %t.so 2>&1 | FileCheck -check-prefix=CHECK-ERR %s
+
+// Test that we do not give an error message for undefined versions when the
+// symbol is not exported to the dynamic symbol table.
+
+// CHECK:          Name: func
+// CHECK-NEXT:     Value:
+// CHECK-NEXT:     Size:
+// CHECK-NEXT:     Binding: Local (0x0)
+
+// CHECK-DYN-NOT: func
+
+// CHECK-ERR: symbol func@@VER2 has undefined version VER2
+// CHECK-ERR-NEXT: symbol func at VER has undefined version VER
+
+ .text
+ .globl _start
+ .globl func
+_start:
+ ret
+
+ .data
+ .quad func
Index: test/ELF/Inputs/versiondef.s
===================================================================
--- /dev/null
+++ test/ELF/Inputs/versiondef.s
@@ -0,0 +1,9 @@
+.text
+.globl func_impl
+func_impl:
+  ret
+.globl func_impl2
+func_impl2:
+  ret
+.symver func_impl, func@@VER2
+.symver func_impl2, func at VER
Index: ELF/Symbols.cpp
===================================================================
--- ELF/Symbols.cpp
+++ ELF/Symbols.cpp
@@ -206,8 +206,10 @@
   // It is an error if the specified version is not defined.
   // Usually version script is not provided when linking executable,
   // but we may still want to override a versioned symbol from DSO,
-  // so we do not report error in this case.
-  if (Config->Shared)
+  // so we do not report error in this case. We also do not error
+  // if the symbol has a local version as it won't be in the dynamic
+  // symbol table.
+  if (Config->Shared && VersionId != VER_NDX_LOCAL)
     error(toString(File) + ": symbol " + S + " has undefined version " +
           Verstr);
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D43126.133604.patch
Type: text/x-patch
Size: 2195 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180209/b0862597/attachment.bin>