From 7c8c158fadfad58a89754e709516469d5d1c8ac9 Mon Sep 17 00:00:00 2001 From: Dunbobbin Date: Tue, 24 Jun 2025 10:20:35 +0100 Subject: [PATCH 1/5] [LLD][COFF] Make /wholearchive thin-archive member identifiers consistent 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 (https://github.com/llvm/llvm-project/pull/126654), where having the member identifier be the path on disk allows distribution of bitcode members during ThinLTO. --- lld/COFF/Driver.cpp | 7 ++++++- lld/test/COFF/thin-archive.s | 39 +++++++++++++++++++++++------------- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp index 8a016d44d4636..8d466486b3784 100644 --- a/lld/COFF/Driver.cpp +++ b/lld/COFF/Driver.cpp @@ -275,7 +275,12 @@ void LinkerDriver::addBuffer(std::unique_ptr mb, int memberIndex = 0; for (MemoryBufferRef m : getArchiveMembers(ctx, archive)) - addArchiveBuffer(m, "", filename, memberIndex++); + if (!archive->isThin()) + addArchiveBuffer(m, "", filename, memberIndex++); + else + // Pass empty string as archive name so that the original filename is + // used as the buffer identifier. + addArchiveBuffer(m, "", "", /*OffsetInArchive=*/0); return; } addFile(make(ctx, mbref)); diff --git a/lld/test/COFF/thin-archive.s b/lld/test/COFF/thin-archive.s index 55d71ea635673..a7c6737443650 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 # 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 /out:%t.exe 2>&1 | \ +# RUN: FileCheck --check-prefix=ERR --allow-empty %s +# RUN: env LLD_IN_TEST=1 not lld-link @%t.rsp %t_thin.lib 2>&1 | \ +# RUN: FileCheck --check-prefix=NOOBJ %s +# RUN: env LLD_IN_TEST=1 not lld-link @%t.rsp /wholearchive:%t_thin.lib 2>&1 | \ +# RUN: FileCheck --check-prefix=NOOBJWHOLE %s +# RUN: env LLD_IN_TEST=1 not lld-link @%t.rsp %t_thin.lib /demangle:no 2>&1 | \ +# RUN: FileCheck --check-prefix=NOOBJNODEMANGLE %s + +# 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 From 7df43648da2a1402bd60e281ef1959faaa8987a2 Mon Sep 17 00:00:00 2001 From: Dunbobbin Date: Mon, 21 Jul 2025 14:30:55 +0100 Subject: [PATCH 2/5] Account for spaces in paths from lit substitutions --- lld/test/COFF/thin-archive.s | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lld/test/COFF/thin-archive.s b/lld/test/COFF/thin-archive.s index a7c6737443650..1d5492c58bb04 100644 --- a/lld/test/COFF/thin-archive.s +++ b/lld/test/COFF/thin-archive.s @@ -22,7 +22,7 @@ # SYMTAB: ?f@@YAHXZ in # NO-SYMTAB-NOT: ?f@@YAHXZ in -# RUN: echo /entry:main %t.main.obj /out:%t.exe > %t.rsp +# 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 From 741543e8c3fc65f80f0deb37f073ef21c402dee9 Mon Sep 17 00:00:00 2001 From: Dunbobbin Date: Mon, 21 Jul 2025 14:32:42 +0100 Subject: [PATCH 3/5] Remove duplicated command line argument in one of the test-cases. --- lld/test/COFF/thin-archive.s | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lld/test/COFF/thin-archive.s b/lld/test/COFF/thin-archive.s index 1d5492c58bb04..f885460ecc6bc 100644 --- a/lld/test/COFF/thin-archive.s +++ b/lld/test/COFF/thin-archive.s @@ -38,7 +38,7 @@ # LOAD_THIN_WHOLE: Loaded {{.*}}.obj for # RUN: rm %t.lib.obj -# RUN: lld-link @%t.rsp %t.lib /out:%t.exe 2>&1 | \ +# RUN: lld-link @%t.rsp %t.lib 2>&1 | \ # RUN: FileCheck --check-prefix=ERR --allow-empty %s # RUN: env LLD_IN_TEST=1 not lld-link @%t.rsp %t_thin.lib 2>&1 | \ # RUN: FileCheck --check-prefix=NOOBJ %s From c2443311bc75848f8bfc80bfa47a2d53cc7a999c Mon Sep 17 00:00:00 2001 From: Dunbobbin Date: Mon, 21 Jul 2025 14:34:18 +0100 Subject: [PATCH 4/5] Make FileCheck argument order consistent --- lld/test/COFF/thin-archive.s | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lld/test/COFF/thin-archive.s b/lld/test/COFF/thin-archive.s index f885460ecc6bc..7fab10c2b57b4 100644 --- a/lld/test/COFF/thin-archive.s +++ b/lld/test/COFF/thin-archive.s @@ -39,13 +39,13 @@ # RUN: rm %t.lib.obj # RUN: lld-link @%t.rsp %t.lib 2>&1 | \ -# RUN: FileCheck --check-prefix=ERR --allow-empty %s +# 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 --check-prefix=NOOBJ %s +# 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 --check-prefix=NOOBJWHOLE %s +# 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 --check-prefix=NOOBJNODEMANGLE %s +# 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): From ba737e02911cac81a5e872d70b824ed33964e731 Mon Sep 17 00:00:00 2001 From: Dunbobbin Date: Mon, 21 Jul 2025 15:29:04 +0100 Subject: [PATCH 5/5] Use common function to add thin archive members. Improve comment explaining parameter choice. --- lld/COFF/Driver.cpp | 23 ++++++++++++++--------- lld/COFF/Driver.h | 1 + 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/lld/COFF/Driver.cpp b/lld/COFF/Driver.cpp index 8d466486b3784..cba02fc2fd4f5 100644 --- a/lld/COFF/Driver.cpp +++ b/lld/COFF/Driver.cpp @@ -274,13 +274,13 @@ void LinkerDriver::addBuffer(std::unique_ptr mb, make>(std::move(file)); // take ownership int memberIndex = 0; - for (MemoryBufferRef m : getArchiveMembers(ctx, archive)) + for (MemoryBufferRef m : getArchiveMembers(ctx, archive)) { if (!archive->isThin()) addArchiveBuffer(m, "", filename, memberIndex++); else - // Pass empty string as archive name so that the original filename is - // used as the buffer identifier. - addArchiveBuffer(m, "", "", /*OffsetInArchive=*/0); + addThinArchiveBuffer(m, ""); + } + return; } addFile(make(ctx, mbref)); @@ -391,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) { @@ -427,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 task); bool run();