[PATCH] D92259: [ELF] Make foo@@v1 resolved undefined foo at v1

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added reviewers: grimar, jhenderson, psmith.
Herald added subscribers: llvm-commits, dmgreen, arichardson, emaste.
Herald added a reviewer: espindola.
Herald added a project: LLVM.
MaskRay requested review of this revision.

The symbol resolution rules for versioned symbols are:

- foo@@v1 (default version) resolves both undefined foo and foo at v1
- foo at v1 (non-default version) resolves undefined foo at v1

Note, foo@@v1 must be defined (the assembler errors if attempting to
create an undefined foo@@v1).

For defined foo@@v1 in a shared object, we call `SymbolTable::addSymbol` twice,
one for foo and the other for foo at v1. We don't do the same for object files, so
foo@@v1 defined in one object file incorrectly does not resolve a foo at v1
reference in another object file.

This patch fixes the issue by reusing the --wrap code to redirect symbols in
object files. This has to be done after processing input files because
foo and foo at v1 are two separate symbols if we haven't seen foo@@v1.

Depends on D92258 <https://reviews.llvm.org/D92258>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92259

Files:
  lld/ELF/Driver.cpp
  lld/test/ELF/symver.s


Index: lld/test/ELF/symver.s
===================================================================
--- lld/test/ELF/symver.s
+++ lld/test/ELF/symver.s
@@ -9,22 +9,24 @@
 # RUN: ld.lld -shared --soname=def1.so --version-script=%t/ver %t/def1.o -o %t/def1.so
 # RUN: ld.lld -shared --soname=hid1.so --version-script=%t/ver %t/hid1.o -o %t/hid1.so
 
-# TODO Report an duplicate definition error for foo at v1 and foo@@v1.
-# RUN: ld.lld -shared --version-script=%t/ver %t/def1.o %t/hid1.o -o /dev/null
+# Report an duplicate definition error for foo at v1 and foo@@v1.
+# RUN: not ld.lld -shared --version-script=%t/ver %t/def1.o %t/hid1.o -o /dev/null 2>&1 | \
+# RUN:   FileCheck %s --check-prefix=DUP
+
+# DUP:      error: duplicate symbol: foo at v1
+# DUP-NEXT: >>> defined at {{.*}}/hid1.o:(.text+0x0)
+# DUP-NEXT: >>> defined at {{.*}}/def1.o:(.text+0x0)
 
-## TODO
 ## foo@@v1 resolves both undefined foo and foo at v1. There is one single definition.
 ## Note, set soname so that the name string referenced by .gnu.version_d is fixed.
 # RUN: ld.lld -shared --soname=t1 --version-script=%t/ver %t/ref.o %t/ref1.o %t/def1.o -o %t1
 # RUN: llvm-readelf -r --dyn-syms %t1 | FileCheck %s
 
-# CHECK:       Relocation section '.rela.plt' at offset {{.*}} contains 2 entries:
+# CHECK:       Relocation section '.rela.plt' at offset {{.*}} contains 1 entries:
 # CHECK-NEXT:  {{.*}} Type               {{.*}}
 # CHECK-NEXT:  {{.*}} R_X86_64_JUMP_SLOT {{.*}} foo@@v1 + 0
-# CHECK-NEXT:  {{.*}} R_X86_64_JUMP_SLOT {{.*}} foo + 0
 
-# CHECK:       1: {{.*}} NOTYPE GLOBAL DEFAULT UND       foo{{$}}
-# CHECK-NEXT:  2: {{.*}} NOTYPE GLOBAL DEFAULT {{[1-9]}} foo@@v1
+# CHECK:       1: {{.*}} NOTYPE GLOBAL DEFAULT {{[1-9]}} foo@@v1
 # CHECK-EMPTY:
 
 ## foo@@v2 does not resolve undefined foo at v1.
Index: lld/ELF/Driver.cpp
===================================================================
--- lld/ELF/Driver.cpp
+++ lld/ELF/Driver.cpp
@@ -1907,17 +1907,43 @@
   return v;
 }
 
-// Do renaming for -wrap by updating pointers to symbols.
+// Do renaming for -wrap and foo at v1 by updating pointers to symbols.
 //
 // When this function is executed, only InputFiles and symbol table
 // contain pointers to symbol objects. We visit them to replace pointers,
 // so that wrapped symbols are swapped as instructed by the command line.
-static void wrapSymbols(ArrayRef<WrappedSymbol> wrapped) {
+static void redirectSymbols(ArrayRef<WrappedSymbol> wrapped) {
   DenseMap<Symbol *, Symbol *> map;
   for (const WrappedSymbol &w : wrapped) {
     map[w.sym] = w.wrap;
     map[w.real] = w.sym;
   }
+  for (Symbol *sym : symtab->symbols()) {
+    // Enumerate symbols with a non-default version (foo at v1). Its name was
+    // truncated at '@' by Symbol::parseSymbolVersion().
+    StringRef name = sym->getName();
+    const char *end1 = name.data() + name.size();
+    if (*end1 != '@' || end1[1] == '@')
+      continue;
+
+    // Check whether the default version foo@@v1 exists. If exists, the symbol
+    // can be found by the name "foo" in the symbol table.
+    Symbol *maybeDefault = symtab->find(name);
+    if (!maybeDefault)
+      continue;
+    const char *end2 =
+        maybeDefault->getName().data() + maybeDefault->getName().size();
+    if (*end2 != '@' || end2[1] != '@' || strcmp(end1 + 1, end2 + 2) != 0)
+      continue;
+
+    // foo at v1 and foo@@v1 should be considered as one symbol. So we redirect
+    // foo at v1 references to foo@@v1.
+    map.try_emplace(sym, maybeDefault);
+    // If foo at v1 is also definied, report a duplicate definition error.
+    sym->resolve(*maybeDefault);
+    // Eliminate foo at v1 from the symbol table.
+    sym->symbolKind = Symbol::PlaceholderKind;
+  }
 
   // Update pointers in input files.
   parallelForEach(objectFiles, [&](InputFile *file) {
@@ -2146,9 +2172,8 @@
       !config->thinLTOModulesToCompile.empty())
     return;
 
-  // Apply symbol renames for -wrap.
-  if (!wrapped.empty())
-    wrapSymbols(wrapped);
+  // Apply symbol renames for -wrap and combine foo at v1 and foo@@v1.
+  redirectSymbols(wrapped);
 
   {
     llvm::TimeTraceScope timeScope("Aggregate sections");


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D92259.308137.patch
Type: text/x-patch
Size: 4123 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201128/30370f03/attachment.bin>