[lld-macho] Filter TAPI re-exports by target
Previously, we were loading re-exports without checking whether
they were compatible with our target. Prior to {D97209}, it meant that
we were defining dylib symbols that were invalid -- usually a silent
failure unless our binary actually used them. D97209 exposed this as an
explicit error.
Along the way, I've extended our TAPI compatibility check to cover the
platform as well, instead of just checking the arch. To this end, I've
replaced MachO::Architecture with MachO::Target in our Config struct.
Reviewed By: #lld-macho, oontvoo
Differential Revision: https://reviews.llvm.org/D97867
GitOrigin-RevId: 55a32812fa5ec4d49839ea6f61b50d08a389ab09
diff --git a/MachO/Config.h b/MachO/Config.h
index df5d1f8..f7a22e1 100644
--- a/MachO/Config.h
+++ b/MachO/Config.h
@@ -15,6 +15,7 @@
#include "llvm/Support/VersionTuple.h"
#include "llvm/TextAPI/MachO/Architecture.h"
#include "llvm/TextAPI/MachO/Platform.h"
+#include "llvm/TextAPI/MachO/Target.h"
#include <vector>
@@ -29,7 +30,6 @@
using SegmentRenameMap = llvm::DenseMap<llvm::StringRef, llvm::StringRef>;
struct PlatformInfo {
- llvm::MachO::PlatformKind kind;
llvm::VersionTuple minimum;
llvm::VersionTuple sdk;
};
@@ -68,8 +68,8 @@
llvm::StringRef outputFile;
llvm::StringRef ltoObjPath;
bool demangle = false;
- llvm::MachO::Architecture arch;
- PlatformInfo platform;
+ llvm::MachO::Target target;
+ PlatformInfo platformInfo;
NamespaceKind namespaceKind = NamespaceKind::twolevel;
UndefinedSymbolTreatment undefinedSymbolTreatment =
UndefinedSymbolTreatment::error;
diff --git a/MachO/Driver.cpp b/MachO/Driver.cpp
index f6d22c8..4868537 100644
--- a/MachO/Driver.cpp
+++ b/MachO/Driver.cpp
@@ -125,21 +125,6 @@
return {};
}
-static TargetInfo *createTargetInfo(opt::InputArgList &args) {
- StringRef archName = args.getLastArgValue(OPT_arch);
- if (archName.empty())
- fatal("must specify -arch");
- config->arch = MachO::getArchitectureFromName(archName);
- switch (MachO::getCPUTypeFromArchitecture(config->arch).first) {
- case MachO::CPU_TYPE_X86_64:
- return createX86_64TargetInfo();
- case MachO::CPU_TYPE_ARM64:
- return createARM64TargetInfo();
- default:
- fatal("missing or unsupported -arch " + archName);
- }
-}
-
static bool warnIfNotDirectory(StringRef option, StringRef path) {
if (!fs::exists(path)) {
warn("directory not found for option -" + option + path);
@@ -558,12 +543,12 @@
map_iterator(s.end(), toLowerDash));
}
-static PlatformInfo getPlatformVersion(const opt::ArgList &args) {
+// Has the side-effect of setting Config::platformInfo.
+static PlatformKind parsePlatformVersion(const opt::ArgList &args) {
const opt::Arg *arg = args.getLastArg(OPT_platform_version);
- PlatformInfo platform;
if (!arg) {
error("must specify -platform_version");
- return platform;
+ return PlatformKind::unknown;
}
StringRef platformStr = arg->getValue(0);
@@ -571,7 +556,7 @@
StringRef sdkVersionStr = arg->getValue(2);
// TODO(compnerd) see if we can generate this case list via XMACROS
- platform.kind =
+ PlatformKind platform =
StringSwitch<PlatformKind>(lowerDash(platformStr))
.Cases("macos", "1", PlatformKind::macOS)
.Cases("ios", "2", PlatformKind::iOS)
@@ -584,19 +569,39 @@
.Cases("watchos-simulator", "9", PlatformKind::watchOSSimulator)
.Cases("driverkit", "10", PlatformKind::driverKit)
.Default(PlatformKind::unknown);
- if (platform.kind == PlatformKind::unknown)
+ if (platform == PlatformKind::unknown)
error(Twine("malformed platform: ") + platformStr);
// TODO: check validity of version strings, which varies by platform
// NOTE: ld64 accepts version strings with 5 components
// llvm::VersionTuple accepts no more than 4 components
// Has Apple ever published version strings with 5 components?
- if (platform.minimum.tryParse(minVersionStr))
+ if (config->platformInfo.minimum.tryParse(minVersionStr))
error(Twine("malformed minimum version: ") + minVersionStr);
- if (platform.sdk.tryParse(sdkVersionStr))
+ if (config->platformInfo.sdk.tryParse(sdkVersionStr))
error(Twine("malformed sdk version: ") + sdkVersionStr);
return platform;
}
+// Has the side-effect of setting Config::target.
+static TargetInfo *createTargetInfo(opt::InputArgList &args) {
+ StringRef archName = args.getLastArgValue(OPT_arch);
+ if (archName.empty())
+ fatal("must specify -arch");
+ PlatformKind platform = parsePlatformVersion(args);
+
+ config->target =
+ MachO::Target(MachO::getArchitectureFromName(archName), platform);
+
+ switch (MachO::getCPUTypeFromArchitecture(config->target.Arch).first) {
+ case MachO::CPU_TYPE_X86_64:
+ return createX86_64TargetInfo();
+ case MachO::CPU_TYPE_ARM64:
+ return createARM64TargetInfo();
+ default:
+ fatal("missing or unsupported -arch " + archName);
+ }
+}
+
static UndefinedSymbolTreatment
getUndefinedSymbolTreatment(const opt::ArgList &args) {
StringRef treatmentStr = args.getLastArgValue(OPT_undefined);
@@ -666,16 +671,16 @@
static bool isPie(opt::InputArgList &args) {
if (config->outputType != MH_EXECUTE || args.hasArg(OPT_no_pie))
return false;
- if (config->arch == AK_arm64 || config->arch == AK_arm64e)
+ if (config->target.Arch == AK_arm64 || config->target.Arch == AK_arm64e)
return true;
// TODO: add logic here as we support more archs. E.g. i386 should default
// to PIE from 10.7
- assert(config->arch == AK_x86_64 || config->arch == AK_x86_64h);
+ assert(config->target.Arch == AK_x86_64 || config->target.Arch == AK_x86_64h);
- PlatformKind kind = config->platform.kind;
+ PlatformKind kind = config->target.Platform;
if (kind == PlatformKind::macOS &&
- config->platform.minimum >= VersionTuple(10, 6))
+ config->platformInfo.minimum >= VersionTuple(10, 6))
return true;
if (kind == PlatformKind::iOSSimulator || kind == PlatformKind::driverKit)
@@ -759,7 +764,6 @@
config = make<Configuration>();
symtab = make<SymbolTable>();
target = createTargetInfo(args);
- config->platform = getPlatformVersion(args);
config->entry = symtab->addUndefined(args.getLastArgValue(OPT_e, "_main"),
/*file=*/nullptr,
diff --git a/MachO/InputFiles.cpp b/MachO/InputFiles.cpp
index 7c35999..cf2b8cc 100644
--- a/MachO/InputFiles.cpp
+++ b/MachO/InputFiles.cpp
@@ -480,10 +480,10 @@
MachO::Architecture arch =
MachO::getArchitectureFromCpuType(hdr->cputype, hdr->cpusubtype);
- if (arch != config->arch) {
+ if (arch != config->target.Arch) {
error(toString(this) + " has architecture " + getArchitectureName(arch) +
" which is incompatible with target architecture " +
- getArchitectureName(config->arch));
+ getArchitectureName(config->target.Arch));
return;
}
// TODO: check platform too
@@ -697,9 +697,9 @@
if (umbrella == nullptr)
umbrella = this;
- if (!interface.getArchitectures().has(config->arch)) {
+ if (!is_contained(interface.targets(), config->target)) {
error(toString(this) + " is incompatible with " +
- getArchitectureName(config->arch));
+ std::string(config->target));
return;
}
@@ -715,7 +715,7 @@
// TODO(compnerd) filter out symbols based on the target platform
// TODO: handle weak defs, thread locals
for (const auto symbol : interface.symbols()) {
- if (!symbol->getArchitectures().has(config->arch))
+ if (!symbol->getArchitectures().has(config->target.Arch))
continue;
switch (symbol->getKind()) {
@@ -740,8 +740,11 @@
const InterfaceFile *topLevel =
interface.getParent() == nullptr ? &interface : interface.getParent();
- for (InterfaceFileRef intfRef : interface.reexportedLibraries())
- loadReexport(intfRef.getInstallName(), exportingFile, topLevel);
+ for (InterfaceFileRef intfRef : interface.reexportedLibraries()) {
+ auto targets = intfRef.targets();
+ if (is_contained(targets, config->target))
+ loadReexport(intfRef.getInstallName(), exportingFile, topLevel);
+ }
}
ArchiveFile::ArchiveFile(std::unique_ptr<object::Archive> &&f)
diff --git a/MachO/LTO.cpp b/MachO/LTO.cpp
index fb74968..ae30bb7 100644
--- a/MachO/LTO.cpp
+++ b/MachO/LTO.cpp
@@ -107,7 +107,8 @@
if (!config->ltoObjPath.empty()) {
filePath = config->ltoObjPath;
path::append(filePath, Twine(i) + "." +
- getArchitectureName(config->arch) + ".lto.o");
+ getArchitectureName(config->target.Arch) +
+ ".lto.o");
saveBuffer(buf[i], filePath);
modTime = getModTime(filePath);
}
diff --git a/MachO/OutputSegment.cpp b/MachO/OutputSegment.cpp
index c00e819..1ea5d49 100644
--- a/MachO/OutputSegment.cpp
+++ b/MachO/OutputSegment.cpp
@@ -31,7 +31,7 @@
}
static uint32_t maxProt(StringRef name) {
- assert(config->arch != AK_i386 &&
+ assert(config->target.Arch != AK_i386 &&
"TODO: i386 has different maxProt requirements");
return initProt(name);
}
diff --git a/MachO/SyntheticSections.cpp b/MachO/SyntheticSections.cpp
index 7f5b0a5..49f5def 100644
--- a/MachO/SyntheticSections.cpp
+++ b/MachO/SyntheticSections.cpp
@@ -63,8 +63,8 @@
if (config->outputType == MachO::MH_EXECUTE && !config->staticLink &&
target->cpuSubtype == MachO::CPU_SUBTYPE_X86_64_ALL &&
- config->platform.kind == MachO::PlatformKind::macOS &&
- config->platform.minimum >= VersionTuple(10, 5))
+ config->target.Platform == MachO::PlatformKind::macOS &&
+ config->platformInfo.minimum >= VersionTuple(10, 5))
subtype |= MachO::CPU_SUBTYPE_LIB64;
return subtype;
diff --git a/MachO/Writer.cpp b/MachO/Writer.cpp
index 0a570a3..a4f8339 100644
--- a/MachO/Writer.cpp
+++ b/MachO/Writer.cpp
@@ -334,7 +334,8 @@
class LCBuildVersion : public LoadCommand {
public:
- LCBuildVersion(const PlatformInfo &platform) : platform(platform) {}
+ LCBuildVersion(PlatformKind platform, const PlatformInfo &platformInfo)
+ : platform(platform), platformInfo(platformInfo) {}
const int ntools = 1;
@@ -346,13 +347,13 @@
auto *c = reinterpret_cast<build_version_command *>(buf);
c->cmd = LC_BUILD_VERSION;
c->cmdsize = getSize();
- c->platform = static_cast<uint32_t>(platform.kind);
- c->minos = ((platform.minimum.getMajor() << 020) |
- (platform.minimum.getMinor().getValueOr(0) << 010) |
- platform.minimum.getSubminor().getValueOr(0));
- c->sdk = ((platform.sdk.getMajor() << 020) |
- (platform.sdk.getMinor().getValueOr(0) << 010) |
- platform.sdk.getSubminor().getValueOr(0));
+ c->platform = static_cast<uint32_t>(platform);
+ c->minos = ((platformInfo.minimum.getMajor() << 020) |
+ (platformInfo.minimum.getMinor().getValueOr(0) << 010) |
+ platformInfo.minimum.getSubminor().getValueOr(0));
+ c->sdk = ((platformInfo.sdk.getMajor() << 020) |
+ (platformInfo.sdk.getMinor().getValueOr(0) << 010) |
+ platformInfo.sdk.getSubminor().getValueOr(0));
c->ntools = ntools;
auto *t = reinterpret_cast<build_tool_version *>(&c[1]);
t->tool = TOOL_LD;
@@ -360,7 +361,8 @@
LLVM_VERSION_PATCH;
}
- const PlatformInfo &platform;
+ PlatformKind platform;
+ const PlatformInfo &platformInfo;
};
// Stores a unique identifier for the output file based on an MD5 hash of its
@@ -523,7 +525,8 @@
uuidCommand = make<LCUuid>();
in.header->addLoadCommand(uuidCommand);
- in.header->addLoadCommand(make<LCBuildVersion>(config->platform));
+ in.header->addLoadCommand(
+ make<LCBuildVersion>(config->target.Platform, config->platformInfo));
int64_t dylibOrdinal = 1;
for (InputFile *file : inputFiles) {
@@ -723,7 +726,7 @@
symtabSection = make<SymtabSection>(*stringTableSection);
indirectSymtabSection = make<IndirectSymtabSection>();
if (config->outputType == MH_EXECUTE &&
- (config->arch == AK_arm64 || config->arch == AK_arm64e))
+ (config->target.Arch == AK_arm64 || config->target.Arch == AK_arm64e))
codeSignatureSection = make<CodeSignatureSection>();
switch (config->outputType) {
diff --git a/test/MachO/Inputs/libStubLink.tbd b/test/MachO/Inputs/libStubLink.tbd
new file mode 100644
index 0000000..6e6237e
--- /dev/null
+++ b/test/MachO/Inputs/libStubLink.tbd
@@ -0,0 +1,24 @@
+--- !tapi-tbd
+tbd-version: 4
+targets: [ arm64-ios-simulator, arm64-ios ]
+uuids:
+ - target: arm64-ios-simulator
+ value: 00000000-0000-0000-0000-000000000002
+ - target: arm64-ios
+ value: 00000000-0000-0000-0000-000000000003
+install-name: '/usr/lib/libStubLink.dylib'
+current-version: 1.0.0
+reexported-libraries:
+ - targets: [ arm64-ios-simulator ]
+ libraries: [ '/usr/lib/libArm64Sim.dylib' ]
+--- !tapi-tbd
+tbd-version: 4
+targets: [ arm64-ios-simulator ]
+uuids:
+ - target: arm64-ios-simulator
+ value: 00000000-0000-0000-0000-000000000001
+install-name: '/usr/lib/libArm64Sim.dylib'
+current-version: 1.0.0
+exports:
+ - targets: [ arm64-ios-simulator ]
+ symbols: [ _arm64_sim_only ]
diff --git a/test/MachO/invalid/Inputs/libincompatible.tbd b/test/MachO/invalid/Inputs/libincompatible.tbd
index a07e81b..4a2ea22 100644
--- a/test/MachO/invalid/Inputs/libincompatible.tbd
+++ b/test/MachO/invalid/Inputs/libincompatible.tbd
@@ -7,3 +7,4 @@
exports:
- archs: [ 'x86_64', 'arm64' ]
symbols: [ ]
+...
diff --git a/test/MachO/invalid/incompatible-arch-tapi.s b/test/MachO/invalid/incompatible-arch-tapi.s
deleted file mode 100644
index e0a0f4a..0000000
--- a/test/MachO/invalid/incompatible-arch-tapi.s
+++ /dev/null
@@ -1,10 +0,0 @@
-# REQUIRES: x86
-# RUN: rm -rf %t; split-file %s %t
-# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-macosx %t/main.s -o %t/main.o
-# RUN: not %lld -arch x86_64 -lSystem %S/Inputs/libincompatible.tbd %t/main.o -o /dev/null 2>&1 | FileCheck %s --check-prefix=ARCH
-# ARCH: error: {{.*}}libincompatible.tbd is incompatible with x86_64
-
-#--- main.s
-.globl _main
-_main:
- ret
diff --git a/test/MachO/invalid/incompatible-target-tapi.test b/test/MachO/invalid/incompatible-target-tapi.test
new file mode 100644
index 0000000..47d5452
--- /dev/null
+++ b/test/MachO/invalid/incompatible-target-tapi.test
@@ -0,0 +1,10 @@
+REQUIRES: x86, aarch64
+RUN: rm -rf %t; mkdir -p %t
+RUN: echo "" | llvm-mc -filetype=obj -triple=x86_64-apple-macosx -o %t/x86_64-test.o
+RUN: echo "" | llvm-mc -filetype=obj -triple=arm64-apple-iossimulator -o %t/arm64-test.o
+RUN: not %lld -dylib -arch x86_64 %S/Inputs/libincompatible.tbd %t/x86_64-test.o \
+RUN: -o /dev/null 2>&1 | FileCheck %s --check-prefix=ARCH
+RUN: not %lld -dylib -arch arm64 -platform_version ios-simulator 14.0 15.0 %t/arm64-test.o \
+RUN: %S/Inputs/libincompatible.tbd -o /dev/null 2>&1 | FileCheck %s --check-prefix=PLATFORM
+ARCH: error: {{.*}}libincompatible.tbd is incompatible with x86_64 (macOS)
+PLATFORM: error: {{.*}}libincompatible.tbd is incompatible with arm64 (iOS Simulator)
diff --git a/test/MachO/stub-link-by-arch.s b/test/MachO/stub-link-by-arch.s
new file mode 100644
index 0000000..405cbfd
--- /dev/null
+++ b/test/MachO/stub-link-by-arch.s
@@ -0,0 +1,19 @@
+# REQUIRES: x86, aarch64
+
+# RUN: mkdir -p %t
+# RUN: llvm-mc -filetype obj -triple arm64-apple-ios14.4 %s -o %t/arm64-ios.o
+# RUN: not %lld -dylib -arch arm64 -platform_version ios 10 11 -o /dev/null \
+# RUN: -lSystem %S/Inputs/libStubLink.tbd %t/arm64-ios.o 2>&1 | FileCheck %s
+
+# RUN: llvm-mc -filetype obj -triple x86_64-apple-iossimulator14.4 %s -o %t/x86_64-sim.o
+# RUN: not %lld -dylib -arch x86_64 -platform_version ios-simulator 10 11 -o /dev/null \
+# RUN: -lSystem %S/Inputs/libStubLink.tbd %t/x86_64-sim.o 2>&1 | FileCheck %s
+
+# RUN: llvm-mc -filetype obj -triple arm64-apple-iossimulator14.4 %s -o %t/arm64-sim.o
+# RUN: %lld -dylib -arch arm64 -platform_version ios-simulator 10 11 -o \
+# RUN: /dev/null %S/Inputs/libStubLink.tbd %t/arm64-sim.o
+
+# CHECK: error: undefined symbol: _arm64_sim_only
+
+.data
+.quad _arm64_sim_only
