[PATCH] D41639: [ELF] Drop unnecessary VersionId setting in scanShlibUndefined

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai created this revision.
smeenai added reviewers: rafael, ruiu.
Herald added a subscriber: emaste.

LLD previously used to handle dynamic lists and version scripts in the
exact same way, even though they have very different semantics for
shared libraries and subtly different semantics for executables. r315114
untangled their semantics for executables (building on previous work to
correct their semantics for shared libraries). With that change, dynamic
lists won't set the default version to VER_NDX_LOCAL, and so resetting
the version to VER_NDX_GLOBAL in scanShlibUndefined is unnecessary.

This was causing an issue because version scripts containing `local: *`
work by setting the default version to VER_NDX_LOCAL, but scanShlibUndefined
would override this default, and therefore symbols which should have
been local would end up in the dynamic symbol table, which differs from
both bfd and gold's behavior. gold silently keeps the symbol hidden in
such a scenario, whereas bfd issues an error. I prefer bfd's behavior
and plan to implement that in LLD in a follow-up (and the test case
added here will be updated accordingly).


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D41639

Files:
  ELF/SymbolTable.cpp
  test/ELF/shlib-undefined-local.s


Index: test/ELF/shlib-undefined-local.s
===================================================================
--- /dev/null
+++ test/ELF/shlib-undefined-local.s
@@ -0,0 +1,19 @@
+# REQUIRES: x86
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-linux-gnu -o %t1.o %S/Inputs/shlib-undefined-ref.s
+# RUN: ld.lld -shared -o %t.so %t1.o
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-linux-gnu -o %t2.o %s
+# RUN: echo "{ local: *; };" > %t.script
+# RUN: ld.lld -version-script %t.script -o %t %t2.o %t.so
+# RUN: llvm-nm -g %t | FileCheck -allow-empty %s
+
+# CHECK-NOT: should_not_be_exported
+
+.globl should_not_be_exported
+should_not_be_exported:
+	ret
+
+.globl _start
+_start:
+	ret
Index: ELF/SymbolTable.cpp
===================================================================
--- ELF/SymbolTable.cpp
+++ ELF/SymbolTable.cpp
@@ -598,12 +598,6 @@
       if (!Sym || !Sym->isDefined())
         continue;
       Sym->ExportDynamic = true;
-
-      // If -dynamic-list is given, the default version is set to
-      // VER_NDX_LOCAL, which prevents a symbol to be exported via .dynsym.
-      // Set to VER_NDX_GLOBAL so the symbol will be handled as if it were
-      // specified by -dynamic-list.
-      Sym->VersionId = VER_NDX_GLOBAL;
     }
   }
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D41639.128355.patch
Type: text/x-patch
Size: 1254 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171230/14d6ff6e/attachment.bin>