[lld-macho] Support -weak_lx, -weak_library, -weak_framework

[int3]

Summary

They cause their corresponding libraries / frameworks to be loaded via
LC_LOAD_WEAK_DYLIB instead of LC_LOAD_DYLIB.

[int3]

rebase

[gkm]

lld/MachO/Driver.cpp

607–615

IMO, there is no benefit in combining these cases, and a deficit in clarity.
By contrast, the other two OPT_weak_l and OPT_weak_framework do benefit from combining, since they have surrounding context that need not be duplicated.

679–683

How about setting the forceWeakImport flag immediately after construction? Perhaps create void addWeakFile(StringRef path) and/or void addFile(StringRef path, bool isWeak) APIs around void addFile(StringRef path) that sets the flag when addFlag() returns a DylibFile. That way you don't need weakImportPaths.

[gkm]

Be sure to remove Flags<[HelpHidden]> from ldd/MachO/Options.td for these.

[int3]

lld/MachO/Driver.cpp

679–683

I agree this is kind of janky, but I think having an isWeak parameter that only has an effect when the resulting path contains a DylibFile is also kind of ugly...

[int3]

helphidden

[gkm]

lld/MachO/Driver.cpp

622–630

How about something like this ...
Also change addFile() to return its newly-constructed InputFile*

[int3]

have addFile return the new file

lld/MachO/Driver.cpp

622–630

that was cleaner than I'd anticipated it being. thanks!

[gkm]

lld/MachO/Driver.cpp

612

I always find explicit nullptr-inequality checks to be overly noisy. I just looked and found that there are only three instances of != nullptr in all of lld/MachO/*.{h,cpp}, which IMO is at least two too many. The third is semi-legit for highlighting the boolean nature of pointer-to-something vs. pointer-to-nothing:

./UnwindInfoSection.cpp:  return (compactUnwindSection != nullptr);

If you don't feel so strongly about it, then please allow me to impose my will regarding this stylistic nit. If you do feel strongly, then I bet you could find a ton of places to add != nullptr. We could even start an edit war! 😆

[int3]

lld/MachO/Driver.cpp

612

¯\_(ツ)_/¯ I mean, I think != nullptr is in line with "explicit over implicit" (yeah I know this isn't Python, but I like to carry the zen of Python wherever I go)... but it's a minor issue in the scheme of things

lld/MachO/Driver.cpp

static std::vector<MemoryBufferRef> getArchiveMembers(MemoryBufferRef mb) {
   }
   if (err)
     fatal(mb.getBufferIdentifier() +
           ": Archive::children failed: " + toString(std::move(err)));
 
   return v;
 }
 
-static void addFile(StringRef path) {
+static InputFile *addFile(StringRef path) {
   Optional<MemoryBufferRef> buffer = readFile(path);
   if (!buffer)
-    return;
+    return nullptr;
   MemoryBufferRef mbref = *buffer;
+  InputFile *newFile = nullptr;
 
   switch (identify_magic(mbref.getBuffer())) {
   case file_magic::archive: {
     std::unique_ptr<object::Archive> file = CHECK(
         object::Archive::create(mbref), path + ": failed to parse archive");
 
     if (!file->isEmpty() && !file->hasSymbolTable())
       error(path + ": archive has no index; run ranlib to add one");
if (config->allLoad) {
       // consider creating a LazyObjFile class in order to avoid double-loading
       // these files here and below (as part of the ArchiveFile).
       if (Optional<MemoryBufferRef> buffer = readFile(path))
         for (MemoryBufferRef member : getArchiveMembers(*buffer))
           if (hasObjCSection(member))
             inputFiles.push_back(make<ObjFile>(member));
     }
 
-    inputFiles.push_back(make<ArchiveFile>(std::move(file)));
+    newFile = make<ArchiveFile>(std::move(file));
     break;
   }
   case file_magic::macho_object:
-    inputFiles.push_back(make<ObjFile>(mbref));
+    newFile = make<ObjFile>(mbref);
     break;
   case file_magic::macho_dynamically_linked_shared_lib:
-    inputFiles.push_back(make<DylibFile>(mbref));
+    newFile = make<DylibFile>(mbref);
     break;
   case file_magic::tapi_file: {
     Optional<DylibFile *> dylibFile = makeDylibFromTAPI(mbref);
     if (!dylibFile)
-      return;
-    inputFiles.push_back(*dylibFile);
+      return nullptr;
+    newFile = *dylibFile;
     break;
   }
   default:
     error(path + ": unhandled file type");
   }
+  inputFiles.push_back(newFile);
+  return newFile;
 }
 
 static void addFileList(StringRef path) {
   Optional<MemoryBufferRef> buffer = readFile(path);
   if (!buffer)
     return;
   MemoryBufferRef mbref = *buffer;
   for (StringRef path : args::getLines(mbref))
message(StringRef("Framework search paths:") +
                  : ""));
     freeArena();
     return !errorCount();
   }
 
   for (const auto &arg : args) {
     const auto &opt = arg->getOption();
     warnIfDeprecatedOption(opt);
-    switch (arg->getOption().getID()) {
+    switch (opt.getID()) {
     case OPT_INPUT:
       addFile(arg->getValue());
       break;
+    case OPT_weak_library: {
+      auto *dylibFile = dyn_cast_or_null<DylibFile>(addFile(arg->getValue()));
+      if (dylibFile != nullptr)
+        dylibFile->forceWeakImport = true;
+      break;
+    }
     case OPT_filelist:
       addFileList(arg->getValue());
       break;
     case OPT_force_load:
       forceLoadArchive(arg->getValue());
       break;
-    case OPT_l: {
+    case OPT_l:
+    case OPT_weak_l: {
       StringRef name = arg->getValue();
       if (Optional<std::string> path = findLibrary(name)) {
-        addFile(*path);
+        auto *dylibFile = dyn_cast_or_null<DylibFile>(addFile(*path));
+        if (opt.getID() == OPT_weak_l && dylibFile != nullptr)
+          dylibFile->forceWeakImport = true;
         break;
       }
       error("library not found for -l" + name);
       break;
     }
-    case OPT_framework: {
+    case OPT_framework:
+    case OPT_weak_framework: {
       StringRef name = arg->getValue();
       if (Optional<std::string> path = findFramework(name)) {
-        addFile(*path);
+        auto *dylibFile = dyn_cast_or_null<DylibFile>(addFile(*path));
+        if (opt.getID() == OPT_weak_framework && dylibFile != nullptr)
+          dylibFile->forceWeakImport = true;
         break;
       }
       error("framework not found for -framework " + name);
       break;
     }
     case OPT_platform_version:
       handlePlatformVersion(arg);
       break;
default:
       break;
     }
   }
 
   config->isPic = config->outputType == MH_DYLIB ||
                   config->outputType == MH_BUNDLE ||
                   (config->outputType == MH_EXECUTE && args.hasArg(OPT_pie));
 
   // Now that all dylibs have been loaded, search for those that should be
   // re-exported.
   for (opt::Arg *arg : args.filtered(OPT_sub_library)) {
     config->hasReexports = true;
     StringRef searchName = arg->getValue();
     if (!markSubLibrary(searchName))
       error("-sub_library " + searchName + " does not match a supplied dylib");
   }
 
   replaceCommonSymbols();
 
   StringRef orderFile = args.getLastArgValue(OPT_order_file);
   if (!orderFile.empty())

lld/MachO/InputFiles.h

public:
   explicit DylibFile(const llvm::MachO::InterfaceFile &interface,
                      DylibFile *umbrella = nullptr);
 
   static bool classof(const InputFile *f) { return f->kind() == DylibKind; }
 
   StringRef dylibName;
   uint64_t ordinal = 0; // Ordinal numbering starts from 1, so 0 is a sentinel
   bool reexport = false;
+  bool forceWeakImport = false;
   std::vector<DylibFile *> reexported;
 };
 
 // .a file
 class ArchiveFile : public InputFile {
 public:
   explicit ArchiveFile(std::unique_ptr<llvm::object::Archive> &&file);
   static bool classof(const InputFile *f) { return f->kind() == ArchiveKind; }

lld/MachO/Options.td

 
 def l : Joined<["-"], "l">,
      MetaVarName<"<name>">,
      HelpText<"Search for lib<name>.dylib or lib<name>.a on the library search path">,
      Group<grp_libs>;
 def weak_l : Joined<["-"], "weak-l">,
      MetaVarName<"<name>">,
      HelpText<"Like -l<name>, but mark library and its references as weak imports">,
-     Flags<[HelpHidden]>,
      Group<grp_libs>;
 def weak_library : Separate<["-"], "weak_library">,
      MetaVarName<"<path>">,
      HelpText<"Like bare <path>, but mark library and its references as weak imports">,
-     Flags<[HelpHidden]>,
      Group<grp_libs>;
 def reexport_l : Joined<["-"], "reexport-l">,
      MetaVarName<"<name>">,
      HelpText<"Like -l<name>, but export all symbols of <name> from newly created library">,
      Flags<[HelpHidden]>,
      Group<grp_libs>;
 def reexport_library : Separate<["-"], "reexport_library">,
      MetaVarName<"<path>">,

 def framework : Separate<["-"], "framework">,
      MetaVarName<"<name>">,
      HelpText<"Search for <name>.framework/<name> on the framework search path">,
      Flags<[HelpHidden]>,
      Group<grp_libs>;
 def weak_framework : Separate<["-"], "weak_framework">,
      MetaVarName<"<name>">,
      HelpText<"Like -framework <name>, but mark framework and its references as weak imports">,
-     Flags<[HelpHidden]>,
      Group<grp_libs>;
 def reexport_framework : Separate<["-"], "reexport_framework">,
      MetaVarName<"<name>">,
      HelpText<"Like -framework <name>, but export all symbols of <name> from the newly created library">,
      Flags<[HelpHidden]>,
      Group<grp_libs>;
 def upward_framework : Separate<["-"], "upward_framework">,
      MetaVarName<"<name>">,

lld/MachO/Writer.cpp

void Writer::createLoadCommands() {
   for (OutputSegment *seg : outputSegments) {
     in.header->addLoadCommand(make<LCSegment>(seg->name, seg));
     seg->index = segIndex++;
   }
 
   uint64_t dylibOrdinal = 1;
   for (InputFile *file : inputFiles) {
     if (auto *dylibFile = dyn_cast<DylibFile>(file)) {
-      in.header->addLoadCommand(
-          make<LCDylib>(LC_LOAD_DYLIB, dylibFile->dylibName));
+      // TODO: dylibs that are only referenced by weak refs should also be
+      // loaded via LC_LOAD_WEAK_DYLIB.
+      LoadCommandType lcType =
+          dylibFile->forceWeakImport ? LC_LOAD_WEAK_DYLIB : LC_LOAD_DYLIB;
+      in.header->addLoadCommand(make<LCDylib>(lcType, dylibFile->dylibName));
       dylibFile->ordinal = dylibOrdinal++;
 
       if (dylibFile->reexport)
         in.header->addLoadCommand(
             make<LCDylib>(LC_REEXPORT_DYLIB, dylibFile->dylibName));
     }
   }
 }

lld/test/MachO/weak-import.s

+# REQUIRES: x86
+# RUN: split-file %s %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/test.s -o %t/test.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %t/foo.s -o %t/foo.o
+# RUN: lld -flavor darwinnew -syslibroot %S/Inputs/MacOSX.sdk -lSystem -dylib %t/foo.o -o %t/libfoo.dylib
+
+# RUN: lld -flavor darwinnew -syslibroot %S/Inputs/MacOSX.sdk -weak-lSystem %t/test.o -weak_framework CoreFoundation -weak_library %t/libfoo.dylib -o %t/test
+# RUN: llvm-objdump --macho --all-headers %t/test | FileCheck %s -DDIR=%t
+
+# CHECK:          cmd LC_LOAD_WEAK_DYLIB
+# CHECK-NEXT: cmdsize
+# CHECK-NEXT:    name /usr/lib/libSystem.B.dylib
+
+# CHECK:          cmd LC_LOAD_WEAK_DYLIB
+# CHECK-NEXT: cmdsize
+# CHECK-NEXT:    name /System/Library/Frameworks/CoreFoundation.framework/CoreFoundation
+
+# CHECK:          cmd LC_LOAD_WEAK_DYLIB
+# CHECK-NEXT: cmdsize
+# CHECK-NEXT:    name [[DIR]]/libfoo.dylib
+
+#--- foo.s
+.globl _foo
+_foo:
+  ret
+
+#--- test.s
+.globl _main
+.text
+_main:
+  ret