Skip to content

Commit 6c2f26a

Browse files
int3int3
committed
[lld-macho] -all_load and -ObjC should not affect LC_LINKER_OPTION flags
In particular, they should not cause archives to be eagerly loaded. This matches ld64's behavior. Fixes PR52246. Reviewed By: #lld-macho, thakis Differential Revision: https://reviews.llvm.org/D112756
1 parent a271f24 commit 6c2f26a

File tree

3 files changed

+63
-28
lines changed

3 files changed

+63
-28
lines changed

‎lld/MachO/Config.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,13 @@ struct SymbolPriorityEntry {
198198
llvm::DenseMap<llvm::StringRef, size_t> objectFiles;
199199
};
200200

201+
// Whether to force-load an archive.
202+
enum class ForceLoad {
203+
Default, // Apply -all_load or -ObjC behaviors if those flags are enabled
204+
Yes, // Always load the archive, regardless of other flags
205+
No, // Never load the archive, regardless of other flags
206+
};
207+
201208
extern Configuration *config;
202209

203210
} // namespace macho

‎lld/MachO/Driver.cpp

Lines changed: 29 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ static llvm::CachePruningPolicy getLTOCachePolicy(InputArgList &args) {
226226

227227
static DenseMap<StringRef, ArchiveFile *> loadedArchives;
228228

229-
static InputFile *addFile(StringRef path, bool forceLoadArchive,
229+
static InputFile *addFile(StringRef path, ForceLoad forceLoadArchive,
230230
bool isExplicit = true, bool isBundleLoader = false) {
231231
Optional<MemoryBufferRef> buffer = readFile(path);
232232
if (!buffer)
@@ -253,11 +253,13 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive,
253253
error(path + ": archive has no index; run ranlib to add one");
254254

255255
auto *file = make<ArchiveFile>(std::move(archive));
256-
if (config->allLoad || forceLoadArchive) {
256+
if ((forceLoadArchive == ForceLoad::Default && config->allLoad) ||
257+
forceLoadArchive == ForceLoad::Yes) {
257258
if (Optional<MemoryBufferRef> buffer = readFile(path)) {
258259
Error e = Error::success();
259260
for (const object::Archive::Child &c : file->getArchive().children(e)) {
260-
StringRef reason = forceLoadArchive ? "-force_load" : "-all_load";
261+
StringRef reason =
262+
forceLoadArchive == ForceLoad::Yes ? "-force_load" : "-all_load";
261263
if (Error e = file->fetch(c, reason))
262264
error(toString(file) + ": " + reason +
263265
" failed to load archive member: " + toString(std::move(e)));
@@ -266,7 +268,8 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive,
266268
error(toString(file) +
267269
": Archive::children failed: " + toString(std::move(e)));
268270
}
269-
} else if (config->forceLoadObjC) {
271+
} else if (forceLoadArchive == ForceLoad::Default &&
272+
config->forceLoadObjC) {
270273
for (const object::Archive::Symbol &sym : file->getArchive().symbols())
271274
if (sym.getName().startswith(objc::klass))
272275
file->fetch(sym);
@@ -331,10 +334,11 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive,
331334
}
332335

333336
static void addLibrary(StringRef name, bool isNeeded, bool isWeak,
334-
bool isReexport, bool isExplicit, bool forceLoad) {
337+
bool isReexport, bool isExplicit,
338+
ForceLoad forceLoadArchive) {
335339
if (Optional<StringRef> path = findLibrary(name)) {
336340
if (auto *dylibFile = dyn_cast_or_null<DylibFile>(
337-
addFile(*path, forceLoad, isExplicit))) {
341+
addFile(*path, forceLoadArchive, isExplicit))) {
338342
if (isNeeded)
339343
dylibFile->forceNeeded = true;
340344
if (isWeak)
@@ -350,10 +354,11 @@ static void addLibrary(StringRef name, bool isNeeded, bool isWeak,
350354
}
351355

352356
static void addFramework(StringRef name, bool isNeeded, bool isWeak,
353-
bool isReexport, bool isExplicit) {
357+
bool isReexport, bool isExplicit,
358+
ForceLoad forceLoadArchive) {
354359
if (Optional<StringRef> path = findFramework(name)) {
355360
if (auto *dylibFile = dyn_cast_or_null<DylibFile>(
356-
addFile(*path, /*forceLoadArchive=*/false, isExplicit))) {
361+
addFile(*path, forceLoadArchive, isExplicit))) {
357362
if (isNeeded)
358363
dylibFile->forceNeeded = true;
359364
if (isWeak)
@@ -392,15 +397,16 @@ void macho::parseLCLinkerOption(InputFile *f, unsigned argc, StringRef data) {
392397
switch (arg->getOption().getID()) {
393398
case OPT_l: {
394399
StringRef name = arg->getValue();
395-
bool forceLoad =
396-
config->forceLoadSwift ? name.startswith("swift") : false;
400+
ForceLoad forceLoadArchive =
401+
config->forceLoadSwift && name.startswith("swift") ? ForceLoad::Yes
402+
: ForceLoad::No;
397403
addLibrary(name, /*isNeeded=*/false, /*isWeak=*/false,
398-
/*isReexport=*/false, /*isExplicit=*/false, forceLoad);
404+
/*isReexport=*/false, /*isExplicit=*/false, forceLoadArchive);
399405
break;
400406
}
401407
case OPT_framework:
402408
addFramework(arg->getValue(), /*isNeeded=*/false, /*isWeak=*/false,
403-
/*isReexport=*/false, /*isExplicit=*/false);
409+
/*isReexport=*/false, /*isExplicit=*/false, ForceLoad::No);
404410
break;
405411
default:
406412
error(arg->getSpelling() + " is not allowed in LC_LINKER_OPTION");
@@ -414,7 +420,7 @@ static void addFileList(StringRef path) {
414420
return;
415421
MemoryBufferRef mbref = *buffer;
416422
for (StringRef path : args::getLines(mbref))
417-
addFile(rerootPath(path), /*forceLoadArchive=*/false);
423+
addFile(rerootPath(path), ForceLoad::Default);
418424
}
419425

420426
// An order file has one entry per line, in the following format:
@@ -947,46 +953,47 @@ static void createFiles(const InputArgList &args) {
947953

948954
switch (opt.getID()) {
949955
case OPT_INPUT:
950-
addFile(rerootPath(arg->getValue()), /*forceLoadArchive=*/false);
956+
addFile(rerootPath(arg->getValue()), ForceLoad::Default);
951957
break;
952958
case OPT_needed_library:
953959
if (auto *dylibFile = dyn_cast_or_null<DylibFile>(
954-
addFile(rerootPath(arg->getValue()), false)))
960+
addFile(rerootPath(arg->getValue()), ForceLoad::Default)))
955961
dylibFile->forceNeeded = true;
956962
break;
957963
case OPT_reexport_library:
958-
if (auto *dylibFile = dyn_cast_or_null<DylibFile>(addFile(
959-
rerootPath(arg->getValue()), /*forceLoadArchive=*/false))) {
964+
if (auto *dylibFile = dyn_cast_or_null<DylibFile>(
965+
addFile(rerootPath(arg->getValue()), ForceLoad::Default))) {
960966
config->hasReexports = true;
961967
dylibFile->reexport = true;
962968
}
963969
break;
964970
case OPT_weak_library:
965971
if (auto *dylibFile = dyn_cast_or_null<DylibFile>(
966-
addFile(rerootPath(arg->getValue()), /*forceLoadArchive=*/false)))
972+
addFile(rerootPath(arg->getValue()), ForceLoad::Default)))
967973
dylibFile->forceWeakImport = true;
968974
break;
969975
case OPT_filelist:
970976
addFileList(arg->getValue());
971977
break;
972978
case OPT_force_load:
973-
addFile(rerootPath(arg->getValue()), /*forceLoadArchive=*/true);
979+
addFile(rerootPath(arg->getValue()), ForceLoad::Yes);
974980
break;
975981
case OPT_l:
976982
case OPT_needed_l:
977983
case OPT_reexport_l:
978984
case OPT_weak_l:
979985
addLibrary(arg->getValue(), opt.getID() == OPT_needed_l,
980986
opt.getID() == OPT_weak_l, opt.getID() == OPT_reexport_l,
981-
/*isExplicit=*/true, /*forceLoad=*/false);
987+
/*isExplicit=*/true, ForceLoad::Default);
982988
break;
983989
case OPT_framework:
984990
case OPT_needed_framework:
985991
case OPT_reexport_framework:
986992
case OPT_weak_framework:
987993
addFramework(arg->getValue(), opt.getID() == OPT_needed_framework,
988994
opt.getID() == OPT_weak_framework,
989-
opt.getID() == OPT_reexport_framework, /*isExplicit=*/true);
995+
opt.getID() == OPT_reexport_framework, /*isExplicit=*/true,
996+
ForceLoad::Default);
990997
break;
991998
default:
992999
break;
@@ -1175,7 +1182,7 @@ bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
11751182
if (const Arg *arg = args.getLastArg(OPT_bundle_loader)) {
11761183
if (config->outputType != MH_BUNDLE)
11771184
error("-bundle_loader can only be used with MachO bundle output");
1178-
addFile(arg->getValue(), /*forceLoadArchive=*/false, /*isExplicit=*/false,
1185+
addFile(arg->getValue(), ForceLoad::Default, /*isExplicit=*/false,
11791186
/*isBundleLoader=*/true);
11801187
}
11811188
if (const Arg *arg = args.getLastArg(OPT_umbrella)) {

‎lld/test/MachO/lc-linker-option.ll

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,24 @@
5858
;; actual archive at Foo.framework/Versions/Current, but we skip that here so
5959
;; that this test can run on Windows.
6060
; RUN: llvm-ar rcs %t/Foo.framework/Foo %t/foo.o
61-
; RUN: llc %t/objfile1.ll -o %t/objfile1.o -filetype=obj
61+
; RUN: llc %t/load-framework-foo.ll -o %t/load-framework-foo.o -filetype=obj
6262
; RUN: llc %t/main.ll -o %t/main.o -filetype=obj
63-
; RUN: %lld %t/objfile1.o %t/main.o -o /dev/null -F%t
63+
; RUN: %lld %t/load-framework-foo.o %t/main.o -o %t/main -F%t
64+
; RUN: llvm-objdump --macho --syms %t/main | FileCheck %s --check-prefix=SYMS
65+
66+
;; Make sure -all_load and -ObjC have no effect on libraries loaded via
67+
;; LC_LINKER_OPTION flags.
68+
; RUN: llc %t/load-library-foo.ll -o %t/load-library-foo.o -filetype=obj
69+
; RUN: llvm-ar rcs %t/libfoo.a %t/foo.o
70+
; RUN: %lld -all_load -ObjC %t/load-framework-foo.o %t/load-library-foo.o \
71+
; RUN: %t/main.o -o %t/main -F%t -L%t
72+
; RUN: llvm-objdump --macho --syms %t/main | FileCheck %s --check-prefix=SYMS
73+
74+
;; Note that _OBJC_CLASS_$_TestClass is *not* included here.
75+
; SYMS: SYMBOL TABLE:
76+
; SYMS-NEXT: g F __TEXT,__text _main
77+
; SYMS-NEXT: g F __TEXT,__text __mh_execute_header
78+
; SYMS-EMPTY:
6479

6580
;--- framework.ll
6681
target triple = "x86_64-apple-macosx10.15.0"
@@ -105,13 +120,20 @@ define void @main() {
105120
ret void
106121
}
107122

108-
;--- objfile1.ll
123+
;--- load-framework-foo.ll
109124
target triple = "x86_64-apple-macosx10.15.0"
110125
target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
111126

112127
!0 = !{!"-framework", !"Foo"}
113128
!llvm.linker.options = !{!0}
114129

130+
;--- load-library-foo.ll
131+
target triple = "x86_64-apple-macosx10.15.0"
132+
target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
133+
134+
!0 = !{!"-lfoo"}
135+
!llvm.linker.options = !{!0}
136+
115137
;--- main.ll
116138
target triple = "x86_64-apple-macosx10.15.0"
117139
target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
@@ -127,6 +149,5 @@ define void @main() {
127149
target triple = "x86_64-apple-macosx10.15.0"
128150
target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
129151

130-
define void @foo() {
131-
ret void
132-
}
152+
%struct._class_t = type {}
153+
@"OBJC_CLASS_$_TestClass" = global %struct._class_t {}, section "__DATA, __objc_data", align 8

0 commit comments

Comments
 (0)