[lld] 55a3281 - [lld-macho] Filter TAPI re-exports by target

Jez Ng via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 4 11:37:13 PST 2021


Author: Jez Ng
Date: 2021-03-04T14:36:47-05:00
New Revision: 55a32812fa5ec4d49839ea6f61b50d08a389ab09

URL: https://github.com/llvm/llvm-project/commit/55a32812fa5ec4d49839ea6f61b50d08a389ab09
DIFF: https://github.com/llvm/llvm-project/commit/55a32812fa5ec4d49839ea6f61b50d08a389ab09.diff

LOG: [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

Added: 
    lld/test/MachO/Inputs/libStubLink.tbd
    lld/test/MachO/invalid/incompatible-target-tapi.test
    lld/test/MachO/stub-link-by-arch.s

Modified: 
    lld/MachO/Config.h
    lld/MachO/Driver.cpp
    lld/MachO/InputFiles.cpp
    lld/MachO/LTO.cpp
    lld/MachO/OutputSegment.cpp
    lld/MachO/SyntheticSections.cpp
    lld/MachO/Writer.cpp
    lld/test/MachO/invalid/Inputs/libincompatible.tbd

Removed: 
    lld/test/MachO/invalid/incompatible-arch-tapi.s


################################################################################
diff  --git a/lld/MachO/Config.h b/lld/MachO/Config.h
index df5d1f867e2d..f7a22e1e3301 100644
--- a/lld/MachO/Config.h
+++ b/lld/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 SectionRenameMap = llvm::DenseMap<NamePair, NamePair>;
 using SegmentRenameMap = llvm::DenseMap<llvm::StringRef, llvm::StringRef>;
 
 struct PlatformInfo {
-  llvm::MachO::PlatformKind kind;
   llvm::VersionTuple minimum;
   llvm::VersionTuple sdk;
 };
@@ -68,8 +68,8 @@ struct Configuration {
   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/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index f6d22c828bb1..48685370d2b5 100644
--- a/lld/MachO/Driver.cpp
+++ b/lld/MachO/Driver.cpp
@@ -125,21 +125,6 @@ static Optional<std::string> findFramework(StringRef name) {
   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 @@ static std::string lowerDash(StringRef s) {
                      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 @@ static PlatformInfo getPlatformVersion(const opt::ArgList &args) {
   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 @@ static PlatformInfo getPlatformVersion(const opt::ArgList &args) {
           .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 const char *getReproduceOption(opt::InputArgList &args) {
 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 @@ bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
   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/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index 7c35999582c8..cf2b8cc16c20 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/lld/MachO/InputFiles.cpp
@@ -480,10 +480,10 @@ ObjFile::ObjFile(MemoryBufferRef mb, uint32_t modTime, StringRef archiveName)
 
   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 @@ DylibFile::DylibFile(const InterfaceFile &interface, DylibFile *umbrella,
   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 @@ DylibFile::DylibFile(const InterfaceFile &interface, DylibFile *umbrella,
   // 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 @@ DylibFile::DylibFile(const InterfaceFile &interface, DylibFile *umbrella,
   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/lld/MachO/LTO.cpp b/lld/MachO/LTO.cpp
index fb749687e864..ae30bb7605d5 100644
--- a/lld/MachO/LTO.cpp
+++ b/lld/MachO/LTO.cpp
@@ -107,7 +107,8 @@ std::vector<ObjFile *> BitcodeCompiler::compile() {
     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/lld/MachO/OutputSegment.cpp b/lld/MachO/OutputSegment.cpp
index c00e819ea3a4..1ea5d49a32a1 100644
--- a/lld/MachO/OutputSegment.cpp
+++ b/lld/MachO/OutputSegment.cpp
@@ -31,7 +31,7 @@ static uint32_t initProt(StringRef name) {
 }
 
 static uint32_t maxProt(StringRef name) {
-  assert(config->arch != AK_i386 &&
+  assert(config->target.Arch != AK_i386 &&
          "TODO: i386 has 
diff erent maxProt requirements");
   return initProt(name);
 }

diff  --git a/lld/MachO/SyntheticSections.cpp b/lld/MachO/SyntheticSections.cpp
index 7f5b0a56d5ff..49f5deffd002 100644
--- a/lld/MachO/SyntheticSections.cpp
+++ b/lld/MachO/SyntheticSections.cpp
@@ -63,8 +63,8 @@ static uint32_t cpuSubtype() {
 
   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/lld/MachO/Writer.cpp b/lld/MachO/Writer.cpp
index 0a570a31758a..a4f83390bbb0 100644
--- a/lld/MachO/Writer.cpp
+++ b/lld/MachO/Writer.cpp
@@ -334,7 +334,8 @@ class LCRPath : public LoadCommand {
 
 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 @@ class LCBuildVersion : public LoadCommand {
     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 @@ class LCBuildVersion : public LoadCommand {
                  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 @@ void Writer::createLoadCommands() {
   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 @@ void Writer::createOutputSections() {
   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/lld/test/MachO/Inputs/libStubLink.tbd b/lld/test/MachO/Inputs/libStubLink.tbd
new file mode 100644
index 000000000000..6e6237edbc8a
--- /dev/null
+++ b/lld/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/lld/test/MachO/invalid/Inputs/libincompatible.tbd b/lld/test/MachO/invalid/Inputs/libincompatible.tbd
index a07e81b0407d..4a2ea227e99a 100644
--- a/lld/test/MachO/invalid/Inputs/libincompatible.tbd
+++ b/lld/test/MachO/invalid/Inputs/libincompatible.tbd
@@ -7,3 +7,4 @@ current-version:  0001.001.1
 exports:
   - archs:        [ 'x86_64', 'arm64' ]
     symbols:      [ ]
+...

diff  --git a/lld/test/MachO/invalid/incompatible-arch-tapi.s b/lld/test/MachO/invalid/incompatible-arch-tapi.s
deleted file mode 100644
index e0a0f4a52741..000000000000
--- a/lld/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/lld/test/MachO/invalid/incompatible-target-tapi.test b/lld/test/MachO/invalid/incompatible-target-tapi.test
new file mode 100644
index 000000000000..47d54527a945
--- /dev/null
+++ b/lld/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/lld/test/MachO/stub-link-by-arch.s b/lld/test/MachO/stub-link-by-arch.s
new file mode 100644
index 000000000000..405cbfd52ed0
--- /dev/null
+++ b/lld/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


        


More information about the llvm-commits mailing list