[lld] 4507f64 - [re-land][lld-macho] Avoid force-loading the same archive twice
Jez Ng via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 18 19:45:08 PDT 2021
Author: Jez Ng
Date: 2021-06-18T22:43:50-04:00
New Revision: 4507f64165fd0c95f08b66d61db39549b22c50d3
URL: https://github.com/llvm/llvm-project/commit/4507f64165fd0c95f08b66d61db39549b22c50d3
DIFF: https://github.com/llvm/llvm-project/commit/4507f64165fd0c95f08b66d61db39549b22c50d3.diff
LOG: [re-land][lld-macho] Avoid force-loading the same archive twice
This reverts commit c9b241efd68c5a0f1f67e9250960ade454f3bc11, which was
a backout diff to fix the buildbots.
The real culprit of the crash is
https://github.com/llvm/llvm-project/commit/1d31fb8d122b1117cf20a9edc09812db8472e930,
which is being reverted.
Differential Revision: https://reviews.llvm.org/D104353
Added:
Modified:
lld/MachO/Driver.cpp
lld/test/MachO/archive.s
lld/test/MachO/force-load.s
Removed:
################################################################################
diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index 52771c2a6fe1..074695f35cf7 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -235,6 +235,8 @@ static std::vector<ArchiveMember> getArchiveMembers(MemoryBufferRef mb) {
return v;
}
+static DenseMap<StringRef, ArchiveFile *> loadedArchives;
+
static InputFile *addFile(StringRef path, bool forceLoadArchive,
bool isExplicit = true,
bool isBundleLoader = false) {
@@ -247,6 +249,16 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive,
file_magic magic = identify_magic(mbref.getBuffer());
switch (magic) {
case file_magic::archive: {
+ // Avoid loading archives twice. If the archives are being force-loaded,
+ // loading them twice would create duplicate symbol errors. In the
+ // non-force-loading case, this is just a minor performance optimization.
+ // We don't take a reference to cachedFile here because the
+ // loadArchiveMember() call below may recursively call addFile() and
+ // invalidate this reference.
+ ArchiveFile *cachedFile = loadedArchives[path];
+ if (cachedFile)
+ return cachedFile;
+
std::unique_ptr<object::Archive> file = CHECK(
object::Archive::create(mbref), path + ": failed to parse archive");
@@ -286,7 +298,7 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive,
}
}
- newFile = make<ArchiveFile>(std::move(file));
+ newFile = loadedArchives[path] = make<ArchiveFile>(std::move(file));
break;
}
case file_magic::macho_object:
diff --git a/lld/test/MachO/archive.s b/lld/test/MachO/archive.s
index 2ac2d302b88d..3a946c0ab935 100644
--- a/lld/test/MachO/archive.s
+++ b/lld/test/MachO/archive.s
@@ -32,6 +32,11 @@
# ALL-LOAD: T _main
# ALL-LOAD: T _unused
+## Multiple archives defining the same symbols aren't an issue, due to lazy
+## loading
+# RUN: cp %t/test.a %t/test2.a
+# RUN: %lld %t/test.a %t/test2.a %t/main.o -o /dev/null
+
#--- 2.s
.globl _boo
_boo:
diff --git a/lld/test/MachO/force-load.s b/lld/test/MachO/force-load.s
index 1cedae3173bc..13d65f9f4a15 100644
--- a/lld/test/MachO/force-load.s
+++ b/lld/test/MachO/force-load.s
@@ -1,23 +1,48 @@
# REQUIRES: x86
# RUN: rm -rf %t; split-file %s %t
# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/archive-foo.s -o %t/archive-foo.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/archive-baz.s -o %t/archive-baz.o
# RUN: llvm-ar rcs %t/foo.a %t/archive-foo.o
+# RUN: llvm-ar rcs %t/baz.a %t/archive-baz.o
# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/foo.s -o %t/foo.o
# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/test.s -o %t/test.o
-# RUN: %lld -force_load %t/foo.a %t/foo.o %t/test.o -o %t/test-force-load-first
+# RUN: %lld -lSystem -force_load %t/foo.a %t/foo.o %t/test.o -o %t/test-force-load-first
# FORCE-LOAD-FIRST: __TEXT,archive _foo
# RUN: llvm-objdump --syms %t/test-force-load-first | FileCheck %s --check-prefix=FORCE-LOAD-FIRST
-# RUN: %lld %t/foo.o -force_load %t/foo.a %t/test.o -o %t/test-force-load-second
+# RUN: %lld %t/foo.o -lSystem -force_load %t/foo.a %t/test.o -o %t/test-force-load-second
# RUN: llvm-objdump --syms %t/test-force-load-second | FileCheck %s --check-prefix=FORCE-LOAD-SECOND
# FORCE-LOAD-SECOND: __TEXT,obj _foo
+## Force-loading the same path twice is fine
+# RUN: %lld -lSystem %t/foo.o -force_load %t/foo.a -force_load %t/foo.a %t/test.o -o /dev/null
+
+## Note that we do not call realpath() before dedup'ing the force-load
+## arguments, so this is an error.
+# RUN: cd %t; not %lld -lSystem %t/foo.o -force_load %t/foo.a -force_load foo.a \
+# RUN: %t/test.o -o /dev/null 2>&1 | FileCheck %s --check-prefix=DUP
+
+# DUP: error: duplicate symbol: _bar
+
+## Force-loading two
diff erent paths w/o conflicting symbols is fine
+# RUN: %lld -lSystem -force_load %t/foo.a -force_load %t/baz.a %t/test.o -o %t/test-two-force-loads
+# RUN: llvm-objdump --syms %t/test-two-force-loads | FileCheck %s --check-prefix=TWICE
+# TWICE-DAG: __TEXT,archive _foo
+# TWICE-DAG: __TEXT,archive _bar
+# TWICE-DAG: __TEXT,archive _baz
+
#--- archive-foo.s
.section __TEXT,archive
-.globl _foo
+.globl _foo, _bar
.weak_definition _foo
_foo:
+_bar:
+
+#--- archive-baz.s
+.section __TEXT,archive
+.globl _baz
+_baz:
#--- foo.s
.section __TEXT,obj
More information about the llvm-commits
mailing list