[llvm-branch-commits] [lld] 544148a - [lld-macho] -weak_{library, framework} should always take priority
Jez Ng via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Tue Dec 15 13:05:11 PST 2020
Author: Jez Ng
Date: 2020-12-15T15:58:26-05:00
New Revision: 544148ae702ac3e94aa41b656e9f6d11b382f9f6
URL: https://github.com/llvm/llvm-project/commit/544148ae702ac3e94aa41b656e9f6d11b382f9f6
DIFF: https://github.com/llvm/llvm-project/commit/544148ae702ac3e94aa41b656e9f6d11b382f9f6.diff
LOG: [lld-macho] -weak_{library,framework} should always take priority
We were not setting forceWeakImport for file paths given by
`-weak_library` if we had already loaded the file. This diff fixes that
by having `loadDylib` return a cached DylibFile instance even if we have
already loaded that file.
We still avoid emitting multiple LC_LOAD_DYLIBs, but we achieve this by
making inputFiles a SetVector instead of relying on the `loadedDylibs`
cache.
Reviewed By: #lld-macho, smeenai
Differential Revision: https://reviews.llvm.org/D93255
Added:
Modified:
lld/MachO/Driver.cpp
lld/MachO/DriverUtils.cpp
lld/MachO/InputFiles.cpp
lld/MachO/InputFiles.h
lld/test/MachO/invalid/duplicate-symbol.s
lld/test/MachO/weak-import.s
Removed:
################################################################################
diff --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index c456d2fb1fd5..96972cb17e6f 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -274,8 +274,7 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive) {
if (config->allLoad || forceLoadArchive) {
if (Optional<MemoryBufferRef> buffer = readFile(path)) {
for (const ArchiveMember &member : getArchiveMembers(*buffer)) {
- inputFiles.push_back(
- make<ObjFile>(member.mbref, member.modTime, path));
+ inputFiles.insert(make<ObjFile>(member.mbref, member.modTime, path));
printArchiveMemberLoad(
(forceLoadArchive ? "-force_load" : "-all_load"),
inputFiles.back());
@@ -293,7 +292,7 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive) {
if (Optional<MemoryBufferRef> buffer = readFile(path)) {
for (const ArchiveMember &member : getArchiveMembers(*buffer)) {
if (hasObjCSection(member.mbref)) {
- inputFiles.push_back(
+ inputFiles.insert(
make<ObjFile>(member.mbref, member.modTime, path));
printArchiveMemberLoad("-ObjC", inputFiles.back());
}
@@ -325,7 +324,7 @@ static InputFile *addFile(StringRef path, bool forceLoadArchive) {
// print the .a name here.
if (config->printEachFile && magic != file_magic::archive)
lld::outs() << toString(newFile) << '\n';
- inputFiles.push_back(newFile);
+ inputFiles.insert(newFile);
}
return newFile;
}
@@ -521,7 +520,7 @@ static void compileBitcodeFiles() {
lto->add(*bitcodeFile);
for (ObjFile *file : lto->compile())
- inputFiles.push_back(file);
+ inputFiles.insert(file);
}
// Replaces common symbols with defined symbols residing in __common sections.
@@ -873,7 +872,7 @@ bool macho::link(llvm::ArrayRef<const char *> argsArr, bool canExitEarly,
StringRef fileName = arg->getValue(2);
Optional<MemoryBufferRef> buffer = readFile(fileName);
if (buffer)
- inputFiles.push_back(make<OpaqueFile>(*buffer, segName, sectName));
+ inputFiles.insert(make<OpaqueFile>(*buffer, segName, sectName));
}
// Initialize InputSections.
diff --git a/lld/MachO/DriverUtils.cpp b/lld/MachO/DriverUtils.cpp
index 05677a9df78d..5040f634e181 100644
--- a/lld/MachO/DriverUtils.cpp
+++ b/lld/MachO/DriverUtils.cpp
@@ -15,7 +15,7 @@
#include "lld/Common/Memory.h"
#include "lld/Common/Reproduce.h"
#include "llvm/ADT/CachedHashString.h"
-#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/DenseMap.h"
#include "llvm/Option/Arg.h"
#include "llvm/Option/ArgList.h"
#include "llvm/Option/Option.h"
@@ -168,33 +168,32 @@ Optional<std::string> macho::resolveDylibPath(StringRef path) {
return {};
}
-static Optional<DylibFile *> makeDylibFromTapi(MemoryBufferRef mbref,
- DylibFile *umbrella) {
- Expected<std::unique_ptr<InterfaceFile>> result = TextAPIReader::get(mbref);
- if (!result) {
- error("could not load TAPI file at " + mbref.getBufferIdentifier() + ": " +
- toString(result.takeError()));
- return {};
- }
- return make<DylibFile>(**result, umbrella);
-}
-
-static DenseSet<CachedHashStringRef> loadedDylibs;
+// It's not uncommon to have multiple attempts to load a single dylib,
+// especially if it's a commonly re-exported core library.
+static DenseMap<CachedHashStringRef, DylibFile *> loadedDylibs;
Optional<DylibFile *> macho::loadDylib(MemoryBufferRef mbref,
DylibFile *umbrella) {
StringRef path = mbref.getBufferIdentifier();
- if (loadedDylibs.contains(CachedHashStringRef(path)))
- return {};
- loadedDylibs.insert(CachedHashStringRef(path));
+ DylibFile *&file = loadedDylibs[CachedHashStringRef(path)];
+ if (file)
+ return file;
file_magic magic = identify_magic(mbref.getBuffer());
- if (magic == file_magic::tapi_file)
- return makeDylibFromTapi(mbref, umbrella);
-
- assert(magic == file_magic::macho_dynamically_linked_shared_lib ||
- magic == file_magic::macho_dynamically_linked_shared_lib_stub);
- return make<DylibFile>(mbref, umbrella);
+ if (magic == file_magic::tapi_file) {
+ Expected<std::unique_ptr<InterfaceFile>> result = TextAPIReader::get(mbref);
+ if (!result) {
+ error("could not load TAPI file at " + mbref.getBufferIdentifier() +
+ ": " + toString(result.takeError()));
+ return {};
+ }
+ file = make<DylibFile>(**result, umbrella);
+ } else {
+ assert(magic == file_magic::macho_dynamically_linked_shared_lib ||
+ magic == file_magic::macho_dynamically_linked_shared_lib_stub);
+ file = make<DylibFile>(mbref, umbrella);
+ }
+ return file;
}
uint32_t macho::getModTime(StringRef path) {
diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index 92fc7d10ebb0..4bf568284c54 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -85,7 +85,7 @@ std::string lld::toString(const InputFile *f) {
.str();
}
-std::vector<InputFile *> macho::inputFiles;
+SetVector<InputFile *> macho::inputFiles;
std::unique_ptr<TarWriter> macho::tar;
int InputFile::idCount = 0;
@@ -516,7 +516,7 @@ static bool isImplicitlyLinked(StringRef path) {
void loadReexport(StringRef path, DylibFile *umbrella) {
Optional<DylibFile *> reexport = loadReexportHelper(path, umbrella);
if (reexport && isImplicitlyLinked(path))
- inputFiles.push_back(*reexport);
+ inputFiles.insert(*reexport);
}
DylibFile::DylibFile(MemoryBufferRef mb, DylibFile *umbrella)
@@ -670,7 +670,7 @@ void ArchiveFile::fetch(const object::Archive::Symbol &sym) {
" has unhandled file type");
return;
}
- inputFiles.push_back(file);
+ inputFiles.insert(file);
// ld64 doesn't demangle sym here even with -demangle. Match that, so
// intentionally no call to toMachOString() here.
diff --git a/lld/MachO/InputFiles.h b/lld/MachO/InputFiles.h
index e5a1566dda20..e52905e75d56 100644
--- a/lld/MachO/InputFiles.h
+++ b/lld/MachO/InputFiles.h
@@ -14,6 +14,7 @@
#include "lld/Common/LLVM.h"
#include "lld/Common/Memory.h"
#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/SetVector.h"
#include "llvm/BinaryFormat/MachO.h"
#include "llvm/DebugInfo/DWARF/DWARFUnit.h"
#include "llvm/Object/Archive.h"
@@ -158,7 +159,7 @@ class BitcodeFile : public InputFile {
std::unique_ptr<llvm::lto::InputFile> obj;
};
-extern std::vector<InputFile *> inputFiles;
+extern llvm::SetVector<InputFile *> inputFiles;
llvm::Optional<MemoryBufferRef> readFile(StringRef path);
diff --git a/lld/test/MachO/invalid/duplicate-symbol.s b/lld/test/MachO/invalid/duplicate-symbol.s
index ce662b6141d0..392630a7f075 100644
--- a/lld/test/MachO/invalid/duplicate-symbol.s
+++ b/lld/test/MachO/invalid/duplicate-symbol.s
@@ -2,6 +2,7 @@
# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t-dup.o
# RUN: not %lld -o /dev/null %t-dup.o %t.o 2>&1 | FileCheck %s
+# RUN: not %lld -o /dev/null %t.o %t.o 2>&1 | FileCheck %s
# CHECK: error: duplicate symbol: _main
diff --git a/lld/test/MachO/weak-import.s b/lld/test/MachO/weak-import.s
index 81e0a9e7e3ec..51736edd5499 100644
--- a/lld/test/MachO/weak-import.s
+++ b/lld/test/MachO/weak-import.s
@@ -6,6 +6,10 @@
# RUN: %lld -weak-lSystem %t/test.o -weak_framework CoreFoundation -weak_library %t/libfoo.dylib -o %t/test
# RUN: llvm-objdump --macho --all-headers %t/test | FileCheck %s -DDIR=%t
+# RUN: %lld -weak-lSystem %t/test.o \
+# RUN: -framework CoreFoundation -weak_framework CoreFoundation -framework CoreFoundation \
+# RUN: %t/libfoo.dylib -weak_library %t/libfoo.dylib %t/libfoo.dylib -o %t/test
+# RUN: llvm-objdump --macho --all-headers %t/test | FileCheck %s -DDIR=%t
# CHECK: cmd LC_LOAD_WEAK_DYLIB
# CHECK-NEXT: cmdsize
More information about the llvm-branch-commits
mailing list