[lld] [llvm] [LLD] [COFF] Fix linking import libraries with -wholearchive: (PR #122806)
Martin Storsjö via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 15 01:36:28 PST 2025
https://github.com/mstorsjo updated https://github.com/llvm/llvm-project/pull/122806
>From ed06ce5196e439d9cd7f26fd17055474918a0d7f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?= <martin at martin.st>
Date: Sun, 6 Oct 2024 00:00:48 +0300
Subject: [PATCH] [LLD] [COFF] Fix linking import libraries with -wholearchive:
When LLD links against an import library (for the regular, short
import libraries), it doesn't actually link in the header/trailer
object files at all, but synthesizes new corresponding data
structures into the right sections.
If the whole of such an import library is forced to be linked,
e.g. with the -wholearchive: option, we actually end up linking
in those header/trailer objects. The header objects contain a
construct which LLD fails to handle; previously we'd error out
with the error ".idata$4 should not refer to special section 0".
Within the import library header object, in the import directory
we have relocations towards the IAT (.idata$4 and .idata$5),
but the header object itself doesn't contain any data for those
sections.
In the case of GNU generated import libraries, the header objects
contain zero length sections .idata$4 and .idata$5, with
relocations against them. However in the case of LLVM generated
import libraries, the sections .idata$4 and .idata$5 are not
included in the list of sections. The symbol table does contain
section symbols for these sections, but without any actual
associated section. This can probably be seen as a declaration of
an empty section.
If the header/trailer objects of a short import library are
linked forcibly and we also reference other functions in the
library, we end up with two import directory entries for this
DLL, one that gets synthesized by LLD, and one from the actual
header object file. This is inelegant, but should be acceptable.
While it would seem unusual to link import libraries with the
-wholearchive: option, this can happen in certain scenarios.
Rust builds libraries that contain relevant import libraries bundled
along with compiled Rust code as regular object files, all within
one single archive. Such an archive can then end up linked with the
-wholarchive: option, if build systems decide to use such an option
for including static libraries.
This should fix https://github.com/msys2/MINGW-packages/issues/21017.
This works for the header/trailer object files in import libraries
generated by LLVM; import libraries generated by MSVC are vaguely
different. ecb5ea6a266d5cc4e05252f6db4c73613b73cc3b did an attempt
at fixing the issue for MSVC generated libraries, but it's not
entirely correct, and isn't enough for making things work for
that case.
---
lld/COFF/InputFiles.cpp | 20 ++++++++++
lld/test/COFF/empty-section-decl.yaml | 56 +++++++++++++++++++++++++++
lld/test/COFF/wholearchive-implib.s | 35 +++++++++++++++++
llvm/include/llvm/Object/COFF.h | 5 +++
4 files changed, 116 insertions(+)
create mode 100644 lld/test/COFF/empty-section-decl.yaml
create mode 100644 lld/test/COFF/wholearchive-implib.s
diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp
index a94c984cfd4870..9c56579d579ef0 100644
--- a/lld/COFF/InputFiles.cpp
+++ b/lld/COFF/InputFiles.cpp
@@ -746,6 +746,26 @@ std::optional<Symbol *> ObjFile::createDefined(
if (sectionNumber == llvm::COFF::IMAGE_SYM_DEBUG)
return nullptr;
+ if (sym.isEmptySectionDeclaration()) {
+ // As there is no coff_section in the object file for these, make a
+ // new virtual one, with everything zeroed out (i.e. an empty section),
+ // with only the name and characteristics set.
+ StringRef name = getName();
+ auto *hdr = make<coff_section>();
+ memset(hdr, 0, sizeof(*hdr));
+ strncpy(hdr->Name, name.data(),
+ std::min(name.size(), (size_t)COFF::NameSize));
+ // We have no idea what characteristics should be assumed here; pick
+ // a default. This matches what is used for .idata sections in the regular
+ // object files in import libraries.
+ hdr->Characteristics = IMAGE_SCN_CNT_INITIALIZED_DATA | IMAGE_SCN_MEM_READ |
+ IMAGE_SCN_MEM_WRITE | IMAGE_SCN_ALIGN_4BYTES;
+ auto *sc = make<SectionChunk>(this, hdr);
+ chunks.push_back(sc);
+ return make<DefinedRegular>(this, /*Name*/ "", /*IsCOMDAT*/ false,
+ /*IsExternal*/ false, sym.getGeneric(), sc);
+ }
+
if (llvm::COFF::isReservedSectionNumber(sectionNumber))
Fatal(ctx) << toString(this) << ": " << getName()
<< " should not refer to special section "
diff --git a/lld/test/COFF/empty-section-decl.yaml b/lld/test/COFF/empty-section-decl.yaml
new file mode 100644
index 00000000000000..320df340000289
--- /dev/null
+++ b/lld/test/COFF/empty-section-decl.yaml
@@ -0,0 +1,56 @@
+# REQUIRES: x86
+
+# RUN: yaml2obj %s -o %t.obj
+# RUN: lld-link -dll -out:%t.dll %t.obj -noentry -subsystem:console -lldmap:%t.map
+# RUN: llvm-objdump -s %t.dll | FileCheck %s
+# RUN: FileCheck %s --check-prefix=MAP < %t.map
+
+# CHECK: Contents of section .itest:
+# CHECK-NEXT: 180001000 0c100080 01000000 00000000 01000000
+
+# MAP: 00001000 0000000a 4 {{.*}}:(.itest$2)
+# MAP: 00001000 00000000 0 .itest$2
+# MAP: 0000100c 00000000 4 {{.*}}:(.itest$4)
+# MAP: 0000100c 00000000 0 .itest$4
+# MAP: 0000100c 00000004 2 {{.*}}:(.itest$6)
+# MAP: 0000100c 00000000 0 .itest$6
+
+--- !COFF
+header:
+ Machine: IMAGE_FILE_MACHINE_AMD64
+ Characteristics: [ ]
+sections:
+ - Name: '.itest$2'
+ Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
+ Alignment: 4
+ SectionData: '00000000000000000000'
+ SizeOfRawData: 10
+ Relocations:
+ - VirtualAddress: 0
+ SymbolName: '.itest$4'
+ Type: IMAGE_REL_AMD64_ADDR64
+ - Name: '.itest$6'
+ Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
+ Alignment: 2
+ SectionData: 01000000
+ SizeOfRawData: 4
+symbols:
+ - Name: '.itest$2'
+ Value: 0
+ SectionNumber: 1
+ SimpleType: IMAGE_SYM_TYPE_NULL
+ ComplexType: IMAGE_SYM_DTYPE_NULL
+ StorageClass: IMAGE_SYM_CLASS_SECTION
+ - Name: '.itest$6'
+ Value: 0
+ SectionNumber: 2
+ SimpleType: IMAGE_SYM_TYPE_NULL
+ ComplexType: IMAGE_SYM_DTYPE_NULL
+ StorageClass: IMAGE_SYM_CLASS_STATIC
+ - Name: '.itest$4'
+ Value: 0
+ SectionNumber: 0
+ SimpleType: IMAGE_SYM_TYPE_NULL
+ ComplexType: IMAGE_SYM_DTYPE_NULL
+ StorageClass: IMAGE_SYM_CLASS_SECTION
+...
diff --git a/lld/test/COFF/wholearchive-implib.s b/lld/test/COFF/wholearchive-implib.s
new file mode 100644
index 00000000000000..0c98ca0ddef072
--- /dev/null
+++ b/lld/test/COFF/wholearchive-implib.s
@@ -0,0 +1,35 @@
+// REQUIRES: x86
+// RUN: split-file %s %t.dir
+// RUN: llvm-lib -machine:amd64 -out:%t.lib -def:%t.dir/lib.def
+// RUN: llvm-mc -filetype=obj -triple=x86_64-windows %t.dir/main.s -o %t.main.obj
+
+// RUN: lld-link -out:%t.exe %t.main.obj -wholearchive:%t.lib -entry:entry -subsystem:console
+// RUN: llvm-readobj --coff-imports %t.exe | FileCheck %s
+
+// As LLD usually doesn't use the header/trailer object files from import
+// libraries, but instead synthesizes those structures, we end up with two
+// import directory entries if we force those objects to be included.
+
+// CHECK: Import {
+// CHECK-NEXT: Name: lib.dll
+// CHECK-NEXT: ImportLookupTableRVA: 0x2050
+// CHECK-NEXT: ImportAddressTableRVA: 0x2068
+// CHECK-NEXT: }
+// CHECK-NEXT: Import {
+// CHECK-NEXT: Name: lib.dll
+// CHECK-NEXT: ImportLookupTableRVA: 0x2058
+// CHECK-NEXT: ImportAddressTableRVA: 0x2070
+// CHECK-NEXT: Symbol: func (0)
+// CHECK-NEXT: }
+
+
+#--- main.s
+.global entry
+entry:
+ call func
+ ret
+
+#--- lib.def
+LIBRARY lib.dll
+EXPORTS
+func
diff --git a/llvm/include/llvm/Object/COFF.h b/llvm/include/llvm/Object/COFF.h
index 05b3587224c296..4de2c680f57b1a 100644
--- a/llvm/include/llvm/Object/COFF.h
+++ b/llvm/include/llvm/Object/COFF.h
@@ -392,6 +392,11 @@ class COFFSymbolRef {
getValue() == 0;
}
+ bool isEmptySectionDeclaration() const {
+ return isSection() && getSectionNumber() == COFF::IMAGE_SYM_UNDEFINED &&
+ getValue() == 0;
+ }
+
bool isWeakExternal() const {
return getStorageClass() == COFF::IMAGE_SYM_CLASS_WEAK_EXTERNAL;
}
More information about the llvm-commits
mailing list