[PATCH] D104353: [lld-macho] Avoid force-loading the same archive twice
Jez Ng via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 16 13:42:54 PDT 2021
int3 updated this revision to Diff 352544.
int3 added a comment.
test symlinks and hardlinks
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104353/new/
https://reviews.llvm.org/D104353
Files:
lld/MachO/Driver.cpp
lld/test/MachO/force-load.s
Index: lld/test/MachO/force-load.s
===================================================================
--- lld/test/MachO/force-load.s
+++ lld/test/MachO/force-load.s
@@ -5,19 +5,40 @@
# 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
+
+## XXX not sure this is worth committing, but it demonstrates that we treat
+## symlinks and hardlinks as distinct inputs too.
+# RUN: ln -s %t/foo.a %t/bar.a
+# RUN: not %lld -lSystem %t/foo.o -force_load %t/foo.a -force_load %t/bar.a \
+# RUN: %t/test.o -o /dev/null 2>&1
+
+# RUN: ln %t/foo.a %t/baz.a
+# RUN: not %lld -lSystem %t/foo.o -force_load %t/foo.a -force_load %t/baz.a \
+# RUN: %t/test.o -o /dev/null 2>&1
+
+# DUP: error: duplicate symbol: _bar
+
#--- archive-foo.s
.section __TEXT,archive
-.globl _foo
+.globl _foo, _bar
.weak_definition _foo
_foo:
+_bar:
#--- foo.s
.section __TEXT,obj
Index: lld/MachO/Driver.cpp
===================================================================
--- lld/MachO/Driver.cpp
+++ lld/MachO/Driver.cpp
@@ -235,6 +235,8 @@
return v;
}
+static DenseMap<StringRef, ArchiveFile *> loadedArchives;
+
static InputFile *addFile(StringRef path, bool forceLoadArchive,
bool isExplicit = true,
bool isBundleLoader = false) {
@@ -247,6 +249,13 @@
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.
+ 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 +295,7 @@
}
}
- newFile = make<ArchiveFile>(std::move(file));
+ newFile = cachedFile = make<ArchiveFile>(std::move(file));
break;
}
case file_magic::macho_object:
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D104353.352544.patch
Type: text/x-patch
Size: 3185 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210616/6922e4b7/attachment.bin>
More information about the llvm-commits
mailing list