Skip to content

Commit

Permalink
[lld-macho] -all_load and -ObjC should not affect LC_LINKER_OPTION flags
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@int3
int3 committed Oct 29, 2021
1 parent a271f24 commit 6c2f26a
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 28 deletions.
7 changes: 7 additions & 0 deletions lld/MachO/Config.h
View file
Expand Up @@ -198,6 +198,13 @@ struct SymbolPriorityEntry {
llvm::DenseMap<llvm::StringRef, size_t> objectFiles;
};

// Whether to force-load an archive.
enum class ForceLoad {
Default, // Apply -all_load or -ObjC behaviors if those flags are enabled
Yes, // Always load the archive, regardless of other flags
No, // Never load the archive, regardless of other flags
};

extern Configuration *config;

} // namespace macho
Expand Down
51 changes: 29 additions & 22 deletions lld/MachO/Driver.cpp
View file
Expand Up @@ -226,7 +226,7 @@ static llvm::CachePruningPolicy getLTOCachePolicy(InputArgList &args) {

static DenseMap<StringRef, ArchiveFile *> loadedArchives;

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

auto *file = make<ArchiveFile>(std::move(archive));
if (config->allLoad || forceLoadArchive) {
if ((forceLoadArchive == ForceLoad::Default && config->allLoad) ||
forceLoadArchive == ForceLoad::Yes) {
if (Optional<MemoryBufferRef> buffer = readFile(path)) {
Error e = Error::success();
for (const object::Archive::Child &c : file->getArchive().children(e)) {
StringRef reason = forceLoadArchive ? "-force_load" : "-all_load";
StringRef reason =
forceLoadArchive == ForceLoad::Yes ? "-force_load" : "-all_load";
if (Error e = file->fetch(c, reason))
error(toString(file) + ": " + reason +
" failed to load archive member: " + toString(std::move(e)));
Expand All @@ -266,7 +268,8 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive,
error(toString(file) +
": Archive::children failed: " + toString(std::move(e)));
}
} else if (config->forceLoadObjC) {
} else if (forceLoadArchive == ForceLoad::Default &&
config->forceLoadObjC) {
for (const object::Archive::Symbol &sym : file->getArchive().symbols())
if (sym.getName().startswith(objc::klass))
file->fetch(sym);
Expand Down Expand Up @@ -331,10 +334,11 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive,
}

static void addLibrary(StringRef name, bool isNeeded, bool isWeak,
bool isReexport, bool isExplicit, bool forceLoad) {
bool isReexport, bool isExplicit,
ForceLoad forceLoadArchive) {
if (Optional<StringRef> path = findLibrary(name)) {
if (auto *dylibFile = dyn_cast_or_null<DylibFile>(
addFile(*path, forceLoad, isExplicit))) {
addFile(*path, forceLoadArchive, isExplicit))) {
if (isNeeded)
dylibFile->forceNeeded = true;
if (isWeak)
Expand All @@ -350,10 +354,11 @@ static void addLibrary(StringRef name, bool isNeeded, bool isWeak,
}

static void addFramework(StringRef name, bool isNeeded, bool isWeak,
bool isReexport, bool isExplicit) {
bool isReexport, bool isExplicit,
ForceLoad forceLoadArchive) {
if (Optional<StringRef> path = findFramework(name)) {
if (auto *dylibFile = dyn_cast_or_null<DylibFile>(
addFile(*path, /*forceLoadArchive=*/false, isExplicit))) {
addFile(*path, forceLoadArchive, isExplicit))) {
if (isNeeded)
dylibFile->forceNeeded = true;
if (isWeak)
Expand Down Expand Up @@ -392,15 +397,16 @@ void macho::parseLCLinkerOption(InputFile *f, unsigned argc, StringRef data) {
switch (arg->getOption().getID()) {
case OPT_l: {
StringRef name = arg->getValue();
bool forceLoad =
config->forceLoadSwift ? name.startswith("swift") : false;
ForceLoad forceLoadArchive =
config->forceLoadSwift && name.startswith("swift") ? ForceLoad::Yes
: ForceLoad::No;
addLibrary(name, /*isNeeded=*/false, /*isWeak=*/false,
/*isReexport=*/false, /*isExplicit=*/false, forceLoad);
/*isReexport=*/false, /*isExplicit=*/false, forceLoadArchive);
break;
}
case OPT_framework:
addFramework(arg->getValue(), /*isNeeded=*/false, /*isWeak=*/false,
/*isReexport=*/false, /*isExplicit=*/false);
/*isReexport=*/false, /*isExplicit=*/false, ForceLoad::No);
break;
default:
error(arg->getSpelling() + " is not allowed in LC_LINKER_OPTION");
Expand All @@ -414,7 +420,7 @@ static void addFileList(StringRef path) {
return;
MemoryBufferRef mbref = *buffer;
for (StringRef path : args::getLines(mbref))
addFile(rerootPath(path), /*forceLoadArchive=*/false);
addFile(rerootPath(path), ForceLoad::Default);
}

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

switch (opt.getID()) {
case OPT_INPUT:
addFile(rerootPath(arg->getValue()), /*forceLoadArchive=*/false);
addFile(rerootPath(arg->getValue()), ForceLoad::Default);
break;
case OPT_needed_library:
if (auto *dylibFile = dyn_cast_or_null<DylibFile>(
addFile(rerootPath(arg->getValue()), false)))
addFile(rerootPath(arg->getValue()), ForceLoad::Default)))
dylibFile->forceNeeded = true;
break;
case OPT_reexport_library:
if (auto *dylibFile = dyn_cast_or_null<DylibFile>(addFile(
rerootPath(arg->getValue()), /*forceLoadArchive=*/false))) {
if (auto *dylibFile = dyn_cast_or_null<DylibFile>(
addFile(rerootPath(arg->getValue()), ForceLoad::Default))) {
config->hasReexports = true;
dylibFile->reexport = true;
}
break;
case OPT_weak_library:
if (auto *dylibFile = dyn_cast_or_null<DylibFile>(
addFile(rerootPath(arg->getValue()), /*forceLoadArchive=*/false)))
addFile(rerootPath(arg->getValue()), ForceLoad::Default)))
dylibFile->forceWeakImport = true;
break;
case OPT_filelist:
addFileList(arg->getValue());
break;
case OPT_force_load:
addFile(rerootPath(arg->getValue()), /*forceLoadArchive=*/true);
addFile(rerootPath(arg->getValue()), ForceLoad::Yes);
break;
case OPT_l:
case OPT_needed_l:
case OPT_reexport_l:
case OPT_weak_l:
addLibrary(arg->getValue(), opt.getID() == OPT_needed_l,
opt.getID() == OPT_weak_l, opt.getID() == OPT_reexport_l,
/*isExplicit=*/true, /*forceLoad=*/false);
/*isExplicit=*/true, ForceLoad::Default);
break;
case OPT_framework:
case OPT_needed_framework:
case OPT_reexport_framework:
case OPT_weak_framework:
addFramework(arg->getValue(), opt.getID() == OPT_needed_framework,
opt.getID() == OPT_weak_framework,
opt.getID() == OPT_reexport_framework, /*isExplicit=*/true);
opt.getID() == OPT_reexport_framework, /*isExplicit=*/true,
ForceLoad::Default);
break;
default:
break;
Expand Down Expand Up @@ -1175,7 +1182,7 @@ bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
if (const Arg *arg = args.getLastArg(OPT_bundle_loader)) {
if (config->outputType != MH_BUNDLE)
error("-bundle_loader can only be used with MachO bundle output");
addFile(arg->getValue(), /*forceLoadArchive=*/false, /*isExplicit=*/false,
addFile(arg->getValue(), ForceLoad::Default, /*isExplicit=*/false,
/*isBundleLoader=*/true);
}
if (const Arg *arg = args.getLastArg(OPT_umbrella)) {
Expand Down
33 changes: 27 additions & 6 deletions lld/test/MachO/lc-linker-option.ll
View file
Expand Up @@ -58,9 +58,24 @@
;; actual archive at Foo.framework/Versions/Current, but we skip that here so
;; that this test can run on Windows.
; RUN: llvm-ar rcs %t/Foo.framework/Foo %t/foo.o
; RUN: llc %t/objfile1.ll -o %t/objfile1.o -filetype=obj
; RUN: llc %t/load-framework-foo.ll -o %t/load-framework-foo.o -filetype=obj
; RUN: llc %t/main.ll -o %t/main.o -filetype=obj
; RUN: %lld %t/objfile1.o %t/main.o -o /dev/null -F%t
; RUN: %lld %t/load-framework-foo.o %t/main.o -o %t/main -F%t
; RUN: llvm-objdump --macho --syms %t/main | FileCheck %s --check-prefix=SYMS

;; Make sure -all_load and -ObjC have no effect on libraries loaded via
;; LC_LINKER_OPTION flags.
; RUN: llc %t/load-library-foo.ll -o %t/load-library-foo.o -filetype=obj
; RUN: llvm-ar rcs %t/libfoo.a %t/foo.o
; RUN: %lld -all_load -ObjC %t/load-framework-foo.o %t/load-library-foo.o \
; RUN: %t/main.o -o %t/main -F%t -L%t
; RUN: llvm-objdump --macho --syms %t/main | FileCheck %s --check-prefix=SYMS

;; Note that _OBJC_CLASS_$_TestClass is *not* included here.
; SYMS: SYMBOL TABLE:
; SYMS-NEXT: g F __TEXT,__text _main
; SYMS-NEXT: g F __TEXT,__text __mh_execute_header
; SYMS-EMPTY:

;--- framework.ll
target triple = "x86_64-apple-macosx10.15.0"
Expand Down Expand Up @@ -105,13 +120,20 @@ define void @main() {
ret void
}

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

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

;--- load-library-foo.ll
target triple = "x86_64-apple-macosx10.15.0"
target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"

!0 = !{!"-lfoo"}
!llvm.linker.options = !{!0}

;--- main.ll
target triple = "x86_64-apple-macosx10.15.0"
target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
Expand All @@ -127,6 +149,5 @@ define void @main() {
target triple = "x86_64-apple-macosx10.15.0"
target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"

define void @foo() {
ret void
}
%struct._class_t = type {}
@"OBJC_CLASS_$_TestClass" = global %struct._class_t {}, section "__DATA, __objc_data", align 8

0 comments on commit 6c2f26a

Please sign in to comment.