[lld] 54d2896 - [ELF] --wrap: Drop __real_ symbol from the symbol table

[Fangrui Song via llvm-commits]

Author: Rui Ueyama
Date: 2020-05-27T16:58:00-07:00
New Revision: 54d289685260da85fc43c59db2550b18df7c33a5

URL: https://github.com/llvm/llvm-project/commit/54d289685260da85fc43c59db2550b18df7c33a5
DIFF: https://github.com/llvm/llvm-project/commit/54d289685260da85fc43c59db2550b18df7c33a5.diff

LOG: [ELF] --wrap: Drop __real_ symbol from the symbol table

In D34993, we discussed and concluded that we should drop `__real_
symbol from the symbol table, but I did the opposite in D50569.
This patch is to drop `__real_` symbol.

MaskRay's note: omitting `__real_` is important if it is undefined:
otherwise a subsequent link may error due to the undefined `__real_` in .dynsym

Differential Revision: https://reviews.llvm.org/D51283

Added: 
    

Modified: 
    lld/ELF/SymbolTable.cpp
    lld/test/ELF/lto/wrap-2.ll
    lld/test/ELF/wrap-no-real.s
    lld/test/ELF/wrap.s

Removed: 
    


################################################################################
diff  --git a/lld/ELF/SymbolTable.cpp b/lld/ELF/SymbolTable.cpp
index ccec90727730..2097234f1adb 100644
--- a/lld/ELF/SymbolTable.cpp
+++ b/lld/ELF/SymbolTable.cpp
@@ -40,12 +40,14 @@ void SymbolTable::wrap(Symbol *sym, Symbol *real, Symbol *wrap) {
   idx2 = idx1;
   idx1 = idx3;
 
-  // Now renaming is complete. No one refers Real symbol. We could leave
-  // Real as-is, but if Real is written to the symbol table, that may
-  // contain irrelevant values. So, we copy all values from Sym to Real.
-  StringRef s = real->getName();
-  memcpy(real, sym, sizeof(SymbolUnion));
-  real->setName(s);
+  // Now renaming is complete, and no one refers to real. We drop real from
+  // .symtab and .dynsym. If real is undefined, it is important that we don't
+  // leave it in .dynsym, because otherwise it might lead to an undefined symbol
+  // error in a subsequent link. If real is defined, we could emit real as an
+  // alias for sym, but that could degrade the user experience of some tools
+  // that can print out only one symbol for each location: sym is a preferred
+  // name than real, but they might print out real instead.
+  real->isUsedInRegularObj = false;
 }
 
 // Find an existing symbol or create a new one.

diff  --git a/lld/test/ELF/lto/wrap-2.ll b/lld/test/ELF/lto/wrap-2.ll
index 4c8993aa0424..a441c9837bda 100644
--- a/lld/test/ELF/lto/wrap-2.ll
+++ b/lld/test/ELF/lto/wrap-2.ll
@@ -27,10 +27,6 @@
 ; BIND-NEXT: Value:
 ; BIND-NEXT: Size:
 ; BIND-NEXT: Binding: Local
-; BIND:      Name: __real_bar
-; BIND-NEXT: Value:
-; BIND-NEXT: Size:
-; BIND-NEXT: Binding: Local
 ; BIND:      Name: __wrap_bar
 ; BIND-NEXT: Value:
 ; BIND-NEXT: Size:

diff  --git a/lld/test/ELF/wrap-no-real.s b/lld/test/ELF/wrap-no-real.s
index 41a23185093b..0ee95edd8e30 100644
--- a/lld/test/ELF/wrap-no-real.s
+++ b/lld/test/ELF/wrap-no-real.s
@@ -5,27 +5,30 @@
 // RUN: ld.lld -o %t3.so -shared %t3.o
 
 // RUN: ld.lld -o %t %t1.o %t2.o -wrap foo
-// RUN: llvm-objdump -d --print-imm-hex %t | FileCheck %s
-
-// RUN: ld.lld -o %t %t1.o %t2.o %t3.so -wrap foo
-// RUN: llvm-objdump -d --print-imm-hex %t | FileCheck %s
+// RUN: llvm-objdump -d %t | FileCheck %s
+// RUN: llvm-readelf -s -x .got %t | FileCheck --check-prefix=READELF --implicit-check-not=__real_ %s
 
 // CHECK: <_start>:
-// CHECK-NEXT: movl $0x11010, %edx
-// CHECK-NEXT: movl $0x11010, %edx
-// CHECK-NEXT: movl $0x11000, %edx
+// CHECK-NEXT: movq {{.*}}(%rip), %rax  # 2021a8
+// CHECK-NEXT: movq {{.*}}(%rip), %rbx  # 2021a8
+// CHECK-NEXT: movq {{.*}}(%rip), %rcx  # 2021b0
 
-// RUN: llvm-objdump -t %t | FileCheck --check-prefix=SYM %s
+// READELF:      0000000000011010  0 NOTYPE GLOBAL DEFAULT ABS __wrap_foo
+// READELF:      0000000000011000  0 NOTYPE GLOBAL DEFAULT ABS foo
+// READELF:      Hex dump of section '.got':
+// READELF-NEXT: 0x[[#%x,ADDR:]] 10100100 00000000 00100100 00000000
 
+// RUN: ld.lld -o %t2 %t1.o %t2.o %t3.so --wrap foo
+// RUN: llvm-objdump -d %t2 | FileCheck --check-prefix=CHECK2 %s
+// RUN: llvm-readelf -s -x .got %t2 | FileCheck --check-prefix=READELF --implicit-check-not=__real_ %s
 
-// SYM:      {{.*}}           l .dynamic 0000000000000000 .hidden _DYNAMIC
-// SYM-NEXT: 0000000000011000 g *ABS*    0000000000000000 __real_foo
-// SYM-NEXT: 0000000000011010 g *ABS*    0000000000000000 __wrap_foo
-// SYM-NEXT: {{.*}}           g .text    0000000000000000 _start
-// SYM-NEXT: 0000000000011000 g *ABS*    0000000000000000 foo
+// CHECK2: <_start>:
+// CHECK2-NEXT: movq {{.*}}(%rip), %rax  # 2022f8
+// CHECK2-NEXT: movq {{.*}}(%rip), %rbx  # 2022f8
+// CHECK2-NEXT: movq {{.*}}(%rip), %rcx  # 202300
 
 .global _start
 _start:
-  movl $foo, %edx
-  movl $__wrap_foo, %edx
-  movl $__real_foo, %edx
+  mov foo at gotpcrel(%rip), %rax
+  mov __wrap_foo at gotpcrel(%rip), %rbx
+  mov __real_foo at gotpcrel(%rip), %rcx

diff  --git a/lld/test/ELF/wrap.s b/lld/test/ELF/wrap.s
index 5718ea45f669..2a3e56cb0af5 100644
--- a/lld/test/ELF/wrap.s
+++ b/lld/test/ELF/wrap.s
@@ -33,12 +33,7 @@
 // SYM2-NEXT: Other [
 // SYM2-NEXT:   STV_PROTECTED
 // SYM2-NEXT: ]
-// SYM3:      Name: __real_foo
-// SYM3-NEXT: Value: 0x11000
-// SYM3-NEXT: Size:
-// SYM3-NEXT: Binding: Global
-// SYM3-NEXT: Type:    None
-// SYM3-NEXT: Other:   0
+// SYM3-NOT:  Name: __real_foo
 
 .global _start
 _start: