[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

603–608

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.

672–676

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

672–676

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

616–623

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

616–623

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

[gkm]

lld/MachO/Driver.cpp

611

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

611

¯\_(ツ)_/¯ 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

if (args.hasArg(OPT_v)) {
     message(StringRef("Framework search paths:") +
             (config->frameworkSearchPaths.size()
                  ? "\n\t" + llvm::join(config->frameworkSearchPaths, "\n\t")
                  : ""));
     freeArena();
     return !errorCount();
   }
 
+  DenseSet<StringRef> weakImportPaths;
+
   for (const auto &arg : args) {
     const auto &opt = arg->getOption();
     warnIfDeprecatedOption(opt);
-    switch (arg->getOption().getID()) {
+    switch (opt.getID()) {
     case OPT_INPUT:
+    case OPT_weak_library:
       addFile(arg->getValue());
+      if (opt.getID() == OPT_weak_library)
+        weakImportPaths.insert(arg->getValue());
       break;
     case OPT_filelist:
       addFileList(arg->getValue());
       break;
     case OPT_force_load:
       forceLoadArchive(arg->getValue());
       break;
     case OPT_weak_l:
-      warn("TODO: -weak_l is currently being treated like -l");
-      LLVM_FALLTHROUGH;
     case OPT_l: {
       StringRef name = arg->getValue();
       if (Optional<std::string> path = findLibrary(name)) {
         addFile(*path);
+        if (opt.getID() == OPT_weak_l)
+          weakImportPaths.insert(saver.save(*path));
         break;
       }
       error("library not found for -l" + name);
       break;
     }
     case OPT_weak_framework:
-      warn("TODO: -weak_framework is currently being treated like -framework");
-      LLVM_FALLTHROUGH;
     case OPT_framework: {
       StringRef name = arg->getValue();
       if (Optional<std::string> path = findFramework(name)) {
         addFile(*path);
+        if (opt.getID() == OPT_weak_framework)
+          weakImportPaths.insert(saver.save(*path));
         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));
 
+  for (InputFile *file : inputFiles)
+    if (auto *dylibFile = dyn_cast<DylibFile>(file))
+      if (weakImportPaths.contains(file->getName()))
+        dylibFile->forceWeakImport = true;
+
   // 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");
   }

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/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