Skip to content

Commit 839155f

Browse files
mstorsjomstorsjo
committed
[LLD] [COFF] Fix linking MSVC generated implib header objects
ecb5ea6 tried to fix cases when LLD links what seems to be import library header objects from MSVC. However, the fix seems incorrect; the review at https://reviews.llvm.org/D133627 concluded that if this (treating this kind of symbol as a common symbol) is what link.exe does, it's fine. However, this is most probably not what link.exe does. The symbol mentioned in the commit message of ecb5ea6 would be a common symbol with a size of around 3 GB; this is not what might have been intended. That commit tried to avoid running into the error ".idata$4 should not refer to special section 0"; that issue is fixed for a similar style of section symbols in XXX (reference to preceding commit/PR). Therefore, revert ecb5ea6 and extend the fix from XXX (preceding commit/PR) to also work for the section symbols in MSVC generated import libraries. The primary strange thing about them, is that the section symbols have the value 3221225536, 0xC0000040. This value matches the Windows status code STATUS_SECTION_TOO_BIG. If interpreted literally, this value would mean that the symbols refer to an offset of 3 GB after the start of a section. Therefore, this seemingly bogus offset needs to be handled somehow. GNU binutils also does support linking against MSVC generated import libraries - I wonder how they do it. Do they ignore the symbol Value field for symbols of type IMAGE_SYM_CLASS_SECTION? The current hacky solution in this commit is not correct as is though. Finally - for testing, I'm constructing the header/trailer object files from .yaml, but it's hard to construct a full proper import library that way, as the short import library files for the individual symbols can't be synthesized from .yaml yet. (And even if we could do that, it's a bit tricky to build a static library with all members named e.g. "foo.dll".)
1 parent aa4da95 commit 839155f

File tree

7 files changed

+154
-25
lines changed

7 files changed

+154
-25
lines changed

‎lld/COFF/Symbols.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,9 @@ class DefinedRegular : public DefinedCOFF {
219219
return s->kind() == DefinedRegularKind;
220220
}
221221

222-
uint64_t getRVA() const { return (*data)->getRVA() + sym->Value; }
222+
uint64_t getRVA() const {
223+
return (*data)->getRVA() + (sym->Value != 0xC0000040 ? sym->Value : 0);
224+
}
223225
SectionChunk *getChunk() const { return *data; }
224226
uint32_t getValue() const { return sym->Value; }
225227

‎lld/test/COFF/Inputs/msvc-implib1.yaml‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
--- !COFF
2+
header:
3+
Machine: IMAGE_FILE_MACHINE_AMD64
4+
Characteristics: [ ]
5+
sections:
6+
- Name: '.idata$2'
7+
Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
8+
Alignment: 4
9+
SectionData: '0000000000000000000000000000000000000000'
10+
SizeOfRawData: 20
11+
Relocations:
12+
- VirtualAddress: 12
13+
SymbolName: '.idata$6'
14+
Type: IMAGE_REL_AMD64_ADDR32NB
15+
- VirtualAddress: 0
16+
SymbolName: '.idata$4'
17+
Type: IMAGE_REL_AMD64_ADDR32NB
18+
- VirtualAddress: 16
19+
SymbolName: '.idata$5'
20+
Type: IMAGE_REL_AMD64_ADDR32NB
21+
- Name: '.idata$6'
22+
Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
23+
Alignment: 2
24+
SectionData: 666F6F2E646C6C00
25+
SizeOfRawData: 8
26+
symbols:
27+
- Name: '@comp.id'
28+
Value: 16877185
29+
SectionNumber: -1
30+
SimpleType: IMAGE_SYM_TYPE_NULL
31+
ComplexType: IMAGE_SYM_DTYPE_NULL
32+
StorageClass: IMAGE_SYM_CLASS_STATIC
33+
- Name: __IMPORT_DESCRIPTOR_foo
34+
Value: 0
35+
SectionNumber: 1
36+
SimpleType: IMAGE_SYM_TYPE_NULL
37+
ComplexType: IMAGE_SYM_DTYPE_NULL
38+
StorageClass: IMAGE_SYM_CLASS_EXTERNAL
39+
- Name: '.idata$2'
40+
Value: 3221225536
41+
SectionNumber: 1
42+
SimpleType: IMAGE_SYM_TYPE_NULL
43+
ComplexType: IMAGE_SYM_DTYPE_NULL
44+
StorageClass: IMAGE_SYM_CLASS_SECTION
45+
- Name: '.idata$6'
46+
Value: 0
47+
SectionNumber: 2
48+
SimpleType: IMAGE_SYM_TYPE_NULL
49+
ComplexType: IMAGE_SYM_DTYPE_NULL
50+
StorageClass: IMAGE_SYM_CLASS_STATIC
51+
- Name: '.idata$4'
52+
Value: 3221225536
53+
SectionNumber: 0
54+
SimpleType: IMAGE_SYM_TYPE_NULL
55+
ComplexType: IMAGE_SYM_DTYPE_NULL
56+
StorageClass: IMAGE_SYM_CLASS_SECTION
57+
- Name: '.idata$5'
58+
Value: 3221225536
59+
SectionNumber: 0
60+
SimpleType: IMAGE_SYM_TYPE_NULL
61+
ComplexType: IMAGE_SYM_DTYPE_NULL
62+
StorageClass: IMAGE_SYM_CLASS_SECTION
63+
- Name: __NULL_IMPORT_DESCRIPTOR
64+
Value: 0
65+
SectionNumber: 0
66+
SimpleType: IMAGE_SYM_TYPE_NULL
67+
ComplexType: IMAGE_SYM_DTYPE_NULL
68+
StorageClass: IMAGE_SYM_CLASS_EXTERNAL
69+
- Name: "foo_NULL_THUNK_DATA"
70+
Value: 0
71+
SectionNumber: 0
72+
SimpleType: IMAGE_SYM_TYPE_NULL
73+
ComplexType: IMAGE_SYM_DTYPE_NULL
74+
StorageClass: IMAGE_SYM_CLASS_EXTERNAL
75+
...

‎lld/test/COFF/Inputs/msvc-implib2.yaml‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
--- !COFF
2+
header:
3+
Machine: IMAGE_FILE_MACHINE_AMD64
4+
Characteristics: [ ]
5+
sections:
6+
- Name: '.idata$3'
7+
Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
8+
Alignment: 4
9+
SectionData: '0000000000000000000000000000000000000000'
10+
SizeOfRawData: 20
11+
symbols:
12+
- Name: '@comp.id'
13+
Value: 16877185
14+
SectionNumber: -1
15+
SimpleType: IMAGE_SYM_TYPE_NULL
16+
ComplexType: IMAGE_SYM_DTYPE_NULL
17+
StorageClass: IMAGE_SYM_CLASS_STATIC
18+
- Name: __NULL_IMPORT_DESCRIPTOR
19+
Value: 0
20+
SectionNumber: 1
21+
SimpleType: IMAGE_SYM_TYPE_NULL
22+
ComplexType: IMAGE_SYM_DTYPE_NULL
23+
StorageClass: IMAGE_SYM_CLASS_EXTERNAL
24+
...

‎lld/test/COFF/Inputs/msvc-implib3.yaml‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
--- !COFF
2+
header:
3+
Machine: IMAGE_FILE_MACHINE_AMD64
4+
Characteristics: [ ]
5+
sections:
6+
- Name: '.idata$5'
7+
Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
8+
Alignment: 8
9+
SectionData: '0000000000000000'
10+
SizeOfRawData: 8
11+
- Name: '.idata$4'
12+
Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
13+
Alignment: 8
14+
SectionData: '0000000000000000'
15+
SizeOfRawData: 8
16+
symbols:
17+
- Name: '@comp.id'
18+
Value: 16877185
19+
SectionNumber: -1
20+
SimpleType: IMAGE_SYM_TYPE_NULL
21+
ComplexType: IMAGE_SYM_DTYPE_NULL
22+
StorageClass: IMAGE_SYM_CLASS_STATIC
23+
- Name: "foo_NULL_THUNK_DATA"
24+
Value: 0
25+
SectionNumber: 1
26+
SimpleType: IMAGE_SYM_TYPE_NULL
27+
ComplexType: IMAGE_SYM_DTYPE_NULL
28+
StorageClass: IMAGE_SYM_CLASS_EXTERNAL
29+
...

‎lld/test/COFF/wholearchive-implib.s‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,18 @@
33
// RUN: llvm-lib -machine:amd64 -out:%t.lib -def:%t.dir/lib.def
44
// RUN: llvm-mc -filetype=obj -triple=x86_64-windows %t.dir/main.s -o %t.main.obj
55

6+
// RUN: yaml2obj %S/Inputs/msvc-implib1.yaml -o %t.msvc-implib1.obj
7+
// RUN: yaml2obj %S/Inputs/msvc-implib2.yaml -o %t.msvc-implib2.obj
8+
// RUN: yaml2obj %S/Inputs/msvc-implib3.yaml -o %t.msvc-implib3.obj
9+
// RUN: llvm-lib -out:%t-msvc.lib %t.msvc-implib1.obj %t.msvc-implib2.obj %t.msvc-implib3.obj
10+
// RUN: llvm-mc -filetype=obj -triple=x86_64-windows %t.dir/main-nocall.s -o %t.main-nocall.obj
11+
612
// RUN: lld-link -out:%t.exe %t.main.obj -wholearchive:%t.lib -entry:entry -subsystem:console
713
// RUN: llvm-readobj --coff-imports %t.exe | FileCheck %s
814

15+
// RUN: lld-link -out:%t-msvc.exe %t.main-nocall.obj -wholearchive:%t-msvc.lib -entry:entry -subsystem:console
16+
// RUN: llvm-readobj --coff-imports %t-msvc.exe | FileCheck %s --check-prefix=CHECK-MSVC
17+
918
// As LLD usually doesn't use the header/trailer object files from import
1019
// libraries, but instead synthesizes those structures, we end up with two
1120
// import directory entries if we force those objects to be included.
@@ -22,13 +31,24 @@
2231
// CHECK-NEXT: Symbol: func (0)
2332
// CHECK-NEXT: }
2433

34+
// CHECK-MSVC: Import {
35+
// CHECK-MSVC-NEXT: Name: foo.dll
36+
// CHECK-MSVC-NEXT: ImportLookupTableRVA: 0x2040
37+
// CHECK-MSVC-NEXT: ImportAddressTableRVA: 0x2050
38+
// CHECK-MSVC-NEXT: }
39+
2540

2641
#--- main.s
2742
.global entry
2843
entry:
2944
call func
3045
ret
3146

47+
#--- main-nocall.s
48+
.global entry
49+
entry:
50+
ret
51+
3252
#--- lib.def
3353
LIBRARY lib.dll
3454
EXPORTS

‎llvm/include/llvm/Object/COFF.h‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -383,8 +383,8 @@ class COFFSymbolRef {
383383
}
384384

385385
bool isCommon() const {
386-
return (isExternal() || isSection()) &&
387-
getSectionNumber() == COFF::IMAGE_SYM_UNDEFINED && getValue() != 0;
386+
return isExternal() && getSectionNumber() == COFF::IMAGE_SYM_UNDEFINED &&
387+
getValue() != 0;
388388
}
389389

390390
bool isUndefined() const {
@@ -393,8 +393,7 @@ class COFFSymbolRef {
393393
}
394394

395395
bool isEmptySectionDeclaration() const {
396-
return isSection() && getSectionNumber() == COFF::IMAGE_SYM_UNDEFINED &&
397-
getValue() == 0;
396+
return isSection() && getSectionNumber() == COFF::IMAGE_SYM_UNDEFINED;
398397
}
399398

400399
bool isWeakExternal() const {

‎llvm/test/Object/coff-sec-sym.test‎

Lines changed: 0 additions & 20 deletions
This file was deleted.

0 commit comments

Comments
 (0)