[lld/mac] Don't strip explicit dylib also mentioned in LC_LINKER_OPTION

[thakis]

Summary

Noticed by Jez in D103499.

[int3]

Thanks!

[int3]

lld/MachO/Driver.cpp

238

lld/MachO/Driver.cpp

static std::vector<ArchiveMember> getArchiveMembers(MemoryBufferRef mb) {
   if (err)
     fatal(mb.getBufferIdentifier() +
           ": Archive::children failed: " + toString(std::move(err)));
 
   return v;
 }
 
 static InputFile *addFile(StringRef path, bool forceLoadArchive,
+                          bool isExplicit = true,
                           bool isBundleLoader = false) {
   Optional<MemoryBufferRef> buffer = readFile(path);
   if (!buffer)
     return nullptr;
   MemoryBufferRef mbref = *buffer;
   InputFile *newFile = nullptr;
 
   file_magic magic = identify_magic(mbref.getBuffer());
static InputFile *addFile(StringRef path, bool forceLoadArchive,
   }
   case file_magic::macho_object:
     newFile = make<ObjFile>(mbref, getModTime(path), "");
     break;
   case file_magic::macho_dynamically_linked_shared_lib:
   case file_magic::macho_dynamically_linked_shared_lib_stub:
   case file_magic::tapi_file:
     if (DylibFile * dylibFile = loadDylib(mbref)) {
+      if (isExplicit)
         dylibFile->explicitlyLinked = true;
       newFile = dylibFile;
     }
     break;
   case file_magic::bitcode:
     newFile = make<BitcodeFile>(mbref);
     break;
   case file_magic::macho_executable:
   case file_magic::macho_bundle:
if (newFile) {
     inputFiles.insert(newFile);
   }
   return newFile;
 }
 
 static void addLibrary(StringRef name, bool isWeak, bool isReexport,
                        bool isExplicit) {
   if (Optional<StringRef> path = findLibrary(name)) {
-    if (auto *dylibFile = dyn_cast_or_null<DylibFile>(addFile(*path, false))) {
-      dylibFile->explicitlyLinked = isExplicit;
+    if (auto *dylibFile = dyn_cast_or_null<DylibFile>(
+            addFile(*path, /*forceLoadArchive=*/false, isExplicit))) {
       if (isWeak)
         dylibFile->forceWeakImport = true;
       if (isReexport) {
         config->hasReexports = true;
         dylibFile->reexport = true;
       }
     }
     return;
   }
   error("library not found for -l" + name);
 }
 
 static void addFramework(StringRef name, bool isWeak, bool isReexport,
                          bool isExplicit) {
   if (Optional<std::string> path = findFramework(name)) {
-    if (auto *dylibFile = dyn_cast_or_null<DylibFile>(addFile(*path, false))) {
-      dylibFile->explicitlyLinked = isExplicit;
+    if (auto *dylibFile = dyn_cast_or_null<DylibFile>(
+            addFile(*path, /*forceLoadArchive=*/false, isExplicit))) {
       if (isWeak)
         dylibFile->forceWeakImport = true;
       if (isReexport) {
         config->hasReexports = true;
         dylibFile->reexport = true;
       }
     }
     return;

 }
 
 static void addFileList(StringRef path) {
   Optional<MemoryBufferRef> buffer = readFile(path);
   if (!buffer)
     return;
   MemoryBufferRef mbref = *buffer;
   for (StringRef path : args::getLines(mbref))
-    addFile(rerootPath(path), false);
+    addFile(rerootPath(path), /*forceLoadArchive=*/false);
 }
 
 // An order file has one entry per line, in the following format:
 //
 //   <cpu>:<object file>:<symbol name>
 //
 // <cpu> and <object file> are optional. If not specified, then that entry
 // matches any symbol of that name. Parsing this format is not quite
void createFiles(const InputArgList &args) {
   // Other options should be handled via filtered() and/or getLastArg().
   for (const Arg *arg : args) {
     const Option &opt = arg->getOption();
     warnIfDeprecatedOption(opt);
     warnIfUnimplementedOption(opt);
 
     switch (opt.getID()) {
     case OPT_INPUT:
-      addFile(rerootPath(arg->getValue()), false);
+      addFile(rerootPath(arg->getValue()), /*forceLoadArchive=*/false);
       break;
     case OPT_reexport_library:
-      if (auto *dylibFile = dyn_cast_or_null<DylibFile>(
-              addFile(rerootPath(arg->getValue()), false))) {
+      if (auto *dylibFile = dyn_cast_or_null<DylibFile>(addFile(
+              rerootPath(arg->getValue()), /*forceLoadArchive=*/false))) {
         config->hasReexports = true;
         dylibFile->reexport = true;
       }
       break;
     case OPT_weak_library:
       if (auto *dylibFile = dyn_cast_or_null<DylibFile>(
-              addFile(rerootPath(arg->getValue()), false)))
+              addFile(rerootPath(arg->getValue()), /*forceLoadArchive=*/false)))
         dylibFile->forceWeakImport = true;
       break;
     case OPT_filelist:
       addFileList(arg->getValue());
       break;
     case OPT_force_load:
-      addFile(rerootPath(arg->getValue()), true);
+      addFile(rerootPath(arg->getValue()), /*forceLoadArchive=*/true);
       break;
     case OPT_l:
     case OPT_reexport_l:
     case OPT_weak_l:
       addLibrary(arg->getValue(), opt.getID() == OPT_weak_l,
                  opt.getID() == OPT_reexport_l, /*isExplicit=*/true);
       break;
     case OPT_framework:
bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
   config->headerPadMaxInstallNames =
       args.hasArg(OPT_headerpad_max_install_names);
   config->printEachFile = args.hasArg(OPT_t);
   config->printWhyLoad = args.hasArg(OPT_why_load);
   config->outputType = getOutputType(args);
   if (const Arg *arg = args.getLastArg(OPT_bundle_loader)) {
     if (config->outputType != MH_BUNDLE)
       error("-bundle_loader can only be used with MachO bundle output");
-    addFile(arg->getValue(), false, true);
+    addFile(arg->getValue(), /*forceLoadArchive=*/false, /*isExplicit=*/false,
+            /*isBundleLoader=*/true);
   }
   config->ltoObjPath = args.getLastArgValue(OPT_object_path_lto);
   config->ltoNewPassManager =
       args.hasFlag(OPT_no_lto_legacy_pass_manager, OPT_lto_legacy_pass_manager,
                    LLVM_ENABLE_NEW_PASS_MANAGER);
   config->runtimePaths = args::getStrings(args, OPT_rpath);
   config->allLoad = args.hasArg(OPT_all_load);
   config->forceLoadObjC = args.hasArg(OPT_ObjC);

lld/test/MachO/dead-strip-dylibs.s

 # NOBARSTRIP: /usr/lib/libSystem.dylib
 # NOBARSTRIP-NOT: bar-strip.dylib
 # NOBARSTRIP: foo.dylib
 # NOBARSTRIP-NOT: bar-strip.dylib
 
 ## LC_LINKER_OPTION does not count as an explicit reference.
 # RUN: llvm-mc %t/linkopt_bar.s -triple=x86_64-apple-macos -filetype=obj -o %t/linkopt_bar.o
 # RUN: %lld -dylib %t/bar.o -o %t/libbar.dylib
-# RUN: %lld -lSystem %t/main.o %t/linkopt_bar.o -o %t/main -L %t %t/foo_with_bar.dylib
-# RUN: llvm-otool -L %t/main | FileCheck --check-prefix=NOBAR %s
+# RUN: %lld -lSystem %t/main.o %t/linkopt_bar.o -o %t/main -L %t %t/foo.dylib
+# RUN: llvm-otool -L %t/main | FileCheck --check-prefix=NOLIBBAR %s
+# NOLIBBAR-NOT: libbar.dylib
+# NOLIBBAR: /usr/lib/libSystem.dylib
+# NOLIBBAR-NOT: libbar.dylib
+# NOLIBBAR: foo.dylib
+# NOLIBBAR-NOT: libbar.dylib
+
+## ...but with an additional explicit reference it's not stripped again.
+# RUN: %lld -lSystem %t/main.o %t/linkopt_bar.o -o %t/main -L %t %t/foo.dylib -lbar
+# RUN: llvm-otool -L %t/main | FileCheck --check-prefix=LIBBAR %s
+# RUN: %lld -lSystem %t/main.o -o %t/main -L %t %t/foo.dylib -lbar %t/linkopt_bar.o
+# RUN: llvm-otool -L %t/main | FileCheck --check-prefix=LIBBAR %s
+# LIBBAR-DAG: /usr/lib/libSystem.dylib
+# LIBBAR-DAG: libbar.dylib
+# LIBBAR-DAG: foo.dylib
 
 ## Test that a DylibSymbol being replaced by a DefinedSymbol marks the
 ## dylib as unreferenced.
 ## (Note: Since there's no dynamic linking in this example, libSystem is
 ## stripped too. Since libSystem.dylib is needed to run an executable for
 ## LC_MAIN, the output would crash when run. This matches ld64's behavior (!)
 ## In practice, every executable uses dynamic linking, which uses
 ## dyld_stub_binder, which keeps libSystem alive.)