[lld] a38ebed - [lld/mac] Implement support for .weak_def_can_be_hidden
Nico Weber via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 22 19:55:57 PDT 2021
Author: Nico Weber
Date: 2021-04-22T22:51:34-04:00
New Revision: a38ebed2581c7dee22f626aaf196bdba9ef9638d
URL: https://github.com/llvm/llvm-project/commit/a38ebed2581c7dee22f626aaf196bdba9ef9638d
DIFF: https://github.com/llvm/llvm-project/commit/a38ebed2581c7dee22f626aaf196bdba9ef9638d.diff
LOG: [lld/mac] Implement support for .weak_def_can_be_hidden
I first had a more invasive patch for this (D101069), but while trying
to get that polished for review I realized that lld's current symbol
merging semantics mean that only a very small code change is needed.
So this goes with the smaller patch for now.
This has no effect on projects that build with -fvisibility=hidden
(e.g. chromium), since these see .private_extern symbols instead.
It does have an effect on projects that build with -fvisibility-inlines-hidden
(e.g. llvm) in -O2 builds, where LLVM's GlobalOpt pass will promote most inline
functions from .weak_definition to .weak_def_can_be_hidden.
Before this patch:
% ls -l out/gn/bin/clang out/gn/lib/libclang.dylib
-rwxr-xr-x 1 thakis staff 113059936 Apr 22 11:51 out/gn/bin/clang
-rwxr-xr-x 1 thakis staff 86370064 Apr 22 11:51 out/gn/lib/libclang.dylib
% out/gn/bin/llvm-objdump --macho --weak-bind out/gn/bin/clang | wc -l
8291
% out/gn/bin/llvm-objdump --macho --weak-bind out/gn/lib/libclang.dylib | wc -l
5698
With this patch:
% ls -l out/gn/bin/clang out/gn/lib/libclang.dylib
-rwxr-xr-x 1 thakis staff 111721096 Apr 22 11:55 out/gn/bin/clang
-rwxr-xr-x 1 thakis staff 85291208 Apr 22 11:55 out/gn/lib/libclang.dylib
thakis at MBP llvm-project % out/gn/bin/llvm-objdump --macho --weak-bind out/gn/bin/clang | wc -l
725
thakis at MBP llvm-project % out/gn/bin/llvm-objdump --macho --weak-bind out/gn/lib/libclang.dylib | wc -l
542
Linking clang becomes a tiny bit faster with this patch:
x 100 0.67263818 0.77847815 0.69430709 0.69877208 0.017715892
+ 100 0.67209601 0.73323393 0.68600798 0.68917346 0.012824377
Difference at 95.0% confidence
-0.00959861 +/- 0.00428661
-1.37364% +/- 0.613449%
(Student's t, pooled s = 0.0154648)
This only happens if lld with the patch and lld without the patch are both
linked with an lld with the patch or both linked with an lld without the patch
(...or with ld64). I accidentally linked the lld with the patch with an lld
without the patch and the other way round at first. In that setup, no
difference is found. That makese sense, since having fewer weak imports will
make the linked output a bit faster too. So not only does this make linking
binaries such as clang a bit faster (since fewer exports need to be written to
the export trie by lld), the linked output binary binary is also a bit faster
(since dyld needs to process fewer dynamic imports).
This also happens to fix the one `check-clang` failure when using lld as host
linker, but mostly for silly reasons: See crbug.com/1183336, mostly comment 26.
The real bug here is that c-index-test links all of LLVM both statically and
dynamically, which is an ODR violation. Things just happen to work with this
patch.
So after this patch, check-clang, check-lld, check-llvm all pass with lld as
host linker :)
Differential Revision: https://reviews.llvm.org/D101080
Added:
lld/test/MachO/weak-def-can-be-hidden.s
Modified:
lld/MachO/InputFiles.cpp
Removed:
################################################################################
diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index b6a776d5ce09..bd6361801049 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -416,17 +416,62 @@ static macho::Symbol *createDefined(const NList &sym, StringRef name,
InputSection *isec, uint64_t value,
uint64_t size) {
// Symbol scope is determined by sym.n_type & (N_EXT | N_PEXT):
- // N_EXT: Global symbols
- // N_EXT | N_PEXT: Linkage unit (think: dylib) scoped
+ // N_EXT: Global symbols. These go in the symbol table during the link,
+ // and also in the export table of the output so that the dynamic
+ // linker sees them.
+ // N_EXT | N_PEXT: Linkage unit (think: dylib) scoped. These go in the
+ // symbol table during the link so that duplicates are
+ // either reported (for non-weak symbols) or merged
+ // (for weak symbols), but they do not go in the export
+ // table of the output.
// N_PEXT: Does not occur in input files in practice,
// a private extern must be external.
- // 0: Translation-unit scoped. These are not in the symbol table.
+ // 0: Translation-unit scoped. These are not in the symbol table during
+ // link, and not in the export table of the output either.
+
+ bool isWeakDefCanBeHidden =
+ (sym.n_desc & (N_WEAK_DEF | N_WEAK_REF)) == (N_WEAK_DEF | N_WEAK_REF);
if (sym.n_type & (N_EXT | N_PEXT)) {
assert((sym.n_type & N_EXT) && "invalid input");
+ bool isPrivateExtern = sym.n_type & N_PEXT;
+
+ // lld's behavior for merging symbols is slightly
diff erent from ld64:
+ // ld64 picks the winning symbol based on several criteria (see
+ // pickBetweenRegularAtoms() in ld64's SymbolTable.cpp), while lld
+ // just merges metadata and keeps the contents of the first symbol
+ // with that name (see SymbolTable::addDefined). For:
+ // * inline function F in a TU built with -fvisibility-inlines-hidden
+ // * and inline function F in another TU built without that flag
+ // ld64 will pick the one from the file built without
+ // -fvisibility-inlines-hidden.
+ // lld will instead pick the one listed first on the link command line and
+ // give it visibility as if the function was built without
+ // -fvisibility-inlines-hidden.
+ // If both functions have the same contents, this will have the same
+ // behavior. If not, it won't, but the input had an ODR violation in
+ // that case.
+ //
+ // Similarly, merging a symbol
+ // that's isPrivateExtern and not isWeakDefCanBeHidden with one
+ // that's not isPrivateExtern but isWeakDefCanBeHidden technically
+ // should produce one
+ // that's not isPrivateExtern but isWeakDefCanBeHidden. That matters
+ // with ld64's semantics, because it means the non-private-extern
+ // definition will continue to take priority if more private extern
+ // definitions are encountered. With lld's semantics there's no observable
+ //
diff erence between a symbol that's isWeakDefCanBeHidden or one that's
+ // privateExtern -- neither makes it into the dynamic symbol table. So just
+ // promote isWeakDefCanBeHidden to isPrivateExtern here.
+ if (isWeakDefCanBeHidden)
+ isPrivateExtern = true;
+
return symtab->addDefined(name, isec->file, isec, value, size,
- sym.n_desc & N_WEAK_DEF, sym.n_type & N_PEXT);
+ sym.n_desc & N_WEAK_DEF, isPrivateExtern);
}
+
+ assert(!isWeakDefCanBeHidden &&
+ "weak_def_can_be_hidden on already-hidden symbol?");
return make<Defined>(name, isec->file, isec, value, size,
sym.n_desc & N_WEAK_DEF,
/*isExternal=*/false, /*isPrivateExtern=*/false);
diff --git a/lld/test/MachO/weak-def-can-be-hidden.s b/lld/test/MachO/weak-def-can-be-hidden.s
new file mode 100644
index 000000000000..4552582df3d1
--- /dev/null
+++ b/lld/test/MachO/weak-def-can-be-hidden.s
@@ -0,0 +1,137 @@
+# REQUIRES: x86
+# RUN: rm -rf %t; split-file %s %t
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \
+# RUN: %t/weak-foo.s -o %t/weak-foo.o
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \
+# RUN: %t/weak-autohide-foo.s -o %t/weak-autohide-foo.o
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \
+# RUN: %t/weak-foo-pe.s -o %t/weak-foo-pe.o
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \
+# RUN: %t/weak-autohide-foo-pe.s -o %t/weak-autohide-foo-pe.o
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos \
+# RUN: %t/ref-foo.s -o %t/ref-foo.o
+
+## Basics: A weak_def_can_be_hidden symbol should not be in the output's
+## export table, nor in the weak bind table, and references to it from
+## within the dylib should not use weak indirect lookups.
+## Think: Inline function compiled without any -fvisibility flags, inline
+## function does not have its address taken. In -O2 compiles, GlobalOpt will
+## upgrade the inline function from .weak_definition to .weak_def_can_be_hidden.
+# RUN: %lld -dylib -o %t/weak-autohide-foo.dylib \
+# RUN: %t/weak-autohide-foo.o %t/ref-foo.o
+# RUN: llvm-objdump --syms --exports-trie %t/weak-autohide-foo.dylib | \
+# RUN: FileCheck --check-prefix=EXPORTS %s
+# RUN: llvm-nm -m %t/weak-autohide-foo.dylib | \
+# RUN: FileCheck --check-prefix=EXPORTS-NM %s
+# RUN: llvm-objdump --macho --bind --weak-bind %t/weak-autohide-foo.dylib | \
+# RUN: FileCheck --check-prefix=WEAKBIND %s
+# RUN: llvm-objdump --macho --private-header %t/weak-autohide-foo.dylib | \
+# RUN: FileCheck --check-prefix=HEADERS %s
+
+## ld64 doesn't treat .weak_def_can_be_hidden as weak symbols in the
+## exports trie or bind tables, but it claims they are weak in the symbol
+## table. lld marks them as local in the symbol table, see also test
+## weak-private-extern.s. These FileCheck lines match both outputs.
+# EXPORTS-LABEL: SYMBOL TABLE:
+# EXPORTS-DAG: [[#%x, FOO_ADDR:]] {{.*}} _foo
+# EXPORTS-LABEL: Exports trie:
+# EXPORTS-NOT: 0x{{0*}}[[#%X, FOO_ADDR]] _foo
+
+## nm output for .weak_def_can_be_hidden says "was a private external" even
+## though it wasn't .private_extern: It was just .weak_def_can_be_hidden.
+## This matches ld64.
+# EXPORTS-NM: (__TEXT,__text) non-external (was a private external) _foo
+
+# WEAKBIND-NOT: __got
+# WEAKBIND-NOT: __la_symbol_ptr
+
+## ld64 sets WEAKBIND and BINDS_TO_WEAK in the mach-o header even though there
+## are no weak bindings or weak definitions after processing the autohide. That
+## looks like a bug in ld64 (?) If you change lit.local.cfg to set %lld to ld to
+## test compatibility, you have to add some arbitrary suffix to these two lines:
+# HEADERS-NOT: WEAK_DEFINES
+# HEADERS-NOT: BINDS_TO_WEAK
+
+## Same behavior for a symbol that's both .weak_def_can_be_hidden and
+## .private_extern. Think: Inline function compiled with
+## -fvisibility-inlines-hidden.
+# RUN: %lld -dylib -o %t/weak-autohide-foo-pe.dylib \
+# RUN: %t/weak-autohide-foo-pe.o %t/ref-foo.o
+# RUN: llvm-objdump --syms --exports-trie %t/weak-autohide-foo-pe.dylib | \
+# RUN: FileCheck --check-prefix=EXPORTS %s
+# RUN: llvm-nm -m %t/weak-autohide-foo-pe.dylib | \
+# RUN: FileCheck --check-prefix=EXPORTS-NM %s
+# RUN: llvm-objdump --macho --bind --weak-bind %t/weak-autohide-foo-pe.dylib | \
+# RUN: FileCheck --check-prefix=WEAKBIND %s
+# RUN: llvm-objdump --macho --private-header %t/weak-autohide-foo-pe.dylib | \
+# RUN: FileCheck --check-prefix=HEADERS %s
+
+## In fact, a regular weak symbol that's .private_extern behaves the same
+## as well.
+# RUN: %lld -dylib -o %t/weak-foo-pe.dylib %t/weak-foo-pe.o %t/ref-foo.o
+# RUN: llvm-objdump --syms --exports-trie %t/weak-foo-pe.dylib | \
+# RUN: FileCheck --check-prefix=EXPORTS %s
+# RUN: llvm-nm -m %t/weak-foo-pe.dylib | \
+# RUN: FileCheck --check-prefix=EXPORTS-NM %s
+# RUN: llvm-objdump --macho --bind --weak-bind %t/weak-foo-pe.dylib | \
+# RUN: FileCheck --check-prefix=WEAKBIND %s
+# RUN: llvm-objdump --macho --private-header %t/weak-foo-pe.dylib | \
+# RUN: FileCheck --check-prefix=HEADERS %s
+
+## Combining a regular weak_definition with a weak_def_can_be_hidden produces
+## a regular weak external.
+# RUN: %lld -dylib -o %t/weak-foo.dylib -lSystem \
+# RUN: %t/weak-autohide-foo.o %t/weak-foo.o %t/ref-foo.o
+# RUN: llvm-objdump --syms --exports-trie %t/weak-foo.dylib | \
+# RUN: FileCheck --check-prefix=WEAK %s
+# RUN: llvm-nm -m %t/weak-foo.dylib | \
+# RUN: FileCheck --check-prefix=WEAK-NM %s
+# RUN: llvm-objdump --macho --bind --weak-bind %t/weak-foo.dylib | \
+# RUN: FileCheck --check-prefix=WEAK-WEAKBIND %s
+# RUN: llvm-objdump --macho --private-header %t/weak-foo.dylib | \
+# RUN: FileCheck --check-prefix=WEAK-HEADERS %s
+# WEAK-LABEL: SYMBOL TABLE:
+# WEAK-DAG: [[#%x, FOO_ADDR:]] w {{.*}} _foo
+# WEAK-LABEL: Exports trie:
+# WEAK-DAG: 0x{{0*}}[[#%X, FOO_ADDR]] _foo
+# WEAK-NM: (__TEXT,__text) weak external _foo
+# WEAK-WEAKBIND: __la_symbol_ptr 0x{{.*}} pointer 0 _foo
+# WEAK-HEADERS: WEAK_DEFINES
+# WEAK-HEADERS: BINDS_TO_WEAK
+
+#--- weak-foo.s
+.globl _foo
+.weak_definition _foo
+_foo:
+ retq
+
+#--- weak-autohide-foo.s
+.globl _foo
+.weak_def_can_be_hidden _foo
+_foo:
+ retq
+
+#--- weak-foo-pe.s
+.private_extern _foo
+.globl _foo
+.weak_definition _foo
+_foo:
+ retq
+
+#--- weak-autohide-foo-pe.s
+.private_extern _foo
+.globl _foo
+.weak_def_can_be_hidden _foo
+_foo:
+ retq
+
+#--- ref-foo.s
+.globl _bar
+_bar:
+ callq _foo
+ retq
More information about the llvm-commits
mailing list