Skip to content

[lld/COFF] Support thin archives in /reproduce: files #121512

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 3, 2025

Conversation

nico
Copy link
Contributor

@nico nico commented Jan 2, 2025

This already worked without /wholearchive; now it works with it too. (Only for thin archives containing relative file names, matching the ELF and Mach-O ports.)

This already worked without /wholearchive; now it works with it
too. (Only for thin archives containing relative file names, matching
the ELF and Mach-O ports.)
@llvmbot
Copy link
Member

llvmbot commented Jan 2, 2025

@llvm/pr-subscribers-platform-windows

@llvm/pr-subscribers-lld-coff

Author: Nico Weber (nico)

Changes

This already worked without /wholearchive; now it works with it too. (Only for thin archives containing relative file names, matching the ELF and Mach-O ports.)


Full diff: https://github.com/llvm/llvm-project/pull/121512.diff

2 Files Affected:

  • (modified) lld/COFF/InputFiles.cpp (+8)
  • (added) lld/test/COFF/linkrepro-thin-archives.s (+23)
diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index e698f66b84f623..a94c984cfd4870 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -149,11 +149,19 @@ std::vector<MemoryBufferRef>
 lld::coff::getArchiveMembers(COFFLinkerContext &ctx, Archive *file) {
   std::vector<MemoryBufferRef> v;
   Error err = Error::success();
+
+  // Thin archives refer to .o files, so --reproduces needs the .o files too.
+  bool addToTar = file->isThin() && ctx.driver.tar;
+
   for (const Archive::Child &c : file->children(err)) {
     MemoryBufferRef mbref =
         CHECK(c.getMemoryBufferRef(),
               file->getFileName() +
                   ": could not get the buffer for a child of the archive");
+    if (addToTar) {
+      ctx.driver.tar->append(relativeToRoot(check(c.getFullName())),
+                             mbref.getBuffer());
+    }
     v.push_back(mbref);
   }
   if (err)
diff --git a/lld/test/COFF/linkrepro-thin-archives.s b/lld/test/COFF/linkrepro-thin-archives.s
new file mode 100644
index 00000000000000..6fde36b84e0af0
--- /dev/null
+++ b/lld/test/COFF/linkrepro-thin-archives.s
@@ -0,0 +1,23 @@
+# REQUIRES: x86
+
+# RUN: rm -rf %t.dir; split-file %s %t.dir
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-windows %t.dir/foo.s -o %t.dir/foo.obj
+# RUN: cd %t.dir
+# RUN: llvm-ar rcsT foo.lib foo.obj
+
+# RUN: lld-link foo.lib /out:/dev/null /reproduce:repro.tar \
+# RUN:     /subsystem:console /machine:x64
+# RUN: tar tf repro.tar | FileCheck -DPATH='repro/%:t.dir' %s
+
+# RUN: lld-link /wholearchive foo.lib /out:/dev/null /reproduce:repro2.tar \
+# RUN:     /subsystem:console /machine:x64
+# RUN: tar tf repro2.tar | FileCheck -DPATH='repro2/%:t.dir' %s
+
+# CHECK-DAG: [[PATH]]/foo.lib
+# CHECK-DAG: [[PATH]]/foo.obj
+
+#--- foo.s
+.globl mainCRTStartup
+mainCRTStartup:
+  nop

@llvmbot
Copy link
Member

llvmbot commented Jan 2, 2025

@llvm/pr-subscribers-lld

Author: Nico Weber (nico)

Changes

This already worked without /wholearchive; now it works with it too. (Only for thin archives containing relative file names, matching the ELF and Mach-O ports.)


Full diff: https://github.com/llvm/llvm-project/pull/121512.diff

2 Files Affected:

  • (modified) lld/COFF/InputFiles.cpp (+8)
  • (added) lld/test/COFF/linkrepro-thin-archives.s (+23)
diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index e698f66b84f623..a94c984cfd4870 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -149,11 +149,19 @@ std::vector<MemoryBufferRef>
 lld::coff::getArchiveMembers(COFFLinkerContext &ctx, Archive *file) {
   std::vector<MemoryBufferRef> v;
   Error err = Error::success();
+
+  // Thin archives refer to .o files, so --reproduces needs the .o files too.
+  bool addToTar = file->isThin() && ctx.driver.tar;
+
   for (const Archive::Child &c : file->children(err)) {
     MemoryBufferRef mbref =
         CHECK(c.getMemoryBufferRef(),
               file->getFileName() +
                   ": could not get the buffer for a child of the archive");
+    if (addToTar) {
+      ctx.driver.tar->append(relativeToRoot(check(c.getFullName())),
+                             mbref.getBuffer());
+    }
     v.push_back(mbref);
   }
   if (err)
diff --git a/lld/test/COFF/linkrepro-thin-archives.s b/lld/test/COFF/linkrepro-thin-archives.s
new file mode 100644
index 00000000000000..6fde36b84e0af0
--- /dev/null
+++ b/lld/test/COFF/linkrepro-thin-archives.s
@@ -0,0 +1,23 @@
+# REQUIRES: x86
+
+# RUN: rm -rf %t.dir; split-file %s %t.dir
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-windows %t.dir/foo.s -o %t.dir/foo.obj
+# RUN: cd %t.dir
+# RUN: llvm-ar rcsT foo.lib foo.obj
+
+# RUN: lld-link foo.lib /out:/dev/null /reproduce:repro.tar \
+# RUN:     /subsystem:console /machine:x64
+# RUN: tar tf repro.tar | FileCheck -DPATH='repro/%:t.dir' %s
+
+# RUN: lld-link /wholearchive foo.lib /out:/dev/null /reproduce:repro2.tar \
+# RUN:     /subsystem:console /machine:x64
+# RUN: tar tf repro2.tar | FileCheck -DPATH='repro2/%:t.dir' %s
+
+# CHECK-DAG: [[PATH]]/foo.lib
+# CHECK-DAG: [[PATH]]/foo.obj
+
+#--- foo.s
+.globl mainCRTStartup
+mainCRTStartup:
+  nop

Copy link
Member

@mstorsjo mstorsjo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nice, thanks!

In the cases where I've run into this issue, I don't remember if the thin archives used absolute or relative paths though. I think the main cases where I've run into this are related to meson, where meson makes non-installed internal static libraries as thin libraries by default. It does seem like meson uses relative paths for those.

@nico nico merged commit 6cd171d into llvm:main Jan 3, 2025
12 checks passed
@nico nico deleted the lld-coff-thin-repro branch January 3, 2025 13:20
@nico
Copy link
Contributor Author

nico commented Jan 15, 2025

(Only for thin archives containing relative file names, matching the ELF and Mach-O ports.)

(Absolute file name support is #97845, which now also applies to COFF.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants