[lld] 484c961 - [lld-macho] Postprocess LC Linker Option

Kyungwoo Lee via llvm-commits llvm-commits at lists.llvm.org
Sun Aug 13 13:39:18 PDT 2023


Author: Kyungwoo Lee
Date: 2023-08-13T13:39:04-07:00
New Revision: 484c961ccdfaf1d72429560a08b2d44da9a292cd

URL: https://github.com/llvm/llvm-project/commit/484c961ccdfaf1d72429560a08b2d44da9a292cd
DIFF: https://github.com/llvm/llvm-project/commit/484c961ccdfaf1d72429560a08b2d44da9a292cd.diff

LOG: [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

Added: 
    lld/test/MachO/lc-linker-option-lto.ll
    lld/test/MachO/lc-linker-option-order.ll

Modified: 
    lld/MachO/Driver.cpp
    lld/MachO/Driver.h
    lld/MachO/InputFiles.cpp
    lld/MachO/InputFiles.h
    lld/test/MachO/lc-linker-option.ll

Removed: 
    


################################################################################
diff  --git a/lld/MachO/Driver.cpp b/lld/MachO/Driver.cpp
index ce7f6d567b613b..88e865af5cd74b 100644
--- a/lld/MachO/Driver.cpp
+++ b/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,10 +428,8 @@ 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);
@@ -439,8 +437,7 @@ static void addLibrary(StringRef name, bool isNeeded, bool isWeak,
 
 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.

diff  --git a/lld/MachO/Driver.h b/lld/MachO/Driver.h
index fe97324e3a35ec..82cd1880ecab1b 100644
--- a/lld/MachO/Driver.h
+++ b/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);
 

diff  --git a/lld/MachO/InputFiles.cpp b/lld/MachO/InputFiles.cpp
index 470b590c5a473f..931d248d016041 100644
--- a/lld/MachO/InputFiles.cpp
+++ b/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)) {

diff  --git a/lld/MachO/InputFiles.h b/lld/MachO/InputFiles.h
index efe5974d78ef72..7a03cef7121632 100644
--- a/lld/MachO/InputFiles.h
+++ b/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);
 

diff  --git a/lld/test/MachO/lc-linker-option-lto.ll b/lld/test/MachO/lc-linker-option-lto.ll
new file mode 100644
index 00000000000000..aadd242d694ca1
--- /dev/null
+++ b/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
+}

diff  --git a/lld/test/MachO/lc-linker-option-order.ll b/lld/test/MachO/lc-linker-option-order.ll
new file mode 100644
index 00000000000000..f8f7cc88c63237
--- /dev/null
+++ b/lld/test/MachO/lc-linker-option-order.ll
@@ -0,0 +1,144 @@
+; REQUIRES: x86
+; RUN: rm -rf %t; split-file %s %t
+; RUN: llc -filetype=obj %t/foo1.ll -o %t/foo1.o
+; RUN: llc -filetype=obj %t/foo2.ll -o %t/foo2.o
+; RUN: llvm-ar rcs %t/libfoo2.a %t/foo2.o
+; RUN: llc -filetype=obj %t/foo3.ll -o %t/foo3.o
+; RUN: llvm-ar rcs %t/libfoo3.a %t/foo3.o
+
+; RUN: llc -filetype=obj %t/zoo2.ll -o %t/zoo2.o
+; RUN: llvm-ar rcs %t/libzoo2.a %t/zoo2.o
+; RUN: llc -filetype=obj %t/zoo3.ll -o %t/zoo3.o
+; RUN: llvm-ar rcs %t/libzoo3.a %t/zoo3.o
+
+; RUN: llc -filetype=obj %t/bar1.ll -o %t/bar1.o
+; RUN: llc -filetype=obj %t/bar2.ll -o %t/bar2.o
+; RUN: llvm-ar rcs %t/libbar2.a %t/bar2.o
+; RUN: llc -filetype=obj %t/bar3.ll -o %t/bar3.o
+; RUN: llvm-ar rcs %t/libbar3.a %t/bar3.o
+
+; RUN: %lld -dylib -lSystem -L%t %t/foo1.o %t/bar1.o -o %t/order.out
+; RUN: llvm-objdump --no-leading-addr --no-show-raw-insn -d %t/order.out | FileCheck %s
+
+; We want to process input object files first
+; before any lc-linker options are actually resolved.
+; The lc-linker options are recursively processed.
+
+; The following shows a chain of auto linker options,
+; starting with foo1.o and bar1.o:
+;
+; foo1.o -> libfoo2.a(foo2.o) -> libfoo3.a(foo3.o)
+;       \
+;        -> libzoo2.a(zoo2.o) -> libzoo3.a(zoo3.o)
+; bar1.o -> libbar2.a(bar2.o) -> libbar3.a(bar3.o)
+
+; CHECK: <_foo1>:
+; CHECK: <_bar1>:
+; CHECK: <_foo2>:
+; CHECK: <_zoo2>:
+; CHECK: <_bar2>:
+; CHECK: <_foo3>:
+; CHECK: <_zoo3>:
+; CHECK: <_bar3>:
+
+;--- foo1.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 = !{!"-lfoo2"}
+!1 = !{!"-lzoo2"}
+!llvm.linker.options = !{!0, !1}
+
+define i32 @foo1() {
+  %call = call i32 @foo2()
+  %call2 = call i32 @zoo2()
+  %add = add nsw i32 %call, %call2
+  ret i32 %add
+}
+
+declare i32 @foo2()
+declare i32 @zoo2()
+
+;--- foo2.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 = !{!"-lfoo3"}
+!llvm.linker.options = !{!0}
+
+define i32 @foo2() {
+  %call = call i32 @foo3()
+  %add = add nsw i32 %call, 2
+  ret i32 %add
+}
+
+declare i32 @foo3()
+
+;--- foo3.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 @foo3() {
+  ret i32 3
+}
+
+;--- zoo2.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 = !{!"-lzoo3"}
+!llvm.linker.options = !{!0}
+
+define i32 @zoo2() {
+  %call = call i32 @zoo3()
+  %add = add nsw i32 %call, 2
+  ret i32 %add
+}
+
+declare i32 @zoo3()
+
+;--- zoo3.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 @zoo3() {
+  ret i32 30
+}
+
+;--- bar1.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 = !{!"-lbar2"}
+!llvm.linker.options = !{!0}
+
+define i32 @bar1() {
+  %call = call i32 @bar2()
+  %add = add nsw i32 %call, 10
+  ret i32 %add
+}
+
+declare i32 @bar2()
+
+;--- bar2.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 = !{!"-lbar3"}
+!llvm.linker.options = !{!0}
+
+define i32 @bar2() {
+  %call = call i32 @bar3()
+  %add = add nsw i32 %call, 200
+  ret i32 %add
+}
+
+declare i32 @bar3()
+
+;--- bar3.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 @bar3() {
+  ret i32 300
+}

diff  --git a/lld/test/MachO/lc-linker-option.ll b/lld/test/MachO/lc-linker-option.ll
index ab16e2121e82b9..399b764134f326 100644
--- a/lld/test/MachO/lc-linker-option.ll
+++ b/lld/test/MachO/lc-linker-option.ll
@@ -153,8 +153,8 @@
 ; SYMS-NO-FOO-NOT:   g     O __DATA,__objc_data _OBJC_CLASS_$_TestClass
 
 ; UNDEFINED-SYMBOL: undefined symbol: __SomeUndefinedSymbol
-; MISSING-AUTO-LINK: {{.+}}load-missing.o: auto-linked framework not found for -framework Foo
-; MISSING-AUTO-LINK: {{.+}}load-missing.o: auto-linked library not found for -lBar
+; MISSING-AUTO-LINK: {{.+}}: auto-linked framework not found for -framework Foo
+; MISSING-AUTO-LINK: {{.+}}: auto-linked library not found for -lBar
 
 ;--- framework.ll
 target triple = "x86_64-apple-macosx10.15.0"


        


More information about the llvm-commits mailing list