[PATCH] D111852: [lld/mac] Mark private externs with GOT relocs as LOCAL in indirect symbtab

[Nico Weber via Phabricator via llvm-commits]

thakis created this revision.
thakis added a reviewer: lld-macho.
Herald added a reviewer: gkm.
Herald added a project: lld-macho.
thakis requested review of this revision.

prepareSymbolRelocation() in Writer.cpp adds both symbols that need binding and
symbols relocated with a pointer relocation to the got.

Pointer relocations are emitted for non-movq GOTPCREL(%rip) loads.  (movqs
become GOT_LOADs so that the linker knows they can be relaxed to leaqs, while
others, such as addq, become just GOT -- a pointer relocation -- since they
can't be relaxed in that way).

For example, this C file produces a private_extern GOT relocation when
compiled with -O2 with clang:

  extern const char kString[];
  const char* g(int a) { return kString + a; }

Linkers need to put pointer-relocated symbols into the GOT, but ld64 marks them
as LOCAL in the indirect symbol table. This matters, since `strip -x` looks at
the indirect symbol table when deciding what to strip.

The indirect symtab emitting code was assuming that only symbols that need
binding are in the GOT, but pointer relocations where there too. Hence, the
code needs to explicitly check if a symbol is a private extern.

Fixes https://crbug.com/1242638, which has some more information in comments 14
and 15. With this patch, the output of `nm -U` on Chromium Framework after
stripping now contains just two symbols when using lld, just like with ld64.


https://reviews.llvm.org/D111852

Files:
  lld/MachO/SyntheticSections.cpp
  lld/test/MachO/indirect-symtab.s


Index: lld/test/MachO/indirect-symtab.s
===================================================================
--- lld/test/MachO/indirect-symtab.s
+++ lld/test/MachO/indirect-symtab.s
@@ -2,8 +2,9 @@
 # RUN: rm -rf %t; split-file %s %t
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/libfoo.s -o %t/libfoo.o
 # RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/test.s -o %t/test.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/bar.s -o %t/bar.o
 # RUN: %lld -dylib %t/libfoo.o -o %t/libfoo.dylib -lSystem
-# RUN: %lld %t/test.o %t/libfoo.dylib -o %t/test -lSystem
+# RUN: %lld %t/test.o %t/bar.o %t/libfoo.dylib -o %t/test -lSystem
 # RUN: llvm-objdump --macho -d --no-show-raw-insn --indirect-symbols %t/test | FileCheck %s
 # RUN: llvm-otool -l %t/test | FileCheck --check-prefix=DYSYMTAB %s
 
@@ -11,6 +12,8 @@
 # CHECK-NEXT: _main:
 # CHECK-NEXT: movq  {{.*}}(%rip), %rax ## literal pool symbol address: _foo
 # CHECK-NEXT: movq  {{.*}}(%rip), %rax ## literal pool symbol address: _bar
+# CHECK-NEXT: leaq  _baz(%rip), %rax
+# CHECK-NEXT: addq  {{.*}}(%rip), %rax
 # CHECK-NEXT: movq  {{.*}}(%rip), %rax ## literal pool symbol address: _foo_tlv
 # CHECK-NEXT: movq  {{.*}}(%rip), %rax ## literal pool symbol address: _bar_tlv
 # CHECK-NEXT: callq {{.*}} ## symbol stub for: _foo_fn
@@ -21,8 +24,9 @@
 # CHECK-NEXT: address            index name
 # CHECK-NEXT: _bar_fn
 # CHECK-NEXT: _foo_fn
-# CHECK-NEXT: Indirect symbols for (__DATA_CONST,__got) 3 entries
+# CHECK-NEXT: Indirect symbols for (__DATA_CONST,__got) 4 entries
 # CHECK-NEXT: address            index name
+# CHECK-NEXT: LOCAL
 # CHECK-NEXT: _bar
 # CHECK-NEXT: _foo
 # CHECK-NEXT: _stub_binder
@@ -35,7 +39,7 @@
 # CHECK-NEXT: _bar_tlv
 # CHECK-NEXT: _foo_tlv
 
-# DYSYMTAB: nindirectsyms 9
+# DYSYMTAB: nindirectsyms 10
 
 #--- libfoo.s
 
@@ -44,6 +48,7 @@
 _foo_fn:
 _bar:
 _bar_fn:
+  ret
 
 .section  __DATA,__thread_vars,thread_local_variables
 .globl _foo_tlv, _bar_tlv
@@ -56,8 +61,19 @@
 _main:
   movq _foo at GOTPCREL(%rip), %rax
   movq _bar at GOTPCREL(%rip), %rax
+  movq _baz at GOTPCREL(%rip), %rax
+  addq _quux at GOTPCREL(%rip), %rax
   mov _foo_tlv at TLVP(%rip), %rax
   mov _bar_tlv at TLVP(%rip), %rax
   callq _foo_fn
   callq _bar_fn
   ret
+
+#--- bar.s
+.data
+.globl _baz,_quux
+.private_extern _baz,_quux
+_baz:
+.asciz "baz"
+_quux:
+.asciz "quux"
Index: lld/MachO/SyntheticSections.cpp
===================================================================
--- lld/MachO/SyntheticSections.cpp
+++ lld/MachO/SyntheticSections.cpp
@@ -1104,8 +1104,12 @@
 }
 
 static uint32_t indirectValue(const Symbol *sym) {
-  return sym->symtabIndex != UINT32_MAX ? sym->symtabIndex
-                                        : INDIRECT_SYMBOL_LOCAL;
+  if (sym->symtabIndex == UINT32_MAX)
+    return INDIRECT_SYMBOL_LOCAL;
+  if (auto *defined = dyn_cast<Defined>(sym))
+    if (defined->privateExtern)
+      return INDIRECT_SYMBOL_LOCAL;
+  return sym->symtabIndex;
 }
 
 void IndirectSymtabSection::writeTo(uint8_t *buf) const {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D111852.379874.patch
Type: text/x-patch
Size: 3030 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211014/73c3d08f/attachment.bin>