Skip to content

release/21.x: [LLD][COFF] Make /wholearchive thin-archive member identifiers consistent (#145487) #150168

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
Jul 24, 2025

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Jul 23, 2025

Backport 9f733f4

Requested by: @bd1976bris

@llvmbot
Copy link
Member Author

llvmbot commented Jul 23, 2025

@aganea What do you think about merging this PR to the release branch?

@llvmbot
Copy link
Member Author

llvmbot commented Jul 23, 2025

@llvm/pr-subscribers-lld-coff

@llvm/pr-subscribers-platform-windows

Author: None (llvmbot)

Changes

Backport 9f733f4

Requested by: @bd1976bris


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

3 Files Affected:

  • (modified) lld/COFF/Driver.cpp (+17-7)
  • (modified) lld/COFF/Driver.h (+1)
  • (modified) lld/test/COFF/thin-archive.s (+25-14)
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 0f649ff1cf4f8..4a7ea6fff217b 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -274,8 +274,13 @@ void LinkerDriver::addBuffer(std::unique_ptr<MemoryBuffer> mb,
       make<std::unique_ptr<Archive>>(std::move(file)); // take ownership
 
       int memberIndex = 0;
-      for (MemoryBufferRef m : getArchiveMembers(ctx, archive))
-        addArchiveBuffer(m, "<whole-archive>", filename, memberIndex++);
+      for (MemoryBufferRef m : getArchiveMembers(ctx, archive)) {
+        if (!archive->isThin())
+          addArchiveBuffer(m, "<whole-archive>", filename, memberIndex++);
+        else
+          addThinArchiveBuffer(m, "<whole-archive>");
+      }
+
       return;
     }
     addFile(make<ArchiveFile>(ctx, mbref));
@@ -386,6 +391,14 @@ void LinkerDriver::addArchiveBuffer(MemoryBufferRef mb, StringRef symName,
   Log(ctx) << "Loaded " << obj << " for " << symName;
 }
 
+void LinkerDriver::addThinArchiveBuffer(MemoryBufferRef mb, StringRef symName) {
+  // Pass an empty string as the archive name and an offset of 0 so that
+  // the original filename is used as the buffer identifier. This is
+  // useful for DTLTO, where having the member identifier be the actual
+  // path on disk enables distribution of bitcode files during ThinLTO.
+  addArchiveBuffer(mb, symName, /*parentName=*/"", /*OffsetInArchive=*/0);
+}
+
 void LinkerDriver::enqueueArchiveMember(const Archive::Child &c,
                                         const Archive::Symbol &sym,
                                         StringRef parentName) {
@@ -422,11 +435,8 @@ void LinkerDriver::enqueueArchiveMember(const Archive::Child &c,
       reportBufferError(errorCodeToError(mbOrErr.second), childName);
     llvm::TimeTraceScope timeScope("Archive: ",
                                    mbOrErr.first->getBufferIdentifier());
-    // Pass empty string as archive name so that the original filename is
-    // used as the buffer identifier.
-    ctx.driver.addArchiveBuffer(takeBuffer(std::move(mbOrErr.first)),
-                                toCOFFString(ctx, sym), "",
-                                /*OffsetInArchive=*/0);
+    ctx.driver.addThinArchiveBuffer(takeBuffer(std::move(mbOrErr.first)),
+                                    toCOFFString(ctx, sym));
   });
 }
 
diff --git a/lld/COFF/Driver.h b/lld/COFF/Driver.h
index 14c97a98875bf..5a9bd5c6d9682 100644
--- a/lld/COFF/Driver.h
+++ b/lld/COFF/Driver.h
@@ -173,6 +173,7 @@ class LinkerDriver {
                  bool lazy);
   void addArchiveBuffer(MemoryBufferRef mbref, StringRef symName,
                         StringRef parentName, uint64_t offsetInArchive);
+  void addThinArchiveBuffer(MemoryBufferRef mbref, StringRef symName);
 
   void enqueueTask(std::function<void()> task);
   bool run();
diff --git a/lld/test/COFF/thin-archive.s b/lld/test/COFF/thin-archive.s
index 55d71ea635673..7fab10c2b57b4 100644
--- a/lld/test/COFF/thin-archive.s
+++ b/lld/test/COFF/thin-archive.s
@@ -22,23 +22,34 @@
 # SYMTAB:        ?f@@YAHXZ in
 # NO-SYMTAB-NOT: ?f@@YAHXZ in
 
-# RUN: lld-link /entry:main %t.main.obj %t.lib /out:%t.exe 2>&1 | \
-# RUN:     FileCheck --allow-empty %s
-# RUN: lld-link /entry:main %t.main.obj %t_thin.lib /out:%t.exe 2>&1 | \
-# RUN:     FileCheck --allow-empty %s
-# RUN: lld-link /entry:main %t.main.obj /wholearchive:%t_thin.lib /out:%t.exe 2>&1 | \
-# RUN:     FileCheck --allow-empty %s
+# RUN: echo "/entry:main \"%t.main.obj\" /out:\"%t.exe\"" > %t.rsp
+
+# RUN: lld-link @%t.rsp %t.lib /verbose 2>&1 | \
+# RUN:     FileCheck %s --check-prefix=LOAD_NON_THIN
+# RUN: lld-link @%t.rsp %t_thin.lib /verbose 2>&1 | \
+# RUN:     FileCheck %s --check-prefix=LOAD_THIN_SYM
+# RUN: lld-link @%t.rsp /wholearchive:%t_thin.lib /verbose 2>&1 | \
+# RUN:     FileCheck %s --check-prefix=LOAD_THIN_WHOLE
+# RUN: lld-link @%t.rsp /wholearchive %t_thin.lib /verbose 2>&1 | \
+# RUN:     FileCheck %s --check-prefix=LOAD_THIN_WHOLE
+
+# LOAD_NON_THIN:   Loaded {{.*}}.lib({{.*}}.obj) for int __cdecl f(void)
+# LOAD_THIN_SYM:   Loaded {{.*}}.obj for int __cdecl f(void)
+# LOAD_THIN_WHOLE: Loaded {{.*}}.obj for <whole-archive>
 
 # RUN: rm %t.lib.obj
-# RUN: lld-link /entry:main %t.main.obj %t.lib /out:%t.exe 2>&1 | \
-# RUN:     FileCheck --allow-empty %s
-# RUN: env LLD_IN_TEST=1 not lld-link /entry:main %t.main.obj %t_thin.lib \
-# RUN:     /out:%t.exe 2>&1 | FileCheck --check-prefix=NOOBJ %s
-# RUN: env LLD_IN_TEST=1 not lld-link /entry:main %t.main.obj %t_thin.lib /out:%t.exe \
-# RUN:     /demangle:no 2>&1 | FileCheck --check-prefix=NOOBJNODEMANGLE %s
-
-# CHECK-NOT: error: could not get the buffer for the member defining
+# RUN: lld-link @%t.rsp %t.lib 2>&1 | \
+# RUN:     FileCheck %s --check-prefix=ERR --allow-empty
+# RUN: env LLD_IN_TEST=1 not lld-link @%t.rsp %t_thin.lib 2>&1 | \
+# RUN:     FileCheck %s --check-prefix=NOOBJ
+# RUN: env LLD_IN_TEST=1 not lld-link @%t.rsp /wholearchive:%t_thin.lib 2>&1 | \
+# RUN:     FileCheck %s --check-prefix=NOOBJWHOLE
+# RUN: env LLD_IN_TEST=1 not lld-link @%t.rsp %t_thin.lib /demangle:no 2>&1 | \
+# RUN:     FileCheck %s --check-prefix=NOOBJNODEMANGLE
+
+# ERR-NOT: error: could not get the buffer for the member defining
 # NOOBJ: error: could not get the buffer for the member defining symbol int __cdecl f(void): {{.*}}.lib({{.*}}.lib.obj):
+# NOOBJWHOLE: error: {{.*}}.lib: could not get the buffer for a child of the archive: '{{.*}}.obj'
 # NOOBJNODEMANGLE: error: could not get the buffer for the member defining symbol ?f@@YAHXZ: {{.*}}.lib({{.*}}.lib.obj):
 
 	.text

@llvmbot
Copy link
Member Author

llvmbot commented Jul 23, 2025

@llvm/pr-subscribers-lld

Author: None (llvmbot)

Changes

Backport 9f733f4

Requested by: @bd1976bris


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

3 Files Affected:

  • (modified) lld/COFF/Driver.cpp (+17-7)
  • (modified) lld/COFF/Driver.h (+1)
  • (modified) lld/test/COFF/thin-archive.s (+25-14)
diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp
index 0f649ff1cf4f8..4a7ea6fff217b 100644
--- a/lld/COFF/Driver.cpp
+++ b/lld/COFF/Driver.cpp
@@ -274,8 +274,13 @@ void LinkerDriver::addBuffer(std::unique_ptr<MemoryBuffer> mb,
       make<std::unique_ptr<Archive>>(std::move(file)); // take ownership
 
       int memberIndex = 0;
-      for (MemoryBufferRef m : getArchiveMembers(ctx, archive))
-        addArchiveBuffer(m, "<whole-archive>", filename, memberIndex++);
+      for (MemoryBufferRef m : getArchiveMembers(ctx, archive)) {
+        if (!archive->isThin())
+          addArchiveBuffer(m, "<whole-archive>", filename, memberIndex++);
+        else
+          addThinArchiveBuffer(m, "<whole-archive>");
+      }
+
       return;
     }
     addFile(make<ArchiveFile>(ctx, mbref));
@@ -386,6 +391,14 @@ void LinkerDriver::addArchiveBuffer(MemoryBufferRef mb, StringRef symName,
   Log(ctx) << "Loaded " << obj << " for " << symName;
 }
 
+void LinkerDriver::addThinArchiveBuffer(MemoryBufferRef mb, StringRef symName) {
+  // Pass an empty string as the archive name and an offset of 0 so that
+  // the original filename is used as the buffer identifier. This is
+  // useful for DTLTO, where having the member identifier be the actual
+  // path on disk enables distribution of bitcode files during ThinLTO.
+  addArchiveBuffer(mb, symName, /*parentName=*/"", /*OffsetInArchive=*/0);
+}
+
 void LinkerDriver::enqueueArchiveMember(const Archive::Child &c,
                                         const Archive::Symbol &sym,
                                         StringRef parentName) {
@@ -422,11 +435,8 @@ void LinkerDriver::enqueueArchiveMember(const Archive::Child &c,
       reportBufferError(errorCodeToError(mbOrErr.second), childName);
     llvm::TimeTraceScope timeScope("Archive: ",
                                    mbOrErr.first->getBufferIdentifier());
-    // Pass empty string as archive name so that the original filename is
-    // used as the buffer identifier.
-    ctx.driver.addArchiveBuffer(takeBuffer(std::move(mbOrErr.first)),
-                                toCOFFString(ctx, sym), "",
-                                /*OffsetInArchive=*/0);
+    ctx.driver.addThinArchiveBuffer(takeBuffer(std::move(mbOrErr.first)),
+                                    toCOFFString(ctx, sym));
   });
 }
 
diff --git a/lld/COFF/Driver.h b/lld/COFF/Driver.h
index 14c97a98875bf..5a9bd5c6d9682 100644
--- a/lld/COFF/Driver.h
+++ b/lld/COFF/Driver.h
@@ -173,6 +173,7 @@ class LinkerDriver {
                  bool lazy);
   void addArchiveBuffer(MemoryBufferRef mbref, StringRef symName,
                         StringRef parentName, uint64_t offsetInArchive);
+  void addThinArchiveBuffer(MemoryBufferRef mbref, StringRef symName);
 
   void enqueueTask(std::function<void()> task);
   bool run();
diff --git a/lld/test/COFF/thin-archive.s b/lld/test/COFF/thin-archive.s
index 55d71ea635673..7fab10c2b57b4 100644
--- a/lld/test/COFF/thin-archive.s
+++ b/lld/test/COFF/thin-archive.s
@@ -22,23 +22,34 @@
 # SYMTAB:        ?f@@YAHXZ in
 # NO-SYMTAB-NOT: ?f@@YAHXZ in
 
-# RUN: lld-link /entry:main %t.main.obj %t.lib /out:%t.exe 2>&1 | \
-# RUN:     FileCheck --allow-empty %s
-# RUN: lld-link /entry:main %t.main.obj %t_thin.lib /out:%t.exe 2>&1 | \
-# RUN:     FileCheck --allow-empty %s
-# RUN: lld-link /entry:main %t.main.obj /wholearchive:%t_thin.lib /out:%t.exe 2>&1 | \
-# RUN:     FileCheck --allow-empty %s
+# RUN: echo "/entry:main \"%t.main.obj\" /out:\"%t.exe\"" > %t.rsp
+
+# RUN: lld-link @%t.rsp %t.lib /verbose 2>&1 | \
+# RUN:     FileCheck %s --check-prefix=LOAD_NON_THIN
+# RUN: lld-link @%t.rsp %t_thin.lib /verbose 2>&1 | \
+# RUN:     FileCheck %s --check-prefix=LOAD_THIN_SYM
+# RUN: lld-link @%t.rsp /wholearchive:%t_thin.lib /verbose 2>&1 | \
+# RUN:     FileCheck %s --check-prefix=LOAD_THIN_WHOLE
+# RUN: lld-link @%t.rsp /wholearchive %t_thin.lib /verbose 2>&1 | \
+# RUN:     FileCheck %s --check-prefix=LOAD_THIN_WHOLE
+
+# LOAD_NON_THIN:   Loaded {{.*}}.lib({{.*}}.obj) for int __cdecl f(void)
+# LOAD_THIN_SYM:   Loaded {{.*}}.obj for int __cdecl f(void)
+# LOAD_THIN_WHOLE: Loaded {{.*}}.obj for <whole-archive>
 
 # RUN: rm %t.lib.obj
-# RUN: lld-link /entry:main %t.main.obj %t.lib /out:%t.exe 2>&1 | \
-# RUN:     FileCheck --allow-empty %s
-# RUN: env LLD_IN_TEST=1 not lld-link /entry:main %t.main.obj %t_thin.lib \
-# RUN:     /out:%t.exe 2>&1 | FileCheck --check-prefix=NOOBJ %s
-# RUN: env LLD_IN_TEST=1 not lld-link /entry:main %t.main.obj %t_thin.lib /out:%t.exe \
-# RUN:     /demangle:no 2>&1 | FileCheck --check-prefix=NOOBJNODEMANGLE %s
-
-# CHECK-NOT: error: could not get the buffer for the member defining
+# RUN: lld-link @%t.rsp %t.lib 2>&1 | \
+# RUN:     FileCheck %s --check-prefix=ERR --allow-empty
+# RUN: env LLD_IN_TEST=1 not lld-link @%t.rsp %t_thin.lib 2>&1 | \
+# RUN:     FileCheck %s --check-prefix=NOOBJ
+# RUN: env LLD_IN_TEST=1 not lld-link @%t.rsp /wholearchive:%t_thin.lib 2>&1 | \
+# RUN:     FileCheck %s --check-prefix=NOOBJWHOLE
+# RUN: env LLD_IN_TEST=1 not lld-link @%t.rsp %t_thin.lib /demangle:no 2>&1 | \
+# RUN:     FileCheck %s --check-prefix=NOOBJNODEMANGLE
+
+# ERR-NOT: error: could not get the buffer for the member defining
 # NOOBJ: error: could not get the buffer for the member defining symbol int __cdecl f(void): {{.*}}.lib({{.*}}.lib.obj):
+# NOOBJWHOLE: error: {{.*}}.lib: could not get the buffer for a child of the archive: '{{.*}}.obj'
 # NOOBJNODEMANGLE: error: could not get the buffer for the member defining symbol ?f@@YAHXZ: {{.*}}.lib({{.*}}.lib.obj):
 
 	.text

@tru tru moved this from Needs Triage to Needs Merge in LLVM Release Status Jul 24, 2025
…tent (llvm#145487)

A thin archive is an archive/library format where the archive itself
contains only references to member object files on disk, rather than
embedding the file contents.

For the non-/wholearchive case, we use the path to the archive member as
the identifier for thin-archive members (see comments in
`enqueueArchiveMember`). This patch modifies the /wholearchive path to
behave the same way.

Apart from consistency, my motivation for fixing this is DTLTO
(llvm#126654), where having the member identifier be the path on disk allows
distribution of bitcode members during ThinLTO.

(cherry picked from commit 9f733f4)
@tru tru merged commit 64a274f into llvm:release/21.x Jul 24, 2025
@github-project-automation github-project-automation bot moved this from Needs Merge to Done in LLVM Release Status Jul 24, 2025
Copy link

@bd1976bris (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

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

Successfully merging this pull request may close these issues.

3 participants