[lld] 10cda6e - [lld/mac] Give range extension thunks for local symbols local visibility
Nico Weber via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 30 13:49:31 PDT 2022
Author: Nico Weber
Date: 2022-03-30T16:45:05-04:00
New Revision: 10cda6e36c05f83c8893e953b7a0f39ca8994ea3
URL: https://github.com/llvm/llvm-project/commit/10cda6e36c05f83c8893e953b7a0f39ca8994ea3
DIFF: https://github.com/llvm/llvm-project/commit/10cda6e36c05f83c8893e953b7a0f39ca8994ea3.diff
LOG: [lld/mac] Give range extension thunks for local symbols local visibility
When two local symbols (think: file-scope static functions, or functions in
unnamed namespaces) with the same name in two different translation units
both needed thunks, ld64.lld previously created external thunks for both
of them. These thunks ended up with the same name, leading to a duplicate
symbol error for the thunk symbols.
Instead, give thunks for local symbols local visibility.
(Hitting this requires a jump to a local symbol from over 128 MiB away.
It's unlikely that a single .o file is 128 MiB large, but with ICF
you can end up with a situation where the local symbol is ICF'd with
a symbol in a separate translation unit. And that can introduce a
large enough jump to require a thunk.)
Fixes PR54599.
Differential Revision: https://reviews.llvm.org/D122624
Added:
lld/test/MachO/arm64-thunk-visibility.s
Modified:
lld/MachO/ConcatOutputSection.cpp
lld/test/MachO/arm64-thunk-starvation.s
Removed:
################################################################################
diff --git a/lld/MachO/ConcatOutputSection.cpp b/lld/MachO/ConcatOutputSection.cpp
index 4cec418e7cf1b..0e70d69f7aa0f 100644
--- a/lld/MachO/ConcatOutputSection.cpp
+++ b/lld/MachO/ConcatOutputSection.cpp
@@ -57,14 +57,14 @@ void ConcatOutputSection::addInput(ConcatInputSection *input) {
// implement thunks. TODO: Adding support for branch islands!
//
// Internally -- as expressed in LLD's data structures -- a
-// branch-range-extension thunk comprises ...
+// branch-range-extension thunk consists of:
//
-// (1) new Defined privateExtern symbol for the thunk named
+// (1) new Defined symbol for the thunk named
// <FUNCTION>.thunk.<SEQUENCE>, which references ...
// (2) new InputSection, which contains ...
// (3.1) new data for the instructions to load & branch to the far address +
// (3.2) new Relocs on instructions to load the far address, which reference ...
-// (4.1) existing Defined extern symbol for the real function in __text, or
+// (4.1) existing Defined symbol for the real function in __text, or
// (4.2) existing DylibSymbol for the real function in a dylib
//
// Nearly-optimal thunk-placement algorithm features:
@@ -89,8 +89,8 @@ void ConcatOutputSection::addInput(ConcatInputSection *input) {
// goes out of range, we increment the sequence number and create a new
// thunk named <FUNCTION>.thunk.<SEQUENCE>.
//
-// * A thunk incarnation comprises (a) private-extern Defined symbol pointing
-// to (b) an InputSection holding machine instructions (similar to a MachO
+// * A thunk consists of (a) a Defined symbol pointing to
+// (b) an InputSection holding machine instructions (similar to a MachO
// stub), and (c) Reloc(s) that reference the real function for fixing-up
// the stub code.
//
@@ -323,11 +323,20 @@ void ConcatOutputSection::finalize() {
StringRef thunkName = saver().save(funcSym->getName() + ".thunk." +
std::to_string(thunkInfo.sequence++));
- r.referent = thunkInfo.sym = symtab->addDefined(
- thunkName, /*file=*/nullptr, thunkInfo.isec, /*value=*/0,
- /*size=*/thunkSize, /*isWeakDef=*/false, /*isPrivateExtern=*/true,
- /*isThumb=*/false, /*isReferencedDynamically=*/false,
- /*noDeadStrip=*/false, /*isWeakDefCanBeHidden=*/false);
+ if (!isa<Defined>(funcSym) || cast<Defined>(funcSym)->isExternal()) {
+ r.referent = thunkInfo.sym = symtab->addDefined(
+ thunkName, /*file=*/nullptr, thunkInfo.isec, /*value=*/0,
+ thunkSize, /*isWeakDef=*/false, /*isPrivateExtern=*/true,
+ /*isThumb=*/false, /*isReferencedDynamically=*/false,
+ /*noDeadStrip=*/false, /*isWeakDefCanBeHidden=*/false);
+ } else {
+ r.referent = thunkInfo.sym = make<Defined>(
+ thunkName, /*file=*/nullptr, thunkInfo.isec, /*value=*/0,
+ thunkSize, /*isWeakDef=*/false, /*isExternal=*/false,
+ /*isPrivateExtern=*/true, /*isThumb=*/false,
+ /*isReferencedDynamically=*/false, /*noDeadStrip=*/false,
+ /*isWeakDefCanBeHidden=*/false);
+ }
thunkInfo.sym->used = true;
target->populateThunk(thunkInfo.isec, funcSym);
finalizeOne(thunkInfo.isec);
diff --git a/lld/test/MachO/arm64-thunk-starvation.s b/lld/test/MachO/arm64-thunk-starvation.s
index 51249a120f95e..9e2b54f843c23 100644
--- a/lld/test/MachO/arm64-thunk-starvation.s
+++ b/lld/test/MachO/arm64-thunk-starvation.s
@@ -18,7 +18,7 @@ _f6: b _fn6
## Currently leaves 12 bytes for one thunk, so 36 bytes.
## Uses < instead of <=, so 40 bytes.
-.global _spacer1, _spacer1
+.global _spacer1, _spacer2
## 0x8000000 is 128 MiB, one more than the forward branch limit,
## distributed over two functions since our thunk insertion algorithm
## can't deal with a single function that's 128 MiB.
diff --git a/lld/test/MachO/arm64-thunk-visibility.s b/lld/test/MachO/arm64-thunk-visibility.s
new file mode 100644
index 0000000000000..0a697adf1357a
--- /dev/null
+++ b/lld/test/MachO/arm64-thunk-visibility.s
@@ -0,0 +1,83 @@
+# REQUIRES: aarch64
+
+# foo.s and bar.s both contain TU-local symbols (think static function)
+# with the same name, and both need a thunk.. This tests that ld64.lld doesn't
+# create a duplicate symbol for the two functions.
+
+# Test this both when the TU-local symbol is the branch source or target,
+# and for both forward and backwards jumps.
+
+# RUN: rm -rf %t; split-file %s %t
+# RUN: llvm-mc -filetype=obj -triple=arm64-apple-darwin %t/foo.s -o %t/foo.o
+# RUN: llvm-mc -filetype=obj -triple=arm64-apple-darwin %t/bar.s -o %t/bar.o
+# RUN: %lld -arch arm64 -lSystem -o %t.out %t/foo.o %t/bar.o
+
+#--- foo.s
+
+.subsections_via_symbols
+
+# Note: No .globl, since these are TU-local symbols.
+.p2align 2
+_early_jumping_local_fn: b _some_late_external
+.p2align 2
+_early_landing_local_fn: ret
+
+.globl _some_early_external
+.p2align 2
+_some_early_external: b _late_landing_local_fn
+
+## 0x8000000 is 128 MiB, one more than the forward branch limit.
+## Distribute that over two functions since our thunk insertion algorithm
+## can't deal with a single function that's 128 MiB.
+.global _spacer1, _spacer2
+_spacer1:
+.space 0x4000000
+_spacer2:
+.space 0x4000000
+
+# Note: No .globl, since these are TU-local symbols.
+.p2align 2
+_late_jumping_local_fn: b _some_early_external
+.p2align 2
+_late_landing_local_fn: ret
+
+.globl _some_late_external
+.p2align 2
+_some_late_external: b _early_landing_local_fn
+
+#--- bar.s
+
+.subsections_via_symbols
+
+# Note: No .globl, since these are TU-local symbols.
+.p2align 2
+_early_jumping_local_fn: b _some_other_late_external
+.p2align 2
+_early_landing_local_fn: ret
+
+.globl _some_other_early_external
+.p2align 2
+_some_other_early_external: b _late_landing_local_fn
+
+## 0x8000000 is 128 MiB, one more than the forward branch limit.
+## Distribute that over two functions since our thunk insertion algorithm
+## can't deal with a single function that's 128 MiB.
+.global _other_spacer1, _other_spacer1
+_spacer1:
+.space 0x4000000
+_spacer2:
+.space 0x4000000
+
+# Note: No .globl, since these are TU-local symbols.
+.p2align 2
+_late_jumping_local_fn: b _some_other_early_external
+.p2align 2
+_late_landing_local_fn: ret
+
+.globl _some_other_late_external
+.p2align 2
+_some_other_late_external: b _early_landing_local_fn
+
+.globl _main
+_main:
+ ret
More information about the llvm-commits
mailing list