[lld] 47b63cd - [lld-macho] Save all thin archive members in repro tarball (#97169)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 18 07:26:36 PDT 2024
Author: Daniel Bertalan
Date: 2024-07-18T16:26:32+02:00
New Revision: 47b63cd508f993d9fab2acfbf0dcf86cdc8c5335
URL: https://github.com/llvm/llvm-project/commit/47b63cd508f993d9fab2acfbf0dcf86cdc8c5335
DIFF: https://github.com/llvm/llvm-project/commit/47b63cd508f993d9fab2acfbf0dcf86cdc8c5335.diff
LOG: [lld-macho] Save all thin archive members in repro tarball (#97169)
Previously, we only saved those members of thin archives into a repro
file that were actually used during linking. However, -ObjC handling
requires us to inspect all members, even those that don't end up being
loaded.
We weren't handling missing members correctly and crashed with an
"unhandled `Error`" failure in LLVM_ENABLE_ABI_BREAKING_CHECKS builds.
To fix this, we now eagerly load all object files and warn when
encountering missing members (in the instances where it wasn't a hard
error before). To avoid having to patch out the checks when dealing
with older repro files, the `--no-warn-thin-archive-missing-members`
flag is added as an escape hatch.
Added:
lld/test/MachO/reproduce-thin-archive-objc.s
Modified:
lld/MachO/Config.h
lld/MachO/Driver.cpp
lld/MachO/InputFiles.cpp
lld/MachO/Options.td
lld/test/MachO/reproduce-thin-archives.s
Removed:
################################################################################
diff --git a/lld/MachO/Config.h b/lld/MachO/Config.h
index 5c354e0fe8821..e79812b16ec12 100644
--- a/lld/MachO/Config.h
+++ b/lld/MachO/Config.h
@@ -212,6 +212,7 @@ struct Configuration {
bool csProfileGenerate = false;
llvm::StringRef csProfilePath;
bool pgoWarnMismatch;
+ bool warnThinArchiveMissingMembers;
bool callGraphProfileSort = false;
llvm::StringRef printSymbolOrder;
diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index ffb3feae25ca4..dc9d635b48ec4 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -270,6 +270,20 @@ struct ArchiveFileInfo {
static DenseMap<StringRef, ArchiveFileInfo> loadedArchives;
+static void saveThinArchiveToRepro(ArchiveFile const *file) {
+ assert(tar && file->getArchive().isThin());
+
+ Error e = Error::success();
+ for (const object::Archive::Child &c : file->getArchive().children(e)) {
+ MemoryBufferRef mb = CHECK(c.getMemoryBufferRef(),
+ toString(file) + ": failed to get buffer");
+ tar->append(relativeToRoot(CHECK(c.getFullName(), file)), mb.getBuffer());
+ }
+ if (e)
+ error(toString(file) +
+ ": Archive::children failed: " + toString(std::move(e)));
+}
+
static InputFile *addFile(StringRef path, LoadType loadType,
bool isLazy = false, bool isExplicit = true,
bool isBundleLoader = false,
@@ -301,6 +315,9 @@ static InputFile *addFile(StringRef path, LoadType loadType,
if (!archive->isEmpty() && !archive->hasSymbolTable())
error(path + ": archive has no index; run ranlib to add one");
file = make<ArchiveFile>(std::move(archive), isForceHidden);
+
+ if (tar && file->getArchive().isThin())
+ saveThinArchiveToRepro(file);
} else {
file = entry->second.file;
// Command-line loads take precedence. If file is previously loaded via
@@ -330,9 +347,13 @@ static InputFile *addFile(StringRef path, LoadType loadType,
reason = "-all_load";
break;
}
- if (Error e = file->fetch(c, reason))
- error(toString(file) + ": " + reason +
- " failed to load archive member: " + toString(std::move(e)));
+ if (Error e = file->fetch(c, reason)) {
+ if (config->warnThinArchiveMissingMembers)
+ warn(toString(file) + ": " + reason +
+ " failed to load archive member: " + toString(std::move(e)));
+ else
+ llvm::consumeError(std::move(e));
+ }
}
if (e)
error(toString(file) +
@@ -349,7 +370,18 @@ static InputFile *addFile(StringRef path, LoadType loadType,
Error e = Error::success();
for (const object::Archive::Child &c : file->getArchive().children(e)) {
Expected<MemoryBufferRef> mb = c.getMemoryBufferRef();
- if (!mb || !hasObjCSection(*mb))
+ if (!mb) {
+ // We used to create broken repro tarballs that only included those
+ // object files from thin archives that ended up being used.
+ if (config->warnThinArchiveMissingMembers)
+ warn(toString(file) + ": -ObjC failed to open archive member: " +
+ toString(mb.takeError()));
+ else
+ llvm::consumeError(mb.takeError());
+ continue;
+ }
+
+ if (!hasObjCSection(*mb))
continue;
if (Error e = file->fetch(c, "-ObjC"))
error(toString(file) + ": -ObjC failed to load archive member: " +
@@ -1699,6 +1731,9 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
config->csProfilePath = args.getLastArgValue(OPT_cs_profile_path);
config->pgoWarnMismatch =
args.hasFlag(OPT_pgo_warn_mismatch, OPT_no_pgo_warn_mismatch, true);
+ config->warnThinArchiveMissingMembers =
+ args.hasFlag(OPT_warn_thin_archive_missing_members,
+ OPT_no_warn_thin_archive_missing_members, true);
config->generateUuid = !args.hasArg(OPT_no_uuid);
for (const Arg *arg : args.filtered(OPT_alias)) {
diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index b40a812f30bd3..3086c9cc4729d 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -2200,10 +2200,6 @@ Error ArchiveFile::fetch(const object::Archive::Child &c, StringRef reason) {
if (!mb)
return mb.takeError();
- // Thin archives refer to .o files, so --reproduce needs the .o files too.
- if (tar && c.getParent()->isThin())
- tar->append(relativeToRoot(CHECK(c.getFullName(), this)), mb->getBuffer());
-
Expected<TimePoint<std::chrono::seconds>> modTime = c.getLastModified();
if (!modTime)
return modTime.takeError();
diff --git a/lld/MachO/Options.td b/lld/MachO/Options.td
index dc2212399222f..bbd8bf70c3a0c 100644
--- a/lld/MachO/Options.td
+++ b/lld/MachO/Options.td
@@ -153,6 +153,9 @@ def cs_profile_path: Joined<["--"], "cs-profile-path=">,
defm pgo_warn_mismatch: BB<"pgo-warn-mismatch",
"turn on warnings about profile cfg mismatch (default)",
"turn off warnings about profile cfg mismatch">, Group<grp_lld>;
+defm warn_thin_archive_missing_members : BB<"warn-thin-archive-missing-members",
+ "Warn on missing object files referenced by thin archives (default)",
+ "Do not warn on missing object files referenced by thin archives">, 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/reproduce-thin-archive-objc.s b/lld/test/MachO/reproduce-thin-archive-objc.s
new file mode 100644
index 0000000000000..c5fe42f130526
--- /dev/null
+++ b/lld/test/MachO/reproduce-thin-archive-objc.s
@@ -0,0 +1,25 @@
+# REQUIRES: x86
+
+## For a long time, LLD only included those members from thin archives that were actually used
+## during linking. However, we need to iterate over all members for -ObjC, check that we don't
+## crash when we encounter a missing member.
+
+# RUN: rm -rf %t; mkdir %t
+# RUN: sed s/SYM/_main/ %s | llvm-mc -filetype=obj -triple=x86_64-apple-macos -o %t/main.o
+# RUN: sed s/SYM/_unused/ %s | llvm-mc -filetype=obj -triple=x86_64-apple-macos -o %t/unused.o
+
+# RUN: cd %t; llvm-ar rcsT unused.a unused.o; rm unused.o
+## FIXME: Absolute paths don't end up relativized in the repro file.
+
+# RUN: %no-fatal-warnings-lld %t/main.o %t/unused.a -ObjC -o /dev/null 2>&1 \
+# RUN: | FileCheck %s --check-prefix=WARN
+
+# RUN: %lld %t/main.o %t/unused.a -ObjC --no-warn-thin-archive-missing-members -o /dev/null \
+# RUN: | FileCheck %s --implicit-check-not 'warning' --allow-empty
+
+# WARN: ld64.lld: warning: {{.*}}unused.a: -ObjC failed to open archive member: 'unused.o'
+
+.text
+.globl SYM
+SYM:
+ ret
diff --git a/lld/test/MachO/reproduce-thin-archives.s b/lld/test/MachO/reproduce-thin-archives.s
index 9dee3f400e06a..33eeaede7aa41 100644
--- a/lld/test/MachO/reproduce-thin-archives.s
+++ b/lld/test/MachO/reproduce-thin-archives.s
@@ -1,10 +1,11 @@
# REQUIRES: x86
-# RUN: rm -rf %t.dir
-# RUN: mkdir -p %t.dir
-# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos %s -o %t.dir/foo.o
+# RUN: rm -rf %t.dir; split-file %s %t.dir
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos %t.dir/foo.s -o %t.dir/foo.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macos %t.dir/unused.s -o %t.dir/unused.o
# RUN: cd %t.dir
-# RUN: llvm-ar rcsT foo.a foo.o
+# RUN: llvm-ar rcsT foo.a foo.o unused.o
# RUN: %lld foo.a -o /dev/null --reproduce repro.tar
# RUN: tar tf repro.tar | FileCheck -DPATH='repro/%:t.dir' %s
@@ -12,9 +13,19 @@
# RUN: %lld -all_load foo.a -o /dev/null --reproduce repro2.tar
# RUN: tar tf repro2.tar | FileCheck -DPATH='repro2/%:t.dir' %s
+# RUN: %lld -ObjC foo.a -o /dev/null --reproduce repro3.tar
+# RUN: tar tf repro3.tar | FileCheck -DPATH='repro3/%:t.dir' %s
+
# CHECK-DAG: [[PATH]]/foo.a
# CHECK-DAG: [[PATH]]/foo.o
+# CHECK-DAG: [[PATH]]/unused.o
+#--- foo.s
.globl _main
_main:
nop
+
+#--- unused.s
+.globl _unused
+_unused:
+ nop
More information about the llvm-commits
mailing list