[lld] [lld/COFF] Handle -start-lib / -end-lib better in /reproduce: output (PR #119752)

Nico Weber via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 12 18:05:51 PST 2024


https://github.com/nico updated https://github.com/llvm/llvm-project/pull/119752

>From 2ca1f5d3627def37e0e8b96e58425ea496ff9b2a Mon Sep 17 00:00:00 2001
From: Nico Weber <thakis at chromium.org>
Date: Thu, 12 Dec 2024 14:16:09 -0500
Subject: [PATCH 1/2] [lld/COFF] Handle -start-lib / -end-lib better in
 /reproduce: output

Previously, we'd collect all input files in Driver::filePaths,
and then write filePaths after all other flags in
createResponseFile(). This meant that `-start-lib foo.obj -end-lib`
would be written as `-start-lib -end-lib foo.obj`, changing semantics.

Instead, remove Driver::filePaths, and handle things that fed into
it directly:

* OPT_INPUT is now handled in the same way as other flags, so that
  we now get `-start-lib foo.obj -end-lib` in response.txt as desired.
  Add a test for -start-lib / -end-lib and /reproduce:.

* OPT_wholearchive_file needs explicit handling now -- but before,
  this was buggy as well: We'd put the flag without a rewritten
  path in response.txt, but also the rewritten input file without
  wholearchive semantics via filePaths. So this commit makes
  --whole-archive work with /reproduce: too, and adds test coverage.

* /defaultlib:foo is now written as /defaultlib:foo into response.txt,
  instead of writing the resolved path previously. While response.txt
  looks slightly differently, both should have the same semantics, and
  this should be mostly a no-op. (It does require updating a test.)

* /defaultlib: from .drectve sections are no longer recorded in
  response.txt. This seems like a progression -- in the non-repro
  case they come from .obj files, so they should come (only) from
  there in the repro case too. This adds test coverage for this case.

Makes createResponseFile() look more like the versions in the ELF
and MachO ports too.
---
 lld/COFF/Driver.cpp          |  15 +++--
 lld/COFF/Driver.h            |   1 -
 lld/test/COFF/linkrepro.test | 107 +++++++++++++++++++++++++++++++++--
 3 files changed, 109 insertions(+), 14 deletions(-)

diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 714de67e88b065..43287530d8e3d4 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -204,7 +204,6 @@ void LinkerDriver::addBuffer(std::unique_ptr<MemoryBuffer> mb,
   StringRef filename = mb->getBufferIdentifier();
 
   MemoryBufferRef mbref = takeBuffer(std::move(mb));
-  filePaths.push_back(filename);
 
   // File type is detected by contents, not by file extension.
   switch (identify_magic(mbref.getBuffer())) {
@@ -857,7 +856,6 @@ static std::string rewritePath(StringRef s) {
 // Reconstructs command line arguments so that so that you can re-run
 // the same command with the same inputs. This is for --reproduce.
 static std::string createResponseFile(const opt::InputArgList &args,
-                                      ArrayRef<StringRef> filePaths,
                                       ArrayRef<StringRef> searchPaths) {
   SmallString<0> data;
   raw_svector_ostream os(data);
@@ -866,11 +864,15 @@ static std::string createResponseFile(const opt::InputArgList &args,
     switch (arg->getOption().getID()) {
     case OPT_linkrepro:
     case OPT_reproduce:
-    case OPT_INPUT:
-    case OPT_defaultlib:
     case OPT_libpath:
     case OPT_winsysroot:
       break;
+    case OPT_INPUT:
+      os << quote(rewritePath(arg->getValue())) << "\n";
+      break;
+    case OPT_wholearchive_file:
+      os << arg->getSpelling() << quote(rewritePath(arg->getValue())) << "\n";
+      break;
     case OPT_call_graph_ordering_file:
     case OPT_deffile:
     case OPT_manifestinput:
@@ -907,9 +909,6 @@ static std::string createResponseFile(const opt::InputArgList &args,
     os << "/libpath:" << quote(relPath) << "\n";
   }
 
-  for (StringRef path : filePaths)
-    os << quote(relativeToRoot(path)) << "\n";
-
   return std::string(data);
 }
 
@@ -2348,7 +2347,7 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
   if (tar) {
     llvm::TimeTraceScope timeScope("Reproducer: response file");
     tar->append("response.txt",
-                createResponseFile(args, filePaths,
+                createResponseFile(args,
                                    ArrayRef<StringRef>(searchPaths).slice(1)));
   }
 
diff --git a/lld/COFF/Driver.h b/lld/COFF/Driver.h
index 3889feb7511c0a..71577aec3a199d 100644
--- a/lld/COFF/Driver.h
+++ b/lld/COFF/Driver.h
@@ -195,7 +195,6 @@ class LinkerDriver {
   bool run();
 
   std::list<std::function<void()>> taskQueue;
-  std::vector<StringRef> filePaths;
   std::vector<MemoryBufferRef> resources;
 
   llvm::DenseSet<StringRef> directivesExports;
diff --git a/lld/test/COFF/linkrepro.test b/lld/test/COFF/linkrepro.test
index d5a34a201a5da4..a5779a9ec82e16 100644
--- a/lld/test/COFF/linkrepro.test
+++ b/lld/test/COFF/linkrepro.test
@@ -1,7 +1,10 @@
 # REQUIRES: x86, shell
 
 # RUN: rm -rf %t.dir
+# RUN: split-file %s %t.dir
+
 # RUN: yaml2obj %p/Inputs/hello32.yaml -o %t.obj
+# RUN: llvm-mc -filetype=obj -triple=i386-windows %t.dir/drectve.s -o %t.dir/drectve.obj
 # RUN: echo '_main at 0' > %t.order
 # RUN: touch %t.def
 # RUN: touch %t.cg
@@ -46,7 +49,7 @@ and various other flags.
 # RUN: diff %t.order repro/%:t.order
 # RUN: diff %t.def repro/%:t.def
 # RUN: diff %p/Inputs/std32.lib repro/%:p/Inputs/std32.lib
-# RUN: FileCheck %s --check-prefix=RSP < repro/response.txt
+# RUN: FileCheck %s --check-prefix=RSP-DEFAULTLIB < repro/response.txt
 # RUN: cd repro; lld-link @response.txt
 
 Test adding .lib files with LIB env var to repro archive,
@@ -61,7 +64,7 @@ and various other flags.
 # RUN: diff %t.order repro/%:t.order
 # RUN: diff %t.def repro/%:t.def
 # RUN: diff %p/Inputs/std32.lib repro/%:p/Inputs/std32.lib
-# RUN: FileCheck %s --check-prefix=RSP < repro/response.txt
+# RUN: FileCheck %s --check-prefix=RSP-DEFAULTLIB < repro/response.txt
 # RUN: cd repro; lld-link @response.txt
 
 # LIST: .obj
@@ -70,14 +73,21 @@ and various other flags.
 # LIST: .def
 # LIST: .order
 
+# RSP: linkrepro.test.tmp.obj
+# RSP: std32.lib
 # RSP: /subsystem:console
 # RSP: /entry:main at 0
 # RSP: /out:
 # RSP-NOT: /order:@/
 # RSP-NOT: /def:/
-# RSP: linkrepro.test.tmp.obj
-# RSP-NOT: defaultlib
-# RSP: std32.lib
+
+# RSP-DEFAULTLIB: linkrepro.test.tmp.obj
+# RSP-DEFAULTLIB: /defaultlib:std32
+# RSP-DEFAULTLIB: /subsystem:console
+# RSP-DEFAULTLIB: /entry:main at 0
+# RSP-DEFAULTLIB: /out:
+# RSP-DEFAULTLIB-NOT: /order:@/
+# RSP-DEFAULTLIB-NOT: /def:/
 
 Test /call-graph-ordering-file (can't be used with /order:, needs separate test)
 # RUN: mkdir -p %t.dir/build5
@@ -96,3 +106,90 @@ Test /call-graph-ordering-file (can't be used with /order:, needs separate test)
 # LISTCG: .cg
 
 # RSPCG-NOT: /call-graph-ordering-file:/
+
+Test /defaultlib: from a .drectve section
+# RUN: mkdir -p %t.dir/build6
+# RUN: cd %t.dir/build6
+# RUN: lld-link %t.obj %t.dir/drectve.obj /libpath:%p/Inputs /subsystem:console \
+# RUN:   /entry:main at 0 /linkrepro:. -safeseh:no /out:%t.exe /order:@%t.order /def:%t.def
+# RUN: tar tf repro.tar | FileCheck --check-prefix=LIST %s
+# RUN: tar xf repro.tar
+# RUN: diff %t.obj repro/%:t.obj
+# RUN: diff %t.order repro/%:t.order
+# RUN: diff %t.def repro/%:t.def
+# RUN: diff %p/Inputs/std32.lib repro/%:p/Inputs/std32.lib
+# RUN: FileCheck %s --check-prefix=RSP-DRECTVE < repro/response.txt
+# RUN: cd repro; lld-link @response.txt
+
+# RSP-DRECTVE: linkrepro.test.tmp.obj
+# RSP-DRECTVE: drectve.obj
+# RSP-DRECTVE: /subsystem:console
+# RSP-DRECTVE: /entry:main at 0
+# RSP-DRECTVE: -safeseh:no
+# RSP-DRECTVE: /out:
+
+Test /wholearchive: with /linkrepro:
+# RUN: llvm-mc -filetype=obj -triple=i386-windows %t.dir/archive.s -o %t.dir/archive.obj
+# RUN: rm -f %t.dir/build7/archive.lib
+# RUN: llvm-ar rcs %t.dir/archive.lib %t.dir/archive.obj
+# RUN: mkdir -p %t.dir/build7
+# RUN: cd %t.dir/build7
+ RUN: lld-link %t.obj /defaultlib:std32 /libpath:%p/Inputs /subsystem:console \
+# RUN:   /entry:main at 0 /linkrepro:. -safeseh:no /wholearchive:%t.dir/archive.lib \
+# RUN:   /out:%t.exe /order:@%t.order /def:%t.def
+# RUN: tar tf repro.tar | FileCheck --check-prefix=LIST %s
+# RUN: tar xf repro.tar
+# RUN: diff %t.obj repro/%:t.obj
+# RUN: diff %t.order repro/%:t.order
+# RUN: diff %t.def repro/%:t.def
+# RUN: diff %p/Inputs/std32.lib repro/%:p/Inputs/std32.lib
+# RUN: FileCheck %s --check-prefix=RSP-WHOLEARCHIVE < repro/response.txt
+# RUN: cd repro; lld-link @response.txt
+
+# RSP-WHOLEARCHIVE: linkrepro.test.tmp.obj
+# RSP-WHOLEARCHIVE: /defaultlib:std32
+# RSP-WHOLEARCHIVE: /subsystem:console
+# RSP-WHOLEARCHIVE: /entry:main at 0
+# RSP-WHOLEARCHIVE: -safeseh:no
+# RSP-WHOLEARCHIVE: /wholearchive:{{.*}}archive.lib
+# RSP-WHOLEARCHIVE: /out:
+
+Test /start-lib / /end-lib with /linkrepro:
+# RUN: mkdir -p %t.dir/build8
+# RUN: cd %t.dir/build8
+# RUN: lld-link %t.obj /defaultlib:std32 /libpath:%p/Inputs /subsystem:console \
+# RUN:   /entry:main at 0 /linkrepro:. -safeseh:no /start-lib %t.dir/drectve.obj /end-lib \
+# RUN:   /out:%t.exe /order:@%t.order /def:%t.def
+# RUN: tar tf repro.tar | FileCheck --check-prefix=LIST %s
+# RUN: tar xf repro.tar
+# RUN: diff %t.obj repro/%:t.obj
+# RUN: diff %t.order repro/%:t.order
+# RUN: diff %t.def repro/%:t.def
+# RUN: diff %p/Inputs/std32.lib repro/%:p/Inputs/std32.lib
+# RUN: FileCheck %s --check-prefix=RSP-STARTLIB < repro/response.txt
+# RUN: cd repro; lld-link @response.txt
+
+# RSP-STARTLIB: linkrepro.test.tmp.obj
+# RSP-STARTLIB: /defaultlib:std32
+# RSP-STARTLIB: /subsystem:console
+# RSP-STARTLIB: /entry:main at 0
+# RSP-STARTLIB: -safeseh:no
+# RSP-STARTLIB: /start-lib
+# RSP-STARTLIB-NEXT: drectve.obj
+# RSP-STARTLIB-NEXT: /end-lib
+# RSP-STARTLIB: /out:
+
+#--- drectve.s
+        .section .drectve, "yn"
+        .ascii "/defaultlib:std32"
+
+#--- archive.s
+	.text
+	.intel_syntax noprefix
+	.globl	exportfn3
+	.p2align	4
+exportfn3:
+	ret
+
+	.section	.drectve,"yni"
+	.ascii	" /EXPORT:exportfn3"

>From bc8225346eb570b9f64d2847dfccc7af4268cc95 Mon Sep 17 00:00:00 2001
From: Nico Weber <thakis at chromium.org>
Date: Thu, 12 Dec 2024 21:05:39 -0500
Subject: [PATCH 2/2] clang-format

---
 lld/COFF/Driver.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 43287530d8e3d4..6bec770e72e226 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -2346,9 +2346,9 @@ void LinkerDriver::linkerMain(ArrayRef<const char *> argsArr) {
 
   if (tar) {
     llvm::TimeTraceScope timeScope("Reproducer: response file");
-    tar->append("response.txt",
-                createResponseFile(args,
-                                   ArrayRef<StringRef>(searchPaths).slice(1)));
+    tar->append(
+        "response.txt",
+        createResponseFile(args, ArrayRef<StringRef>(searchPaths).slice(1)));
   }
 
   // Handle /largeaddressaware



More information about the llvm-commits mailing list