[llvm] 9a2e2de - [lld-macho] Change loadReexport to handle the case where a TAPI re-exports to reference documents nested within other TBD.
Vy Nguyen via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 2 09:15:03 PST 2021
Author: Vy Nguyen
Date: 2021-03-02T12:14:31-05:00
New Revision: 9a2e2de15f108f943ae50e6183719a4af81104e8
URL: https://github.com/llvm/llvm-project/commit/9a2e2de15f108f943ae50e6183719a4af81104e8
DIFF: https://github.com/llvm/llvm-project/commit/9a2e2de15f108f943ae50e6183719a4af81104e8.diff
LOG: [lld-macho] Change loadReexport to handle the case where a TAPI re-exports to reference documents nested within other TBD.
Currently, it was delibrately impleneted to not handle this case, but as it has turnt out, we need this feature.
The concrete use case is
`System/Library/Frameworks/Cocoa.framework/Versions/A/Cocoa` reexports
/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit , which then rexports
/System/Library/PrivateFrameworks/UIFoundation.framework/Versions/A/UIFoundation
The current implemention uses a global currentTopLevelTapi, which is not reset until it finishes loading the whole tree.
This is a problem because if the top-level is set to Cocoa, then when we get to UIFoundation, it will try to find UIFoundation in the current top level, which is Cocoa and will not find it.
The right thing should be:
- When loading a library from a TBD file, re-exports need to be looked up in the auxiliary documents within the same TBD.
- When loading from an actual dylib, no additional TBD documents need to be examined.
- In no case does a re-export mentioned in one TBD file need to be looked up in a document in an auxiliary document from a different TBD file
Differential Revision: https://reviews.llvm.org/D97438
Added:
lld/test/MachO/Inputs/iPhoneSimulator.sdk/usr/lib/libReexportSystem.tbd
lld/test/MachO/reexport-nested-lib.s
Modified:
lld/MachO/Driver.h
lld/MachO/InputFiles.cpp
lld/test/MachO/Inputs/iPhoneSimulator.sdk/usr/lib/libSystem.tbd
llvm/include/llvm/TextAPI/MachO/InterfaceFile.h
llvm/lib/TextAPI/MachO/InterfaceFile.cpp
Removed:
################################################################################
diff --git a/lld/MachO/Driver.h b/lld/MachO/Driver.h
index bf7abdda4e01..8176e9828035 100644
--- a/lld/MachO/Driver.h
+++ b/lld/MachO/Driver.h
@@ -15,6 +15,12 @@
#include "llvm/Option/OptTable.h"
#include "llvm/Support/MemoryBuffer.h"
+namespace llvm {
+namespace MachO {
+class InterfaceFile;
+} // namespace MachO
+} // namespace llvm
+
namespace lld {
namespace macho {
diff --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index dd1b65525fde..0015cff2e4a5 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -575,19 +575,16 @@ static Optional<DylibFile *> loadDylib(StringRef path, DylibFile *umbrella) {
// TBD files are parsed into a series of TAPI documents (InterfaceFiles), with
// the first document storing child pointers to the rest of them. When we are
-// processing a given TBD file, we store that top-level document here. When
-// processing re-exports, we search its children for potentially matching
-// documents in the same TBD file. Note that the children themselves don't
-// point to further documents, i.e. this is a two-level tree.
+// processing a given TBD file, we store that top-level document in
+// currentTopLevelTapi. When processing re-exports, we search its children for
+// potentially matching documents in the same TBD file. Note that the children
+// themselves don't point to further documents, i.e. this is a two-level tree.
//
-// ld64 allows a TAPI re-export to reference documents nested within other TBD
-// files, but that seems like a strange design, so this is an intentional
-// deviation.
-const InterfaceFile *currentTopLevelTapi = nullptr;
-
// Re-exports can either refer to on-disk files, or to documents within .tbd
// files.
-static Optional<DylibFile *> findDylib(StringRef path, DylibFile *umbrella) {
+static Optional<DylibFile *>
+findDylib(StringRef path, DylibFile *umbrella,
+ const InterfaceFile *currentTopLevelTapi) {
if (path::is_absolute(path, path::Style::posix))
for (StringRef root : config->systemLibraryRoots)
if (Optional<std::string> dylibPath =
@@ -631,8 +628,10 @@ static bool isImplicitlyLinked(StringRef path) {
return false;
}
-void loadReexport(StringRef path, DylibFile *umbrella) {
- Optional<DylibFile *> reexport = findDylib(path, umbrella);
+void loadReexport(StringRef path, DylibFile *umbrella,
+ const InterfaceFile *currentTopLevelTapi) {
+ Optional<DylibFile *> reexport =
+ findDylib(path, umbrella, currentTopLevelTapi);
if (!reexport)
error("unable to locate re-export with install name " + path);
else if (isImplicitlyLinked(path))
@@ -690,7 +689,7 @@ DylibFile::DylibFile(MemoryBufferRef mb, DylibFile *umbrella,
const auto *c = reinterpret_cast<const dylib_command *>(cmd);
StringRef reexportPath =
reinterpret_cast<const char *>(c) + read32le(&c->dylib.name);
- loadReexport(reexportPath, umbrella);
+ loadReexport(reexportPath, umbrella, nullptr);
}
// FIXME: What about LC_LOAD_UPWARD_DYLIB, LC_LAZY_LOAD_DYLIB,
@@ -701,7 +700,7 @@ DylibFile::DylibFile(MemoryBufferRef mb, DylibFile *umbrella,
const auto *c = reinterpret_cast<const dylib_command *>(cmd);
StringRef dylibPath =
reinterpret_cast<const char *>(c) + read32le(&c->dylib.name);
- Optional<DylibFile *> dylib = findDylib(dylibPath, umbrella);
+ Optional<DylibFile *> dylib = findDylib(dylibPath, umbrella, nullptr);
if (!dylib)
error(Twine("unable to locate library '") + dylibPath +
"' loaded from '" + toString(this) + "' for -flat_namespace");
@@ -758,17 +757,11 @@ DylibFile::DylibFile(const InterfaceFile &interface, DylibFile *umbrella,
}
}
- bool isTopLevelTapi = false;
- if (currentTopLevelTapi == nullptr) {
- currentTopLevelTapi = &interface;
- isTopLevelTapi = true;
- }
+ const InterfaceFile *topLevel =
+ interface.getParent() == nullptr ? &interface : interface.getParent();
for (InterfaceFileRef intfRef : interface.reexportedLibraries())
- loadReexport(intfRef.getInstallName(), umbrella);
-
- if (isTopLevelTapi)
- currentTopLevelTapi = nullptr;
+ loadReexport(intfRef.getInstallName(), umbrella, topLevel);
}
ArchiveFile::ArchiveFile(std::unique_ptr<object::Archive> &&f)
diff --git a/lld/test/MachO/Inputs/iPhoneSimulator.sdk/usr/lib/libReexportSystem.tbd b/lld/test/MachO/Inputs/iPhoneSimulator.sdk/usr/lib/libReexportSystem.tbd
new file mode 100644
index 000000000000..c0fa06bcc8a4
--- /dev/null
+++ b/lld/test/MachO/Inputs/iPhoneSimulator.sdk/usr/lib/libReexportSystem.tbd
@@ -0,0 +1,10 @@
+--- !tapi-tbd-v3
+archs: [ i386, x86_64 ]
+uuids: [ 'i386: 00000000-0000-0000-0000-000000000000', 'x86_64: 00000000-0000-0000-0000-000000000001' ]
+platform: ios
+install-name: '/usr/lib/libReexportSystem.dylib'
+exports:
+ - archs: [ i386, x86_64 ]
+ re-exports: [ '/usr/lib/libSystem' ]
+ symbols: [ __crashreporter_info__, _cache_create ]
+
diff --git a/lld/test/MachO/Inputs/iPhoneSimulator.sdk/usr/lib/libSystem.tbd b/lld/test/MachO/Inputs/iPhoneSimulator.sdk/usr/lib/libSystem.tbd
index 64c512b712a9..529bb608d8a2 100644
--- a/lld/test/MachO/Inputs/iPhoneSimulator.sdk/usr/lib/libSystem.tbd
+++ b/lld/test/MachO/Inputs/iPhoneSimulator.sdk/usr/lib/libSystem.tbd
@@ -6,8 +6,8 @@ install-name: '/usr/lib/libSystem.dylib'
current-version: 1281
exports:
- archs: [ i386, x86_64 ]
- re-exports: [ '/usr/lib/system/libcache.dylib' ]
- symbols: [ __crashreporter_info__ ]
+ re-exports: [ '/usr/lib/system/libcache.dylib', ]
+ symbols: [ __crashreporter_info__, _cache_create ]
--- !tapi-tbd-v3
archs: [ i386, x86_64 ]
uuids: [ 'i386: 00000000-0000-0000-0000-000000000002', 'x86_64: 00000000-0000-0000-0000-000000000003' ]
diff --git a/lld/test/MachO/reexport-nested-lib.s b/lld/test/MachO/reexport-nested-lib.s
new file mode 100644
index 000000000000..c6e8c19d2b63
--- /dev/null
+++ b/lld/test/MachO/reexport-nested-lib.s
@@ -0,0 +1,28 @@
+# REQUIRES: x86
+#
+# This tests that we can reference symbols from a dylib,
+# re-exported by a top-level tapi document, which itself is
+# re-exported by another top-level tapi document.
+#
+# RUN: rm -rf %t; mkdir -p %t
+# RUN: llvm-mc -filetype obj -triple x86_64-apple-darwin %s -o %t/test.o
+# RUN: %lld -o %t/test -syslibroot %S/Inputs/iPhoneSimulator.sdk -lReexportSystem %t/test.o
+# RUN: llvm-objdump %t/test --macho --bind %t/test | FileCheck %s
+
+# CHECK: segment section address type addend dylib symbol
+# CHECK: __DATA __data 0x{{[0-9a-f]*}} pointer 0 libReexportSystem __crashreporter_info__
+# CHECK: __DATA __data 0x{{[0-9a-f]*}} pointer 0 libReexportSystem _cache_create
+
+.text
+.globl _main
+
+_main:
+ ret
+
+.data
+// This symbol is from libSystem, which is re-exported by libReexportSystem.
+// Reference it here to verify that it is visible.
+.quad __crashreporter_info__
+
+// This symbol is from /usr/lib/system/libcache.dylib, which is re-exported in libSystem.
+.quad _cache_create
diff --git a/llvm/include/llvm/TextAPI/MachO/InterfaceFile.h b/llvm/include/llvm/TextAPI/MachO/InterfaceFile.h
index f77601d037c3..3aaf6e23b633 100644
--- a/llvm/include/llvm/TextAPI/MachO/InterfaceFile.h
+++ b/llvm/include/llvm/TextAPI/MachO/InterfaceFile.h
@@ -338,6 +338,9 @@ class InterfaceFile {
///\param Document The library to inline with top level library.
void addDocument(std::shared_ptr<InterfaceFile> &&Document);
+ /// Returns the pointer to parent document if exists or nullptr otherwise.
+ InterfaceFile *getParent() const { return Parent; }
+
/// Get the list of inlined libraries.
///
/// \return Returns a list of the inlined frameworks.
@@ -431,6 +434,7 @@ class InterfaceFile {
std::vector<std::shared_ptr<InterfaceFile>> Documents;
std::vector<std::pair<Target, std::string>> UUIDs;
SymbolMapType Symbols;
+ InterfaceFile *Parent = nullptr;
};
template <typename DerivedT, typename KeyInfoT, typename BucketT>
diff --git a/llvm/lib/TextAPI/MachO/InterfaceFile.cpp b/llvm/lib/TextAPI/MachO/InterfaceFile.cpp
index 5b73cfefd28b..9e13a61fdf5f 100644
--- a/llvm/lib/TextAPI/MachO/InterfaceFile.cpp
+++ b/llvm/lib/TextAPI/MachO/InterfaceFile.cpp
@@ -124,6 +124,7 @@ void InterfaceFile::addDocument(std::shared_ptr<InterfaceFile> &&Document) {
const std::shared_ptr<InterfaceFile> &RHS) {
return LHS->InstallName < RHS->InstallName;
});
+ Document->Parent = this;
Documents.insert(Pos, Document);
}
More information about the llvm-commits
mailing list