[mac/lld] Add support for the LC_LINKER_OPTION load command in o files

[thakis]

Summary

clang puts -framework CoreFoundation in this load command for files
that use @available / __builtin_available. Without support for this,
binaries that don't explicitly link to CoreFoundation fail to link.

[thakis]

rename parseLoadCommand -> parseLCLinkerOption

[thakis]

(clang ref: http://llvm-cs.pcc.me.uk/tools/clang/lib/CodeGen/CGObjC.cpp#3714)

[int3]

lld/MachO/Driver.cpp

355

would be good to have a short comment about what LC_LINKER_OPTION contains

370–371

is this necessary given that we are going to error out anyway at line 382 below?

lld/test/MachO/lc-linker-option.ll

22

nit: the other tests generally include the "error: " prefix

[thakis]

comments

[thakis]

Thanks!

lld/MachO/Driver.cpp

370–371

I guess it's not strictly necessary. It matches the flow in parseDirectives() in lld/COFF. The reasoning is probably that this first stage gives you a list of generally valid args, and the 2nd pass below checks which of them are valid in directives / load commands.

[int3]

lgtm

lld/MachO/Driver.cpp

370–371

I see. How about dropping the "ignoring" and just having "unknown argument:" then? "Ignoring" suggests that we are not erroring out... it looks like parseDirectives in the ELF port issues a warning, and {ELF,MachO}OptTable::parse just errors with "unknown argument".

As an aside... it's a bit unfortunate that "unknown argument" refers to the CLI flag, while "missing argument" refers to the argument to a flag... but this overload of "argument" seems to happen in many places in the LLVM codebase already so ¯\_(ツ)_/¯

[thakis]

lld/MachO/Driver.cpp

370–371

Sounds good :)

lld/MachO/Driver.h

 // Create enum with OPT_xxx values for each option in Options.td
 enum {
   OPT_INVALID = 0,
 #define OPTION(_1, _2, ID, _4, _5, _6, _7, _8, _9, _10, _11, _12) OPT_##ID,
 #include "Options.inc"
 #undef OPTION
 };
 
+void parseLCLinkerOption(InputFile*, unsigned argc, StringRef data);
+
 std::string createResponseFile(const llvm::opt::InputArgList &args);
 
 // Check for both libfoo.dylib and libfoo.tbd (in that order).
 llvm::Optional<std::string> resolveDylibPath(llvm::StringRef path);
 
 llvm::Optional<DylibFile *> makeDylibFromTAPI(llvm::MemoryBufferRef mbref,
                                               DylibFile *umbrella = nullptr);
 
 uint32_t getModTime(llvm::StringRef path);
 
 void printArchiveMemberLoad(StringRef reason, const InputFile *);
 
 } // namespace macho
 } // namespace lld
 
 #endif

lld/MachO/Driver.cpp

if (newFile) {
     // print the .a name here.
     if (config->printEachFile && magic != file_magic::archive)
       lld::outs() << toString(newFile) << '\n';
     inputFiles.push_back(newFile);
   }
   return newFile;
 }
 
+static void addLibrary(StringRef name, bool isWeak) {
+  if (Optional<std::string> path = findLibrary(name)) {
+    auto *dylibFile = dyn_cast_or_null<DylibFile>(addFile(*path, false));
+    if (isWeak && dylibFile)
+      dylibFile->forceWeakImport = true;
+    return;
+  }
+  error("library not found for -l" + name);
+}
+
+static void addFramework(StringRef name, bool isWeak) {
+  if (Optional<std::string> path = findFramework(name)) {
+    auto *dylibFile = dyn_cast_or_null<DylibFile>(addFile(*path, false));
+    if (isWeak && dylibFile)
+      dylibFile->forceWeakImport = true;
+    return;
+  }
+  error("framework not found for -framework " + name);
+}
+
+// Parses LC_LINKER_OPTION contents, which can add additional command line flags.
+void macho::parseLCLinkerOption(InputFile* f, unsigned argc, StringRef data) {
+  SmallVector<const char *, 4> argv;
+  size_t offset = 0;
+  for (unsigned i = 0; i < argc && offset < data.size(); ++i) {
+    argv.push_back(data.data() + offset);
+    offset += strlen(data.data() + offset) + 1;
+  }
+  if (argv.size() != argc || offset > data.size())
+    fatal(toString(f) + ": invalid LC_LINKER_OPTION");
+
+  MachOOptTable table;
+  unsigned missingIndex, missingCount;
+  opt::InputArgList args = table.ParseArgs(argv, missingIndex, missingCount);
+  if (missingCount)
+    fatal(Twine(args.getArgString(missingIndex)) + ": missing argument");
+  for (auto *arg : args.filtered(OPT_UNKNOWN))
+    error("ignoring unknown argument: " + arg->getAsString(args));
+
+  for (auto *arg : args) {
+    switch (arg->getOption().getID()) {
+    case OPT_l:
+      addLibrary(arg->getValue(), false);
+      break;
+    case OPT_framework:
+      addFramework(arg->getValue(), false);
+      break;
+    default:
+      error(arg->getSpelling() + " is not allowed in LC_LINKER_OPTION");
+    }
+  }
+}
+
 static void addFileList(StringRef path) {
   Optional<MemoryBufferRef> buffer = readFile(path);
   if (!buffer)
     return;
   MemoryBufferRef mbref = *buffer;
   for (StringRef path : args::getLines(mbref))
     addFile(path, false);
 }
for (const auto &arg : args) {
     }
     case OPT_filelist:
       addFileList(arg->getValue());
       break;
     case OPT_force_load:
       addFile(arg->getValue(), true);
       break;
     case OPT_l:
-    case OPT_weak_l: {
-      StringRef name = arg->getValue();
-      if (Optional<std::string> path = findLibrary(name)) {
-        auto *dylibFile = dyn_cast_or_null<DylibFile>(addFile(*path, false));
-        if (opt.getID() == OPT_weak_l && dylibFile)
-          dylibFile->forceWeakImport = true;
-        break;
-      }
-      error("library not found for -l" + name);
+    case OPT_weak_l:
+      addLibrary(arg->getValue(), opt.getID() == OPT_weak_l);
       break;
-    }
     case OPT_framework:
-    case OPT_weak_framework: {
-      StringRef name = arg->getValue();
-      if (Optional<std::string> path = findFramework(name)) {
-        auto *dylibFile = dyn_cast_or_null<DylibFile>(addFile(*path, false));
-        if (opt.getID() == OPT_weak_framework && dylibFile)
-          dylibFile->forceWeakImport = true;
-        break;
-      }
-      error("framework not found for -framework " + name);
-      break;
-    }
+    case OPT_weak_framework:
+      addFramework(arg->getValue(), opt.getID() == OPT_weak_framework);
     case OPT_platform_version:
       handlePlatformVersion(arg);
       break;
     default:
       break;
     }
   }
 

lld/MachO/InputFiles.cpp

 
 ObjFile::ObjFile(MemoryBufferRef mb, uint32_t modTime, StringRef archiveName)
     : InputFile(ObjKind, mb), modTime(modTime) {
   this->archiveName = std::string(archiveName);
 
   auto *buf = reinterpret_cast<const uint8_t *>(mb.getBufferStart());
   auto *hdr = reinterpret_cast<const mach_header_64 *>(mb.getBufferStart());
 
+  if (const load_command *cmd = findCommand(hdr, LC_LINKER_OPTION)) {
+    auto *c = reinterpret_cast<const linker_option_command *>(cmd);
+    StringRef data{reinterpret_cast<const char *>(c + 1),
+                   c->cmdsize - sizeof(linker_option_command)};
+    parseLCLinkerOption(this, c->count, data);
+  }
+
   if (const load_command *cmd = findCommand(hdr, LC_SEGMENT_64)) {
     auto *c = reinterpret_cast<const segment_command_64 *>(cmd);
     sectionHeaders = ArrayRef<section_64>{
         reinterpret_cast<const section_64 *>(c + 1), c->nsects};
     parseSections(sectionHeaders);
   }
 
   // TODO: Error on missing LC_SYMTAB?

lld/test/MachO/lc-linker-option.ll

+# REQUIRES: x86
+# RUN: rm -rf %t
+# RUN: split-file %s %t
+
+# RUN: llvm-as %t/framework.ll -o %t/framework.o
+# RUN: %lld %t/framework.o -o %t/frame
+# RUN: llvm-objdump --macho --all-headers %t/frame | FileCheck --check-prefix=FRAME %s
+# FRAME:          cmd LC_LOAD_DYLIB
+# FRAME-NEXT: cmdsize
+# FRAME-NEXT:    name /System/Library/Frameworks/CoreFoundation.framework/CoreFoundation
+
+# RUN: llvm-as %t/l.ll -o %t/l.o
+# RUN: %lld %t/l.o -o %t/l
+# RUN: llvm-objdump --macho --all-headers %t/l | FileCheck --check-prefix=LIB %s
+# LIB:          cmd LC_LOAD_DYLIB
+# LIB-NEXT: cmdsize
+# LIB-NEXT:    name /usr/lib/libSystem.B.dylib
+
+# RUN: llvm-as %t/invalid.ll -o %t/invalid.o
+# RUN: not %lld %t/invalid.o -o /dev/null 2>&1 | FileCheck --check-prefix=INVALID %s
+# INVALID: error: -why_load is not allowed in LC_LINKER_OPTION
+
+#--- framework.ll
+target triple = "x86_64-apple-macosx10.15.0"
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+
+!0 = !{!"-framework", !"CoreFoundation"}
+!llvm.linker.options = !{!0}
+
+define void @main() {
+  ret void
+}
+
+#--- l.ll
+target triple = "x86_64-apple-macosx10.15.0"
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+
+!0 = !{!"-lSystem"}
+!llvm.linker.options = !{!0}
+
+define void @main() {
+  ret void
+}
+
+#--- invalid.ll
+
+target triple = "x86_64-apple-macosx10.15.0"
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+
+!0 = !{!"-why_load"}
+!llvm.linker.options = !{!0}
+
+define void @main() {
+  ret void
+}