Skip to content

Commit

Permalink
[lld-macho] Use ld64's LC_LINKER_OPTIONS behavior by default
Browse files Browse the repository at this point in the history 
By default ld64 ignores invalid LC_LINKER_OPTIONS unless the link fails,
in which case it prints a warning. Originally lld chose to be strict
about these, but it has uncovered that many of these exist in open
source projects today, since before developers never would have noticed
this issue. In order to make adoption of lld easier, this mirrors ld64's
behavior, while also adding a `--strict-auto-link-options` flag if
projects want to audit their libraries for these invalid options.

More discussion on https://reviews.llvm.org/D140225
Fixes #59627

Differential Revision: https://reviews.llvm.org/D140491
  • Loading branch information
@keith
keith committed Dec 23, 2022
1 parent 2e5989e commit d6cd8d6
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 4 deletions.
1 change: 1 addition & 0 deletions lld/MachO/Config.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ struct Configuration {
// exist. This allows users to ignore the specific invalid options in the case
// they can't easily fix them.
llvm::StringSet<> ignoreAutoLinkOptions;
bool strictAutoLink = false;
PlatformInfo platformInfo;
std::optional<PlatformInfo> secondaryPlatformInfo;
NamespaceKind namespaceKind = NamespaceKind::twolevel;
Expand Down
31 changes: 27 additions & 4 deletions lld/MachO/Driver.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -406,9 +406,10 @@ static InputFile *addFile(StringRef path, LoadType loadType,
return newFile;
}

static std::vector<StringRef> missingAutolinkWarnings;
static void addLibrary(StringRef name, bool isNeeded, bool isWeak,
bool isReexport, bool isHidden, bool isExplicit,
LoadType loadType) {
LoadType loadType, InputFile *originFile = nullptr) {
if (std::optional<StringRef> path = findLibrary(name)) {
if (auto *dylibFile = dyn_cast_or_null<DylibFile>(
addFile(*path, loadType, /*isLazy=*/false, isExplicit,
Expand All @@ -424,12 +425,20 @@ static void addLibrary(StringRef name, bool isNeeded, bool isWeak,
}
return;
}
if (loadType == LoadType::LCLinkerOption) {
assert(originFile);
missingAutolinkWarnings.push_back(
saver().save(toString(originFile) +
": auto-linked library not found for -l" + name));
return;
}
error("library not found for -l" + name);
}

static DenseSet<StringRef> loadedObjectFrameworks;
static void addFramework(StringRef name, bool isNeeded, bool isWeak,
bool isReexport, bool isExplicit, LoadType loadType) {
bool isReexport, bool isExplicit, LoadType loadType,
InputFile *originFile = nullptr) {
if (std::optional<StringRef> path = findFramework(name)) {
if (loadedObjectFrameworks.contains(*path))
return;
Expand All @@ -456,6 +465,13 @@ static void addFramework(StringRef name, bool isNeeded, bool isWeak,
}
return;
}
if (loadType == LoadType::LCLinkerOption) {
assert(originFile);
missingAutolinkWarnings.push_back(saver().save(
toString(originFile) +
": auto-linked framework not found for -framework " + name));
return;
}
error("framework not found for -framework " + name);
}

Expand All @@ -482,14 +498,14 @@ void macho::parseLCLinkerOption(InputFile *f, unsigned argc, StringRef data) {
return;
addLibrary(arg, /*isNeeded=*/false, /*isWeak=*/false,
/*isReexport=*/false, /*isHidden=*/false, /*isExplicit=*/false,
LoadType::LCLinkerOption);
LoadType::LCLinkerOption, f);
} else if (arg == "-framework") {
StringRef name = argv[++i];
if (config->ignoreAutoLinkOptions.contains(name))
return;
addFramework(name, /*isNeeded=*/false, /*isWeak=*/false,
/*isReexport=*/false, /*isExplicit=*/false,
LoadType::LCLinkerOption);
LoadType::LCLinkerOption, f);
} else {
error(arg + " is not allowed in LC_LINKER_OPTION");
}
Expand Down Expand Up @@ -1347,6 +1363,7 @@ bool macho::link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
inputSections.clear();
loadedArchives.clear();
loadedObjectFrameworks.clear();
missingAutolinkWarnings.clear();
syntheticSections.clear();
thunkMap.clear();

Expand Down Expand Up @@ -1566,6 +1583,7 @@ bool macho::link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
config->ignoreAutoLink = args.hasArg(OPT_ignore_auto_link);
for (const Arg *arg : args.filtered(OPT_ignore_auto_link_option))
config->ignoreAutoLinkOptions.insert(arg->getValue());
config->strictAutoLink = args.hasArg(OPT_strict_auto_link);

for (const Arg *arg : args.filtered(OPT_alias)) {
config->aliasedSymbols.push_back(
Expand Down Expand Up @@ -1880,5 +1898,10 @@ bool macho::link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,

timeTraceProfilerCleanup();
}

if (errorCount() != 0 || config->strictAutoLink)
for (const auto &warning : missingAutolinkWarnings)
warn(warning);

return errorCount() == 0;
}
3 changes: 3 additions & 0 deletions lld/MachO/Options.td
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ def ignore_auto_link_option_eq : Joined<["--"], "ignore-auto-link-option=">,
Alias<!cast<Separate>(ignore_auto_link_option)>,
HelpText<"Ignore a single auto-linked library or framework. Useful to ignore invalid options that ld64 ignores">,
Group<grp_lld>;
def strict_auto_link : Flag<["--"], "strict-auto-link">,
HelpText<"Always warn for missing frameworks or libraries if they are loaded via LC_LINKER_OPTIONS">,
Group<grp_lld>;

// This is a complete Options.td compiled from Apple's ld(1) manpage
// dated 2018-03-07 and cross checked with ld64 source code in repo
Expand Down
40 changes: 40 additions & 0 deletions lld/test/MachO/lc-linker-option.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@
;; that this test can run on Windows.
; RUN: llvm-ar rcs %t/Foo.framework/Foo %t/foo.o
; RUN: llc %t/load-framework-foo.ll -o %t/load-framework-foo.o -filetype=obj
; RUN: llc %t/load-framework-undefined-symbol.ll -o %t/load-framework-undefined-symbol.o -filetype=obj
; RUN: llc %t/load-missing.ll -o %t/load-missing.o -filetype=obj
; RUN: llc %t/main.ll -o %t/main.o -filetype=obj
; 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
Expand Down Expand Up @@ -134,6 +136,26 @@
; RUN: %lld %t/main -F %t -framework Foo -framework Foo -o /dev/null
; RUN: %lld -F %t -framework Foo -framework Foo %t/main -o /dev/null

;; Checks that "framework not found" errors from LC_LINKER_OPTIONS are not
;; emitted unless the link fails or --strict-auto-link is passed.
; RUN: %lld -ObjC %t/load-framework-foo.o %t/main.o -o %t/main-no-foo.out
; RUN: llvm-objdump --macho --syms %t/main-no-foo.out | FileCheck %s --check-prefix=SYMS-NO-FOO
; RUN: not %lld --strict-auto-link -ObjC %t/load-missing.o %t/main.o -o %t/main-no-foo.out 2>&1 \
; RUN: | FileCheck %s --check-prefix=MISSING-AUTO-LINK
; RUN: %no-fatal-warnings-lld --strict-auto-link -ObjC %t/load-missing.o %t/main.o -o %t/main-no-foo.out 2>&1 \
; RUN: | FileCheck %s --check-prefix=MISSING-AUTO-LINK
; RUN: not %lld -ObjC %t/load-framework-undefined-symbol.o %t/load-missing.o %t/main.o -o %t/main-no-foo.out 2>&1 \
; RUN: | FileCheck %s --check-prefixes=UNDEFINED-SYMBOL,MISSING-AUTO-LINK

;; Verify that nothing from the framework is included.
; SYMS-NO-FOO: SYMBOL TABLE:
; SYMS-NO-FOO-NEXT: g F __TEXT,__text _main
; SYMS-NO-FOO-NOT: g O __DATA,__objc_data _OBJC_CLASS_$_TestClass

; UNDEFINED-SYMBOL: undefined symbol: __SomeUndefinedSymbol
; MISSING-AUTO-LINK: {{.+}}load-missing.o: auto-linked framework not found for -framework Foo
; MISSING-AUTO-LINK: {{.+}}load-missing.o: auto-linked library not found for -lBar

;--- framework.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 Down Expand Up @@ -184,6 +206,24 @@ target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16
!0 = !{!"-framework", !"Foo"}
!llvm.linker.options = !{!0}

;--- load-missing.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"}
!1 = !{!"-lBar"}
!llvm.linker.options = !{!0, !1}

;--- load-framework-undefined-symbol.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"

declare void @_SomeUndefinedSymbol(...)
define void @foo() {
call void @_SomeUndefinedSymbol()
ret void
}

;--- load-framework-twice.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 Down

0 comments on commit d6cd8d6

Please sign in to comment.