lld: initial pass at supporting TBD

[compnerd]

Summary

Add support to lld to use Text Based API stubs for linking. This is
support is incomplete not filtering out platforms. It also does not
account for architecture specific API handling and potentially does not
correctly handle trees of re-exports with inlined libraries being
treated as direct children of the top level library.

[Ktwu]

This is largely incomplete, not filtering out platforms and architectures.

This is a must-have for an intro diff imo. Unfortunately filtering things down isn't as simple as what @int3 implemented for true dylibs since architecture and platform information is per symbol, not a chunk of the file...

[compnerd]

Update test to validate the binding

[compnerd]

Fix memory corruption in the binding name. We were holding a reference to a StringRef beyond the lifetime of the buffer.

Add a failing demonstrating the failure to filter out the architecture specific symbols.

[compnerd]

Add support for filtering architectures. Fix the negative test case.

[Ktwu]

This is largely incomplete, not filtering out platforms and architectures.

Not true anymore :D

potentially does not correct handle trees of re-exports with inlined

libraries being treated as direct children of the top level library.

Add a TODO?

lld/test/MachO/iPhoneSimulator.sdk/usr/lib/libSystem.tbd

1

It's nice to know where these files came from, but there's not a need to create a full copy that even preserves the path, right? If anything, you should be able to combine the two stub files into one manually and use them both for your test.

For example, you don't use crashreporeter_info, so get rid of it.

lld/test/MachO/stub-link-2.s

1

stub-link-2 isn't descriptive. Move this into /test/MachO/invalid and give it a better name like "undefined-stub-symbol"?

[compnerd]

lld/test/MachO/iPhoneSimulator.sdk/usr/lib/libSystem.tbd

1

Keeping them separate is important IMO, and far more useful. This is going to grow over time, and get used for future tests as the functionality improves. The paths here are not indicative of where they originate from - the UUIDs are pretty obviously sequential - as these are fabricated. The versions happen to be cause I had a tab open to some revision of opensource.apple.com.

The single symbol there is there because its useful for testing symbol origins from the parent umbrella vs re-exported dylibs.

lld/test/MachO/stub-link-2.s

1

Its not really an invalid test, its a negative test case.

[compnerd]

Update commit message, add some TODOs as suggested.

[Ktwu]

lld/test/MachO/iPhoneSimulator.sdk/usr/lib/libSystem.tbd

1

This is going to grow over time, and get used for future tests as the functionality improves.

Hmmm, I was under the impression that it's better to be intentional in testing for supported features (don't add red herrings in tests that could give a reader a false impression of which scenarios are tested).

The single symbol there is there because its useful for testing symbol origins from the parent umbrella vs re-exported dylibs.

But this isn't actively tested :(

The versions happen to be cause I had a tab open to some revision of opensource.apple.com

Ok, since the paths aren't really important, can you flatten them and stick them into lld/test/Macho/Inputs?

lld/test/MachO/stub-link-2.s

1

"invalid" is where negative / error-based tests live for lld (or ought to live, there are probably a few exceptions from before thinking such a place was a good idea).

[compnerd]

lld/test/MachO/iPhoneSimulator.sdk/usr/lib/libSystem.tbd

1

I agree about being intentional about testing. However, reconstructing this seems unnecessary. This is going to end up getting used in short order. The TBD support here is very rudimentary and needs to expand. This is related to one of the TODOs that is mentioned. I don't think that the extra symbol is that big of a deal and is something that will make it easier for expanding the test coverage.

Sure, moving the SDKs into the Inputs is fine, I'll move them. However, keeping the SDKs as SDKs is important IMO as they make it easier to understand what is being tested. It makes it easier to understand the test cases when you are looking at the invocation.

lld/test/MachO/stub-link-2.s

1

Hmm, okay, I can move the test there. I think that we should remove the directory - this is going against the rest of the project's practices and creates unnecessary friction.

[compnerd]

shuffle files around as recommended by @Ktwu

[Ktwu]

Thanks!

[compnerd]

rG6fe27b5fed92aca21386a1ec3d1e4aa70e82e9fa

lld/MachO/Driver.cpp

if (missingCount)
     error(Twine(args.getArgString(missingIndex)) + ": missing argument");
 
   for (opt::Arg *arg : args.filtered(OPT_UNKNOWN))
     error("unknown argument: " + arg->getSpelling());
   return args;
 }
 
 static Optional<std::string> findLibrary(StringRef name) {
+  std::string stub = (llvm::Twine("lib") + name + ".tbd").str();
   std::string shared = (llvm::Twine("lib") + name + ".dylib").str();
   std::string archive = (llvm::Twine("lib") + name + ".a").str();
   llvm::SmallString<260> location;
 
   for (StringRef dir : config->searchPaths) {
-    for (StringRef library : {shared, archive}) {
+    for (StringRef library : {stub, shared, archive}) {
       location = dir;
       llvm::sys::path::append(location, library);
       if (fs::exists(location))
         return location.str().str();
     }
   }
-  return None;
+  return {};
 }
 
 static TargetInfo *createTargetInfo(opt::InputArgList &args) {
   StringRef arch = llvm::Triple(LLVM_DEFAULT_TARGET_TRIPLE).getArchName();
   config->arch =
       llvm::MachO::getArchitectureFromName(args.getLastArgValue(OPT_arch, arch));
   switch (config->arch) {
   case llvm::MachO::AK_x86_64:
case file_magic::archive: {
     break;
   }
   case file_magic::macho_object:
     inputFiles.push_back(make<ObjFile>(mbref));
     break;
   case file_magic::macho_dynamically_linked_shared_lib:
     inputFiles.push_back(make<DylibFile>(mbref));
     break;
+  case file_magic::tapi_file: {
+    llvm::Expected<std::unique_ptr<llvm::MachO::InterfaceFile>> result =
+        TextAPIReader::get(mbref);
+    if (!result)
+      return;
+
+    std::unique_ptr<llvm::MachO::InterfaceFile> interface{std::move(*result)};
+    inputFiles.push_back(make<DylibFile>(std::move(interface)));
+    break;
+  }
   default:
     error(path + ": unhandled file type");
   }
 }
 
 static std::array<StringRef, 6> archNames{"arm",    "arm64", "i386",
                                           "x86_64", "ppc",   "ppc64"};
 static bool isArchString(StringRef s) {

lld/MachO/InputFiles.h

 
 #include "MachOStructs.h"
 
 #include "lld/Common/LLVM.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/BinaryFormat/MachO.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/TextAPI/MachO/InterfaceFile.h"
+#include "llvm/TextAPI/MachO/TextAPIReader.h"
 
 #include <map>
 #include <vector>
 
 namespace lld {
 namespace macho {
 
 class InputSection;

 public:
   explicit ObjFile(MemoryBufferRef mb);
   static bool classof(const InputFile *f) { return f->kind() == ObjKind; }
 };
 
 // .dylib file
 class DylibFile : public InputFile {
 public:
+  explicit DylibFile(std::shared_ptr<llvm::MachO::InterfaceFile> interface,
+                     DylibFile *umbrella = nullptr);
+
   // Mach-O dylibs can re-export other dylibs as sub-libraries, meaning that the
   // symbols in those sub-libraries will be available under the umbrella
   // library's namespace. Those sub-libraries can also have their own
   // re-exports. When loading a re-exported dylib, `umbrella` should be set to
   // the root dylib to ensure symbols in the child library are correctly bound
   // to the root. On the other hand, if a dylib is being directly loaded
   // (through an -lfoo flag), then `umbrella` should be a nullptr.
   explicit DylibFile(MemoryBufferRef mb, DylibFile *umbrella = nullptr);
+
   static bool classof(const InputFile *f) { return f->kind() == DylibKind; }
 
   // Do not use this constructor!! This is meant only for createLibSystemMock(),
   // but it cannot be made private as we call it via make().
   DylibFile();
   static DylibFile *createLibSystemMock();
 
   StringRef dylibName;

lld/MachO/InputFiles.cpp

for (uint32_t i = 0, n = hdr->ncmds; i < n; ++i) {
     if (!buffer) {
       error("unable to read re-exported dylib at " + reexportPath);
       return;
     }
     reexported.push_back(make<DylibFile>(*buffer, umbrella));
   }
 }
 
+DylibFile::DylibFile(std::shared_ptr<llvm::MachO::InterfaceFile> interface,
+                     DylibFile *umbrella)
+    : InputFile(DylibKind, MemoryBufferRef()) {
+  if (umbrella == nullptr)
+    umbrella = this;
+
+  dylibName = saver.save(interface->getInstallName());
+  for (const auto symbol : interface->symbols())
+    if (symbol->getArchitectures().has(config->arch))
+      symbols.push_back(symtab->addDylib(saver.save(symbol->getName()),
+                                         umbrella));
+  for (auto document : interface->documents())
+    reexported.push_back(make<DylibFile>(document, umbrella));
+}
+
 DylibFile::DylibFile() : InputFile(DylibKind, MemoryBufferRef()) {}
 
 DylibFile *DylibFile::createLibSystemMock() {
   auto *file = make<DylibFile>();
   file->mb = MemoryBufferRef("", "/usr/lib/libSystem.B.dylib");
   file->dylibName = "/usr/lib/libSystem.B.dylib";
   file->symbols.push_back(symtab->addDylib("dyld_stub_binder", file));
   return file;

lld/MachO/Writer.cpp

private:
   // different location.
   const StringRef path = "/usr/lib/dyld";
 };
 } // namespace
 
 void Writer::scanRelocations() {
   for (InputSection *isec : inputSections) {
     for (Reloc &r : isec->relocs) {
-      if (auto *s = r.target.dyn_cast<Symbol *>()) {
+      if (auto *s = r.target.dyn_cast<lld::macho::Symbol *>()) {
         if (isa<Undefined>(s))
           error("undefined symbol " + s->getName() + ", referenced from " +
                 sys::path::filename(isec->file->getName()));
         else if (auto *dylibSymbol = dyn_cast<DylibSymbol>(s))
           target->prepareDylibSymbolRelocation(*dylibSymbol, r.type);
       }
     }
   }
auto addSym = [&](Defined &sym) {
     SymbolPriorityEntry &entry = it->second;
     size_t &priority = sectionPriorities[sym.isec];
     priority = std::max(priority, getSymbolPriority(entry, *sym.isec->file));
   };
 
   // TODO: Make sure this handles weak symbols correctly.
   for (InputFile *file : inputFiles)
     if (isa<ObjFile>(file) || isa<ArchiveFile>(file))
-      for (Symbol *sym : file->symbols)
+      for (lld::macho::Symbol *sym : file->symbols)
         if (auto *d = dyn_cast<Defined>(sym))
           addSym(*d);
 
   return sectionPriorities;
 }
 
 // Sorting only can happen once all outputs have been collected. Here we sort
 // segments, output sections within each segment, and input sections within each

lld/test/MachO/MacOSX.sdk/usr/lib/libSystem.tbd

+--- !tapi-tbd-v3
+archs:            [ x86_64 ]
+uuids:            [ 'x86_64: 00000000-0000-0000-0000-000000000000' ]
+platform:         macosx
+install-name:     '/usr/lib/libSystem.B.dylib'
+current-version:  0001.001.1
+exports:
+  - archs:        [ 'x86_64' ]
+    re-exports:   [ '/usr/lib/system/libdyld.dylib',
+                    '/usr/lib/system/libsystem_c.dylib',
+                    '/usr/lib/system/libsystem_m.dylib' ]
+--- !tapi-tbd-v3
+archs:            [ x86_64 ]
+uuids:            [ 'x86_64: 00000000-0000-0000-0000-000000000001' ]
+platform:         macosx
+install-name:     '/usr/lib/libdyld.dylib'
+current-version:  0001.001.1
+parent-umbrella:  System
+exports:
+  - archs:        [ 'x86_64' ]
+    symbols:      [ dyld_stub_binder ]
+--- !tapi-tbd-v3
+archs:            [ x86_64 ]
+uuids:            [ 'x86_64: 00000000-0000-0000-0000-000000000002' ]
+platform:         macosx
+install-name:     '/usr/lib/libsystem_c.dylib'
+current-version:  0001.001.1
+parent-umbrella:  System
+exports:
+  - archs:        [ 'x86_64' ]
+    symbols:      [ ]
+--- !tapi-tbd-v3
+archs:            [ x86_64 ]
+uuids:            [ 'x86_64: 00000000-0000-0000-0000-000000000003' ]
+platform:         macosx
+install-name:     '/usr/lib/libsystem_m.dylib'
+current-version:  0001.001.1
+parent-umbrella:  System
+exports:
+  - archs:        [ 'x86_64' ]
+    symbols:      [ ___nan ]
+...

lld/test/MachO/iPhoneSimulator.sdk/usr/lib/libSystem.tbd

+--- !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/libSystem.B.dylib'
+current-version:  1281
+exports:
+  - archs:      [ i386, x86_64 ]
+    re-exports: [ '/usr/lib/system/libcache.dylib' ]
+    symbols:    [ __crashreporeter_info__ ]
+--- !tapi-tbd-v3
+archs:            [ i386, x86_64 ]
+uuids:            [ 'i386: 00000000-0000-0000-0000-000000000002', 'x86_64: 00000000-0000-0000-0000-000000000003' ]
+platform:         ios
+install-name:     '/usr/lib/libcache.dylib'
+current-version:  83
+parent-umbrella:  System
+exports:
+  - archs:      [ i386 ]
+    symbols:    [ __cache_handle_memory_pressure_event ]
+  - archs:      [ i386, x86_64 ]
+    symbols:    [ _cache_create, _cache_destroy, _cache_get ]
+...

lld/test/MachO/stub-link-2.s

+# REQUIRES: x86
+
+# RUN: mkdir -p %t
+#
+# RUN: llvm-mc -filetype obj -triple x86_64-apple-ios %s -o %t/test.o
+# RUN: not lld -flavor darwinnew -o %t/test -Z -L%S/iPhoneSimulator.sdk/usr/lib -lSystem %t/test.o 2>&1 | FileCheck %s
+
+# CHECK: error: undefined symbol __cache_handle_memory_pressure_event
+
+.section __TEXT,__text
+.global _main
+
+_main:
+  movq __cache_handle_memory_pressure_event@GOTPCREL(%rip), %rax
+  ret

lld/test/MachO/stub-link.s

+# REQUIRES: x86
+
+# RUN: mkdir -p %t
+#
+# RUN: llvm-mc -filetype obj -triple x86_64-apple-darwin %s -o %t/test.o
+# RUN: lld -flavor darwinnew -o %t/test -Z -L%S/MacOSX.sdk/usr/lib -lSystem %t/test.o
+#
+# RUN: llvm-objdump --bind --no-show-raw-insn -d -r %t/test | FileCheck %s
+
+# CHECK: Disassembly of section __TEXT,__text:
+# CHECK: movq {{.*}} # [[ADDR:[0-9a-f]+]]
+
+# CHECK: Bind table:
+# CHECK: __DATA_CONST __got 0x[[ADDR]] pointer 0 libSystem ___nan
+
+.section __TEXT,__text
+.global _main
+
+_main:
+  movq ___nan@GOTPCREL(%rip), %rax
+  ret