[LLD] [COFF] Fix linking import libraries with -wholearchive: (llvm#1…

…22806)

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 msys2/MINGW-packages#21017.

This works for the header/trailer object files in import libraries
generated by LLVM; import libraries generated by MSVC are vaguely
different. ecb5ea6 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

@@ -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 "

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
+...

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

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;
   }