Refine our --wrap implementation.

Before this patch we would copy foo into real_foo and wrap_foo into
foo. The net result is that __wrap_foo shows up twice in the symbol
table.

With this patch we:

* save a copy of __real_foo before copying foo.
* drop one of the __wrap_foo from the symbol table.
* if __real_foo was not undefined, add a *new* symbol with that content to
  the symbol table.

The net result is that

Anything using foo now uses __wrap_foo
Anything using __real_foo now uses foo.
Anything using __wrap_foo still does.

And the symbol table has foo, __wrap_foo and __real_foo (if defined).

Which I think is the desired behavior.

llvm-svn: 315097

lld/ELF/SymbolTable.cpp

@@ -164,6 +164,8 @@ template <class ELFT> void SymbolTable::addSymbolWrap(StringRef Name) {
 
   defsym(Real, Sym);
   defsym(Sym, Wrap);
+
+  WrapSymbols.push_back({Wrap, Real});
 }
 
 // Creates alias for symbol. Used to implement --defsym=ALIAS=SYM.
@@ -183,11 +185,43 @@ void SymbolTable::addSymbolAlias(StringRef Alias, StringRef Name) {
 // LTO (if LTO is running) not to include these symbols in IPO. Now that the
 // symbols are finalized, we can perform the replacement.
 void SymbolTable::applySymbolRenames() {
+  // This function rotates 3 symbols:
+  //
+  // __real_foo becomes foo
+  // foo        becomes __wrap_foo
+  // __wrap_foo becomes __real_foo
+  //
+  // The last part is special in that we don't want to change what references to
+  // __wrap_foo point to, we just want have __real_foo in the symbol table.
+
+  // First make a copy of __real_foo
+  std::vector<Symbol> Origs;
+  for (const auto &P : WrapSymbols)
+    Origs.push_back(*P.second);
+
+  // Replace __real_foo with foo and foo with __wrap_foo
   for (SymbolRenaming &S : Defsyms) {
     S.Dst->body()->copyFrom(S.Src->body());
     S.Dst->File = S.Src->File;
     S.Dst->Binding = S.Binding;
   }
+
+  // Hide one of the copies of __wrap_foo, create a new symbol and copy
+  // __real_foo into it.
+  for (unsigned I = 0, N = WrapSymbols.size(); I < N; ++I) {
+    // We now have two copies of __wrap_foo. Drop one.
+    Symbol *Wrap = WrapSymbols[I].first;
+    Wrap->IsUsedInRegularObj = false;
+
+    Symbol *Real = &Origs[I];
+    // If __real_foo was undefined, we don't want it in the symbol table.
+    if (Real->body()->isUndefined())
+      continue;
+
+    auto *NewSym = make<Symbol>();
+    memcpy(NewSym, Real, sizeof(Symbol));
+    SymVector.push_back(NewSym);
+  }
 }
 
 static uint8_t getMinVisibility(uint8_t VA, uint8_t VB) {

lld/ELF/SymbolTable.h

@@ -144,6 +144,9 @@ private:
   // For -defsym or -wrap.
   std::vector<SymbolRenaming> Defsyms;
 
+  // For -wrap.
+  std::vector<std::pair<Symbol *, Symbol *>> WrapSymbols;
+
   // For LTO.
   std::unique_ptr<BitcodeCompiler> LTO;
 };

lld/test/ELF/Inputs/wrap-no-real.s

@@ -0,0 +1,3 @@
+.globl foo, __wrap_foo
+foo = 0x11000
+__wrap_foo = 0x11010

lld/test/ELF/lto/wrap-2.ll

@@ -28,11 +28,11 @@
 ; THIN-NEXT: jmp{{.*}}<bar>
 
 ; Check that bar and __wrap_bar retain their original binding.
-; BIND:      Name: bar
+; BIND:      Name: __wrap_bar
 ; BIND-NEXT: Value:
 ; BIND-NEXT: Size:
 ; BIND-NEXT: Binding: Local
-; BIND:      Name: __wrap_bar
+; BIND:      Name: bar
 ; BIND-NEXT: Value:
 ; BIND-NEXT: Size:
 ; BIND-NEXT: Binding: Local

lld/test/ELF/wrap-no-real.s

@@ -0,0 +1,26 @@
+// REQUIRES: x86
+// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t1.o
+// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/wrap-no-real.s -o %t2.o
+
+// RUN: ld.lld -o %t %t1.o %t2.o -wrap foo
+// RUN: llvm-objdump -d -print-imm-hex %t | FileCheck %s
+
+// CHECK: _start:
+// CHECK-NEXT: movl $0x11010, %edx
+// CHECK-NEXT: movl $0x11010, %edx
+// CHECK-NEXT: movl $0x11000, %edx
+
+// RUN: llvm-readobj -t -s %t | FileCheck -check-prefix=SYM %s
+// SYM-NOT:  Name: __real_foo
+// SYM:      Name: foo
+// SYM-NEXT: Value: 0x11000
+// SYM-NOT:  Name: __real_foo
+// SYM:      Name: __wrap_foo
+// SYM-NEXT: Value: 0x11010
+// SYM-NOT:  Name: __real_foo
+
+.global _start
+_start:
+  movl $foo, %edx
+  movl $__wrap_foo, %edx
+  movl $__real_foo, %edx

lld/test/ELF/wrap.s

@@ -12,17 +12,13 @@
 // CHECK-NEXT: movl $0x11010, %edx
 // CHECK-NEXT: movl $0x11000, %edx
 
-// This shows an oddity of our implementation. The symbol foo gets
-// mapped to __wrap_foo, but stays in the symbol table. This results
-// in it showing up twice in the output.
-
 // RUN: llvm-readobj -t -s %t3 | FileCheck -check-prefix=SYM %s
 // SYM:      Name: foo
 // SYM-NEXT: Value: 0x11000
 // SYM:      Name: __wrap_foo
 // SYM-NEXT: Value: 0x11010
-// SYM:      Name: __wrap_foo
-// SYM-NEXT: Value: 0x11010
+// SYM:      Name: __real_foo
+// SYM-NEXT: Value: 0x11020
 
 .global _start
 _start: