[lld-macho] Postprocess LC Linker Option

LLD resolves symbols regardless of LTO modes early when reading and parsing input files in order. The object files built from LTO passes are appended later.
Because LLD eagerly resolves the LC linker options while parsing a new object file (and its chain of dependent libraries), the prior decision on pending prevailing symbols (belonging to some bitcode files) can change to ones in those native libraries that are just loaded.

This patch delays processing LC linker options until all the native object files are added after LTO is done, similar to LD64. This way we preserve the decision on prevailing symbols LLD made, regardless of LTO modes.
 - When parsing a new object file in `parseLinkerOptions()`, it just parses LC linker options in the header, and saves those contents to `unprocessedLCLinkerOptions`.
 - After LTO is finished, `resolveLCLinkerOptions()` is called to recursively load dependent libraries, starting with initial linker options collected in `unprocessedLCLinkerOptions` (which also updates during recursions)

Reviewed By: #lld-macho, int3

Differential Revision: https://reviews.llvm.org/D157716

Author: Kyungwoo Lee

lld/MachO/Driver.cpp

@@ -411,7 +411,7 @@ static InputFile *addFile(StringRef path, LoadType loadType,
 static std::vector<StringRef> missingAutolinkWarnings;
 static void addLibrary(StringRef name, bool isNeeded, bool isWeak,
                        bool isReexport, bool isHidden, bool isExplicit,
-                       LoadType loadType, InputFile *originFile = nullptr) {
+                       LoadType loadType) {
   if (std::optional<StringRef> path = findLibrary(name)) {
     if (auto *dylibFile = dyn_cast_or_null<DylibFile>(
             addFile(*path, loadType, /*isLazy=*/false, isExplicit,
@@ -428,19 +428,16 @@ static void addLibrary(StringRef name, bool isNeeded, bool isWeak,
     return;
   }
   if (loadType == LoadType::LCLinkerOption) {
-    assert(originFile);
     missingAutolinkWarnings.push_back(
-        saver().save(toString(originFile) +
-                     ": auto-linked library not found for -l" + name));
+        saver().save("auto-linked library not found for -l" + name));
     return;
   }
   error("library not found for -l" + name);
 }
 
 static DenseSet<StringRef> loadedObjectFrameworks;
 static void addFramework(StringRef name, bool isNeeded, bool isWeak,
-                         bool isReexport, bool isExplicit, LoadType loadType,
-                         InputFile *originFile = nullptr) {
+                         bool isReexport, bool isExplicit, LoadType loadType) {
   if (std::optional<StringRef> path = findFramework(name)) {
     if (loadedObjectFrameworks.contains(*path))
       return;
@@ -468,10 +465,8 @@ static void addFramework(StringRef name, bool isNeeded, bool isWeak,
     return;
   }
   if (loadType == LoadType::LCLinkerOption) {
-    assert(originFile);
-    missingAutolinkWarnings.push_back(saver().save(
-        toString(originFile) +
-        ": auto-linked framework not found for -framework " + name));
+    missingAutolinkWarnings.push_back(
+        saver().save("auto-linked framework not found for -framework " + name));
     return;
   }
   error("framework not found for -framework " + name);
@@ -480,7 +475,9 @@ static void addFramework(StringRef name, bool isNeeded, bool isWeak,
 // Parses LC_LINKER_OPTION contents, which can add additional command line
 // flags. This directly parses the flags instead of using the standard argument
 // parser to improve performance.
-void macho::parseLCLinkerOption(InputFile *f, unsigned argc, StringRef data) {
+void macho::parseLCLinkerOption(
+    llvm::SmallVectorImpl<StringRef> &LCLinkerOptions, InputFile *f,
+    unsigned argc, StringRef data) {
   if (config->ignoreAutoLink)
     return;
 
@@ -498,19 +495,42 @@ void macho::parseLCLinkerOption(InputFile *f, unsigned argc, StringRef data) {
   if (arg.consume_front("-l")) {
     if (config->ignoreAutoLinkOptions.contains(arg))
       return;
-    addLibrary(arg, /*isNeeded=*/false, /*isWeak=*/false,
-               /*isReexport=*/false, /*isHidden=*/false, /*isExplicit=*/false,
-               LoadType::LCLinkerOption, f);
   } else if (arg == "-framework") {
     StringRef name = argv[++i];
     if (config->ignoreAutoLinkOptions.contains(name))
       return;
-    addFramework(name, /*isNeeded=*/false, /*isWeak=*/false,
-                 /*isReexport=*/false, /*isExplicit=*/false,
-                 LoadType::LCLinkerOption, f);
   } else {
     error(arg + " is not allowed in LC_LINKER_OPTION");
   }
+
+  LCLinkerOptions.append(argv);
+}
+
+void macho::resolveLCLinkerOptions() {
+  while (!unprocessedLCLinkerOptions.empty()) {
+    SmallVector<StringRef> LCLinkerOptions(unprocessedLCLinkerOptions);
+    unprocessedLCLinkerOptions.clear();
+
+    for (unsigned i = 0; i < LCLinkerOptions.size(); ++i) {
+      StringRef arg = LCLinkerOptions[i];
+      if (arg.consume_front("-l")) {
+        if (config->ignoreAutoLinkOptions.contains(arg))
+          continue;
+        addLibrary(arg, /*isNeeded=*/false, /*isWeak=*/false,
+                   /*isReexport=*/false, /*isHidden=*/false,
+                   /*isExplicit=*/false, LoadType::LCLinkerOption);
+      } else if (arg == "-framework") {
+        StringRef name = LCLinkerOptions[++i];
+        if (config->ignoreAutoLinkOptions.contains(name))
+          continue;
+        addFramework(name, /*isNeeded=*/false, /*isWeak=*/false,
+                     /*isReexport=*/false, /*isExplicit=*/false,
+                     LoadType::LCLinkerOption);
+      } else {
+        error(arg + " is not allowed in LC_LINKER_OPTION");
+      }
+    }
+  }
 }
 
 static void addFileList(StringRef path, bool isLazy) {
@@ -1387,6 +1407,7 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
     missingAutolinkWarnings.clear();
     syntheticSections.clear();
     thunkMap.clear();
+    unprocessedLCLinkerOptions.clear();
 
     firstTLVDataSection = nullptr;
     tar = nullptr;
@@ -1889,6 +1910,8 @@ bool link(ArrayRef<const char *> argsArr, llvm::raw_ostream &stdoutOS,
 
     bool didCompileBitcodeFiles = compileBitcodeFiles();
 
+    resolveLCLinkerOptions();
+
     // If --thinlto-index-only is given, we should create only "index
     // files" and not object files. Index file creation is already done
     // in compileBitcodeFiles, so we are done if that's the case.

lld/MachO/Driver.h

@@ -40,7 +40,9 @@ enum {
 #undef OPTION
 };
 
-void parseLCLinkerOption(InputFile *, unsigned argc, StringRef data);
+void parseLCLinkerOption(llvm::SmallVectorImpl<StringRef> &LCLinkerOptions,
+                         InputFile *f, unsigned argc, StringRef data);
+void resolveLCLinkerOptions();
 
 std::string createResponseFile(const llvm::opt::InputArgList &args);
 

lld/MachO/InputFiles.cpp

@@ -950,6 +950,19 @@ OpaqueFile::OpaqueFile(MemoryBufferRef mb, StringRef segName,
   section.subsections.push_back({0, isec});
 }
 
+template <class LP>
+void ObjFile::parseLinkerOptions(SmallVectorImpl<StringRef> &LCLinkerOptions) {
+  using Header = typename LP::mach_header;
+  auto *hdr = reinterpret_cast<const Header *>(mb.getBufferStart());
+
+  for (auto *cmd : findCommands<linker_option_command>(hdr, LC_LINKER_OPTION)) {
+    StringRef data{reinterpret_cast<const char *>(cmd + 1),
+                   cmd->cmdsize - sizeof(linker_option_command)};
+    parseLCLinkerOption(LCLinkerOptions, this, cmd->count, data);
+  }
+}
+
+SmallVector<StringRef> macho::unprocessedLCLinkerOptions;
 ObjFile::ObjFile(MemoryBufferRef mb, uint32_t modTime, StringRef archiveName,
                  bool lazy, bool forceHidden, bool compatArch)
     : InputFile(ObjKind, mb, lazy), modTime(modTime), forceHidden(forceHidden) {
@@ -983,11 +996,11 @@ template <class LP> void ObjFile::parse() {
   if (!(compatArch = compatWithTargetArch(this, hdr)))
     return;
 
-  for (auto *cmd : findCommands<linker_option_command>(hdr, LC_LINKER_OPTION)) {
-    StringRef data{reinterpret_cast<const char *>(cmd + 1),
-                   cmd->cmdsize - sizeof(linker_option_command)};
-    parseLCLinkerOption(this, cmd->count, data);
-  }
+  // We will resolve LC linker options once all native objects are loaded after
+  // LTO is finished.
+  SmallVector<StringRef, 4> LCLinkerOptions;
+  parseLinkerOptions<LP>(LCLinkerOptions);
+  unprocessedLCLinkerOptions.append(LCLinkerOptions);
 
   ArrayRef<SectionHeader> sectionHeaders;
   if (const load_command *cmd = findCommand(hdr, LP::segmentLCType)) {

lld/MachO/InputFiles.h

@@ -164,6 +164,8 @@ class ObjFile final : public InputFile {
   ArrayRef<llvm::MachO::data_in_code_entry> getDataInCode() const;
   ArrayRef<uint8_t> getOptimizationHints() const;
   template <class LP> void parse();
+  template <class LP>
+  void parseLinkerOptions(llvm::SmallVectorImpl<StringRef> &LinkerOptions);
 
   static bool classof(const InputFile *f) { return f->kind() == ObjKind; }
 
@@ -317,6 +319,7 @@ class BitcodeFile final : public InputFile {
 
 extern llvm::SetVector<InputFile *> inputFiles;
 extern llvm::DenseMap<llvm::CachedHashStringRef, MemoryBufferRef> cachedReads;
+extern llvm::SmallVector<StringRef> unprocessedLCLinkerOptions;
 
 std::optional<MemoryBufferRef> readFile(StringRef path);
 

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

@@ -0,0 +1,83 @@
+; REQUIRES: x86
+; RUN: rm -rf %t; split-file %s %t
+
+; RUN: llc -filetype=obj %t/q.ll -o %t/q.o
+; RUN: llvm-ar cru %t/libq.a %t/q.o
+
+; RUN: llc -filetype=obj %t/f.ll -o %t/f.nolto.o
+; RUN: opt --thinlto-bc %t/f.ll -o %t/f.thinlto.o
+; RUN: opt %t/f.ll -o %t/f.lto.o
+
+; RUN: llc -filetype=obj %t/b.ll -o %t/b.nolto.o
+; RUN: opt --thinlto-bc %t/b.ll -o %t/b.thinlto.o
+; RUN: opt %t/b.ll -o %t/b.lto.o
+
+; (1) NoLTO-NoLTO
+; RUN: %lld -dylib -lSystem -L%t %t/f.nolto.o %t/b.nolto.o -o %t/nolto-nolto.out
+; RUN: llvm-objdump --syms %t/nolto-nolto.out | FileCheck %s
+
+; (2) NoLTO-ThinLTO
+; RUN: %lld -dylib -lSystem -L%t %t/f.nolto.o %t/b.thinlto.o -o %t/nolto-thinlto.out
+; RUN: llvm-objdump --syms %t/nolto-thinlto.out | FileCheck %s
+
+; (3) ThinLTO-NoLTO
+; RUN: %lld -dylib -lSystem -L%t %t/f.thinlto.o %t/b.nolto.o -o %t/thinlto-nolto.out
+; RUN: llvm-objdump --syms %t/thinlto-nolto.out | FileCheck %s
+
+; (4) NoLTO-LTO
+; RUN: %lld -dylib -lSystem -L%t %t/f.nolto.o %t/b.lto.o -o %t/nolto-lto.out
+; RUN: llvm-objdump --syms %t/nolto-lto.out | FileCheck %s
+
+; (5) LTO-NoLTO
+; RUN: %lld -dylib -lSystem -L%t %t/f.lto.o %t/b.nolto.o -o %t/lto-nolto.out
+; RUN: llvm-objdump --syms %t/lto-nolto.out | FileCheck %s
+
+; (6) LTO-ThinLTO
+; RUN: %lld -dylib -lSystem -L%t %t/f.lto.o %t/b.thinlto.o -o %t/lto-thinlto.out
+; RUN: llvm-objdump --syms %t/lto-thinlto.out | FileCheck %s
+
+; (7) ThinLTO-NoLTO
+; RUN: %lld -dylib -lSystem -L%t %t/f.thinlto.o %t/b.lto.o -o %t/thinlto-lto.out
+; RUN: llvm-objdump --syms %t/thinlto-lto.out | FileCheck %s
+
+; We expect to resolve _weak1 from f.ll and _weak2 from b.ll as per the input order.
+; As _weak2 from q.ll pulled in via LC_LINKER_OPTION is processed
+; in the second pass, it won't prevail due to _weak2 from b.ll.
+
+; CHECK:          w    O __TEXT,f _weak1
+; CHECK:          w    O __TEXT,b _weak2
+
+;--- q.ll
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.15.0"
+
+define i32 @weak2() section "__TEXT,q" {
+  ret i32 2
+}
+
+;--- f.ll
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.15.0"
+
+!0 = !{!"-lq"}
+!llvm.linker.options = !{!0}
+
+define weak i32 @weak1() section "__TEXT,f" {
+  %call = call i32 @weak2()
+  %add = add nsw i32 %call, 1
+  ret i32 %add
+}
+
+declare i32 @weak2(...)
+
+;--- b.ll
+target datalayout = "e-m:o-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.15.0"
+
+define weak i32 @weak1() section "__TEXT,b" {
+  ret i32 3
+}
+
+define weak i32 @weak2() section "__TEXT,b" {
+  ret i32 4
+}