Skip to content

Commit 97a4324

Browse files
authored
[lld-macho] Fix ICF differentiation of safe_thunks relocs (#111811)
In `--icf=safe_thunks` mode, the linker differentiates `keepUnique` functions by creating thunks during a post-processing step after Identical Code Folding (ICF). While this ensures that `keepUnique` functions themselves are not incorrectly merged, it overlooks functions that reference these `keepUnique` symbols. If two functions are identical except for references to different `keepUnique` functions, the current ICF algorithm incorrectly considers them identical because it doesn't account for the future differentiation introduced by thunks. This leads to incorrect deduplication of functions that should remain distinct. To address this issue, we modify the ICF comparison to explicitly check for references to `keepUnique` functions during deduplication. By doing so, functions that reference different `keepUnique` symbols are correctly identified as distinct, preventing erroneous merging and ensuring the correctness of the linked output.
1 parent fc467b4 commit 97a4324

File tree

2 files changed

+60
-0
lines changed

2 files changed

+60
-0
lines changed

lld/MachO/ICF.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,17 @@ bool ICF::equalsConstant(const ConcatInputSection *ia,
147147
isecB = rb.referent.get<InputSection *>();
148148
}
149149

150+
// Typically, we should not encounter sections marked with `keepUnique` at
151+
// this point as they would have resulted in different hashes and therefore
152+
// no need for a full comparison.
153+
// However, in `safe_thunks` mode, it's possible for two different
154+
// relocations to reference identical `keepUnique` functions that will be
155+
// distinguished later via thunks - so we need to handle this case
156+
// explicitly.
157+
if ((isecA != isecB) && ((isecA->keepUnique && isCodeSection(isecA)) ||
158+
(isecB->keepUnique && isCodeSection(isecB))))
159+
return false;
160+
150161
if (isecA->parent != isecB->parent)
151162
return false;
152163
// Sections with identical parents should be of the same kind.

lld/test/MachO/icf-safe-thunks.ll

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,13 @@
2222
; CHECK-ARM64-NEXT: _func_3identical_v3_canmerge:
2323
; CHECK-ARM64-NEXT: mov {{.*}}, #0x21
2424
;
25+
; CHECK-ARM64: _func_call_thunked_1_nomerge:
26+
; CHECK-ARM64-NEXT: stp x29
27+
;
28+
; CHECK-ARM64: _func_call_thunked_2_nomerge:
29+
; CHECK-ARM64-NEXT: _func_call_thunked_2_merge:
30+
; CHECK-ARM64-NEXT: stp x29
31+
;
2532
; CHECK-ARM64: _call_all_funcs:
2633
; CHECK-ARM64-NEXT: stp x29
2734
;
@@ -43,6 +50,9 @@
4350
; CHECK-ARM64-MAP-NEXT: 0x00000010 [ 2] _func_3identical_v1_canmerge
4451
; CHECK-ARM64-MAP-NEXT: 0x00000000 [ 2] _func_3identical_v2_canmerge
4552
; CHECK-ARM64-MAP-NEXT: 0x00000000 [ 2] _func_3identical_v3_canmerge
53+
; CHECK-ARM64-MAP-NEXT: 0x00000020 [ 2] _func_call_thunked_1_nomerge
54+
; CHECK-ARM64-MAP-NEXT: 0x00000020 [ 2] _func_call_thunked_2_nomerge
55+
; CHECK-ARM64-MAP-NEXT: 0x00000000 [ 2] _func_call_thunked_2_merge
4656
; CHECK-ARM64-MAP-NEXT: 0x00000034 [ 2] _call_all_funcs
4757
; CHECK-ARM64-MAP-NEXT: 0x00000050 [ 2] _take_func_addr
4858
; CHECK-ARM64-MAP-NEXT: 0x00000004 [ 2] _func_2identical_v2
@@ -125,6 +135,30 @@ entry:
125135
ret void
126136
}
127137

138+
; Function Attrs: mustprogress nofree noinline norecurse nounwind ssp memory(readwrite, argmem: none) uwtable(sync)
139+
define void @func_call_thunked_1_nomerge() local_unnamed_addr #0 {
140+
entry:
141+
tail call void @func_2identical_v1()
142+
store volatile i8 77, ptr @g_val, align 1, !tbaa !5
143+
ret void
144+
}
145+
146+
; Function Attrs: mustprogress nofree noinline norecurse nounwind ssp memory(readwrite, argmem: none) uwtable(sync)
147+
define void @func_call_thunked_2_nomerge() local_unnamed_addr #0 {
148+
entry:
149+
tail call void @func_2identical_v2()
150+
store volatile i8 77, ptr @g_val, align 1, !tbaa !5
151+
ret void
152+
}
153+
154+
; Function Attrs: mustprogress nofree noinline norecurse nounwind ssp memory(readwrite, argmem: none) uwtable(sync)
155+
define void @func_call_thunked_2_merge() local_unnamed_addr #0 {
156+
entry:
157+
tail call void @func_2identical_v2()
158+
store volatile i8 77, ptr @g_val, align 1, !tbaa !5
159+
ret void
160+
}
161+
128162
; Function Attrs: mustprogress nofree noinline norecurse nounwind ssp uwtable(sync)
129163
define void @call_all_funcs() local_unnamed_addr #1 {
130164
entry:
@@ -227,6 +261,21 @@ attributes #1 = { mustprogress nofree noinline norecurse nounwind ssp uwtable(sy
227261
; g_val = 33;
228262
; }
229263
;
264+
; ATTR void func_call_thunked_1_nomerge() {
265+
; func_2identical_v1();
266+
; g_val = 77;
267+
; }
268+
;
269+
; ATTR void func_call_thunked_2_nomerge() {
270+
; func_2identical_v2();
271+
; g_val = 77;
272+
; }
273+
;
274+
; ATTR void func_call_thunked_2_merge() {
275+
; func_2identical_v2();
276+
; g_val = 77;
277+
; }
278+
;
230279
; ATTR void call_all_funcs() {
231280
; func_unique_1();
232281
; func_unique_2_canmerge();

0 commit comments

Comments
 (0)