[lld] d6cd8d6 - [lld-macho] Use ld64's LC_LINKER_OPTIONS behavior by default
Keith Smiley via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 22 16:04:27 PST 2022
Author: Keith Smiley
Date: 2022-12-22T16:02:46-08:00
New Revision: d6cd8d6b1987be6abc2c2e854358f72c00d910a5
URL: https://github.com/llvm/llvm-project/commit/d6cd8d6b1987be6abc2c2e854358f72c00d910a5
DIFF: https://github.com/llvm/llvm-project/commit/d6cd8d6b1987be6abc2c2e854358f72c00d910a5.diff
LOG: [lld-macho] Use ld64's LC_LINKER_OPTIONS behavior by default
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 https://github.com/llvm/llvm-project/issues/59627
Differential Revision: https://reviews.llvm.org/D140491
Added:
Modified:
lld/MachO/Config.h
lld/MachO/Driver.cpp
lld/MachO/Options.td
lld/test/MachO/lc-linker-option.ll
Removed:
################################################################################
diff --git a/lld/MachO/Config.h b/lld/MachO/Config.h
index bbc8006f2992..6586824b87e2 100644
--- a/lld/MachO/Config.h
+++ b/lld/MachO/Config.h
@@ -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;
diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index 1e4e1bcee82e..3449e68352b7 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -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,
@@ -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;
@@ -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);
}
@@ -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");
}
@@ -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();
@@ -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(
@@ -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;
}
diff --git a/lld/MachO/Options.td b/lld/MachO/Options.td
index 458731003eea..b0ccf13630a4 100644
--- a/lld/MachO/Options.td
+++ b/lld/MachO/Options.td
@@ -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
diff --git a/lld/test/MachO/lc-linker-option.ll b/lld/test/MachO/lc-linker-option.ll
index 51234aec9bb2..1a4aff138e9e 100644
--- a/lld/test/MachO/lc-linker-option.ll
+++ b/lld/test/MachO/lc-linker-option.ll
@@ -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
@@ -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"
@@ -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"
More information about the llvm-commits
mailing list