Skip to content

Commit e4ddbbb

Browse files
committed
Fix issues with GlobalMerge on Mach-O.
After PR llvm#101222, we GlobalMerge started making transforms which are unsafe on Mach-O platforms. Two issues, in particular, are fixed here: 1. We must never merge symbols in the `__cfstring` section, as the linker assumes each object in this section is only ever referenced directly, and that it can split the section as it likes. Previously, we avoded this problem because CFString literals are identified by private-linkage symbols. This patch adds a list of section-names to avoid merging, under Mach-O. 2. When the code was originally written, it had to be careful about emitting symbol aliases, due to issues with Mach-O's subsection splitting in the linker with `-dead_strip` enabled. This issue was fixed in 2016, via creation of the `.alt_entry` assembler directive, which allows creation of a symbol also implying the start of a new subsection. Unfortunately, Apple's ld-prime's support for this is buggy. Therefore, we must _continue_ to be careful not to emit such symbol aliases. The code already avoided it for InternalLinkage symbols, but after the triggering PR, we also need to avoid it for PrivateLinkage symbols. I will file an Apple bug-report about this issue, so that it can be fixed in a future version of ld-prime. Fixes llvm#104625
1 parent 315ba77 commit e4ddbbb

File tree

3 files changed

+80
-6
lines changed

3 files changed

+80
-6
lines changed

llvm/lib/CodeGen/GlobalMerge.cpp

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -578,12 +578,18 @@ bool GlobalMergeImpl::doMerge(const SmallVectorImpl<GlobalVariable *> &Globals,
578578
Globals[k]->replaceAllUsesWith(GEP);
579579
Globals[k]->eraseFromParent();
580580

581-
// When the linkage is not internal we must emit an alias for the original
582-
// variable name as it may be accessed from another object. On non-Mach-O
583-
// we can also emit an alias for internal linkage as it's safe to do so.
584-
// It's not safe on Mach-O as the alias (and thus the portion of the
585-
// MergedGlobals variable) may be dead stripped at link time.
586-
if (Linkage != GlobalValue::InternalLinkage || !IsMachO) {
581+
// Emit an alias for the original variable name. This is necessary for an
582+
// external symbol, as it may be accessed from another object. For
583+
// internal symbols, it's not strictly required, but it's useful.
584+
//
585+
// This _should_ also work on Mach-O ever since '.alt_entry' support was
586+
// added in 2016. Unfortunately, there's a bug in ld-prime (present at
587+
// least from Xcode 15.0 through Xcode 16.0), in which -dead_strip doesn't
588+
// always honor alt_entry. To workaround this issue, we don't emit aliases
589+
// on Mach-O. Except, we _must_ do so for external symbols. That means
590+
// MergeExternal is broken with that linker. (That option is currently off
591+
// by default on MachO).
592+
if (!IsMachO || Linkage == GlobalValue::ExternalLinkage) {
587593
GlobalAlias *GA = GlobalAlias::create(Tys[StructIdxs[idx]], AddrSpace,
588594
Linkage, Name, GEP, &M);
589595
GA->setVisibility(Visibility);
@@ -640,6 +646,17 @@ void GlobalMergeImpl::setMustKeepGlobalVariables(Module &M) {
640646
}
641647
}
642648

649+
// This function returns true if the given data Section name has custom
650+
// subsection-splitting semantics in Mach-O (such as splitting by a fixed size)
651+
//
652+
// See also ObjFile::parseSections and getRecordSize in lld/MachO/InputFiles.cpp
653+
static bool isSpecialMachOSection(StringRef Section) {
654+
// Uses starts_with, since section attributes can appear at the end of the name.
655+
return Section.starts_with("__DATA,__cfstring") ||
656+
Section.starts_with("__DATA,__objc_classrefs") ||
657+
Section.starts_with("__DATA,__objc_selrefs");
658+
}
659+
643660
bool GlobalMergeImpl::run(Module &M) {
644661
if (!EnableGlobalMerge)
645662
return false;
@@ -678,6 +695,10 @@ bool GlobalMergeImpl::run(Module &M) {
678695
unsigned AddressSpace = PT->getAddressSpace();
679696
StringRef Section = GV.getSection();
680697

698+
// On Mach-O, some section names have special semantics. Don't merge these.
699+
if (IsMachO && isSpecialMachOSection(Section))
700+
continue;
701+
681702
// Ignore all 'special' globals.
682703
if (GV.getName().starts_with("llvm.") || GV.getName().starts_with(".llvm."))
683704
continue;
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
; RUN: opt -global-merge -global-merge-max-offset=100 -S -o - %s | FileCheck %s
2+
; RUN: opt -passes='global-merge<max-offset=100>' -S -o - %s | FileCheck %s
3+
4+
;; Check that we do _not_ merge data with certain special section-names under Mach-O
5+
6+
target datalayout = "e-p:64:64"
7+
target triple = "x86_64-apple-macos11"
8+
9+
; CHECK-NOT: @_MergedGlobals
10+
11+
@cfstring1 = private global i32 1, section "__DATA,__cfstring"
12+
@cfstring2 = private global i32 2, section "__DATA,__cfstring"
13+
@objcclassrefs1 = private global i32 3, section "__DATA,__objc_classrefs,regular,no_dead_strip"
14+
@objcclassrefs2 = private global i32 4, section "__DATA,__objc_classrefs,regular,no_dead_strip"
15+
@objcselrefs1 = private global i32 5, section "__DATA,__objc_selrefs,literal_pointers,no_dead_strip"
16+
@objcselrefs2 = private global i32 6, section "__DATA,__objc_selrefs,literal_pointers,no_dead_strip"
17+
define void @use() {
18+
load ptr, ptr @cfstring1
19+
load ptr, ptr @cfstring2
20+
load ptr, ptr @objcclassrefs1
21+
load ptr, ptr @objcclassrefs2
22+
load ptr, ptr @objcselrefs1
23+
load ptr, ptr @objcselrefs2
24+
ret void
25+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
; RUN: opt -global-merge -global-merge-max-offset=100 -S -o - %s | FileCheck %s
2+
; RUN: opt -passes='global-merge<max-offset=100>' -S -o - %s | FileCheck %s
3+
4+
;; For Mach-O, we do not expect any alias symbols to be created for
5+
;; internal/private symbols by GlobalMerge.
6+
7+
target datalayout = "e-p:64:64"
8+
target triple = "x86_64-apple-macos11"
9+
10+
@a = private global i32 1
11+
@b = private global i32 2
12+
@c = internal global i32 3
13+
@d = internal global i32 4
14+
15+
; CHECK: @_MergedGlobals = internal global <{ i32, i32, i32, i32 }> <{ i32 1, i32 2, i32 3, i32 4 }>, align 4
16+
; CHECK-NOT: alias
17+
18+
define void @use() {
19+
; CHECK: load i32, ptr @_MergedGlobals,
20+
%x = load i32, ptr @a
21+
; CHECK: load i32, ptr getelementptr inbounds (<{ i32, i32, i32, i32 }>, ptr @_MergedGlobals, i32 0, i32 1)
22+
%y = load i32, ptr @b
23+
; CHECK: load i32, ptr getelementptr inbounds (<{ i32, i32, i32, i32 }>, ptr @_MergedGlobals, i32 0, i32 2)
24+
%z1 = load i32, ptr @c
25+
; CHECK: load i32, ptr getelementptr inbounds (<{ i32, i32, i32, i32 }>, ptr @_MergedGlobals, i32 0, i32 3)
26+
%z2 = load i32, ptr @d
27+
ret void
28+
}

0 commit comments

Comments
 (0)