[LLD][COFF] Prevent to emit relocations for discarded weak wrapped symbols #156214
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
|
@llvm/pr-subscribers-lld-coff Author: Tomohiro Kashiwada (kikairoya) ChangesFixes #150739 Full diff: https://github.com/llvm/llvm-project/pull/156214.diff 5 Files Affected:
diff --git a/lld/COFF/Chunks.cpp b/lld/COFF/Chunks.cpp
index 39fc25047f3b1..cb5cba5c414a1 100644
--- a/lld/COFF/Chunks.cpp
+++ b/lld/COFF/Chunks.cpp
@@ -564,7 +564,7 @@ void SectionChunk::getBaserels(std::vector<Baserel> *res) {
if (ty == IMAGE_REL_BASED_ABSOLUTE)
continue;
Symbol *target = file->getSymbol(rel.SymbolTableIndex);
- if (!target || isa<DefinedAbsolute>(target))
+ if (!isa_and_nonnull<Defined>(target) || isa<DefinedAbsolute>(target))
continue;
res->emplace_back(rva + rel.VirtualAddress, ty);
}
diff --git a/lld/COFF/MinGW.cpp b/lld/COFF/MinGW.cpp
index e7117cbea2e88..7c6f4281145ce 100644
--- a/lld/COFF/MinGW.cpp
+++ b/lld/COFF/MinGW.cpp
@@ -233,6 +233,17 @@ void lld::coff::addWrappedSymbols(SymbolTable &symtab,
symtab.addUndefined(mangle("__wrap_" + name, symtab.machine));
v.push_back({sym, real, wrap});
+ if (auto *usym = dyn_cast<Undefined>(sym)) {
+ if (auto *ureal = dyn_cast<Undefined>(real);
+ ureal && ureal->weakAlias && !usym->weakAlias) {
+ usym->weakAlias = ureal->weakAlias;
+ }
+ if (auto *uwrap = dyn_cast<Undefined>(wrap);
+ uwrap && usym->weakAlias && !uwrap->weakAlias) {
+ uwrap->weakAlias = usym->weakAlias;
+ }
+ }
+
// These symbols may seem undefined initially, but don't bail out
// at symtab.reportUnresolvable() due to them, but let wrapSymbols
// below sort things out before checking finally with
@@ -269,10 +280,15 @@ void lld::coff::wrapSymbols(SymbolTable &symtab) {
// (We can't easily distinguish whether any object file actually
// referenced it or not, though.)
if (imp) {
- DefinedLocalImport *wrapimp = make<DefinedLocalImport>(
- symtab.ctx, saver().save("__imp_" + w.wrap->getName()), d);
- symtab.localImportChunks.push_back(wrapimp->getChunk());
- map[imp] = wrapimp;
+ if (Symbol *wrapimp =
+ symtab.find(("__imp_" + w.wrap->getName()).str())) {
+ map[imp] = wrapimp;
+ } else {
+ DefinedLocalImport *localwrapimp = make<DefinedLocalImport>(
+ symtab.ctx, saver().save("__imp_" + w.wrap->getName()), d);
+ symtab.localImportChunks.push_back(localwrapimp->getChunk());
+ map[imp] = localwrapimp;
+ }
}
}
}
diff --git a/lld/test/COFF/reloc-undefined-weak.s b/lld/test/COFF/reloc-undefined-weak.s
new file mode 100644
index 0000000000000..5f9baa7a3dfb0
--- /dev/null
+++ b/lld/test/COFF/reloc-undefined-weak.s
@@ -0,0 +1,52 @@
+// REQUIRES: x86
+
+// Check that base-relocations for unresolved weak symbols will be omitted.
+
+// RUN: rm -rf %t.dir && split-file %s %t.dir && cd %t.dir
+// RUN: llvm-mc -filetype=obj -triple=x86_64-mingw main.s -o main.o
+// RUN: llvm-mc -filetype=obj -triple=x86_64-mingw other.s -o other.o
+
+// RUN: ld.lld -m i386pep -dll -o other.dll other.o -entry= --export-all-symbols --out-implib other.dll.a
+// RUN: ld.lld -m i386pep -o main.exe main.o other.dll.a -e entry --wrap foo --verbose
+// RUN: llvm-readobj --sections --symbols --coff-imports --coff-basereloc main.exe | FileCheck %s --implicit-check-not=other.dll
+
+// CHECK: Number: 4
+// CHECK-NEXT: Name: .data
+// CHECK-NEXT: VirtualSize:
+// CHECK-NEXT: VirtualAddress: 0x[[#%x,SECTOP:0x4000]]
+// CHECK: Name: ref_foo
+// CHECK-NEXT: Value: [[#%d,SYMVAL:]]
+// CHECK: BaseReloc [
+// CHECK-NOT: Address: 0x[[#%x,SECTOP+SYMVAL]]
+
+#--- main.s
+.global entry
+entry:
+ movq ref_foo(%rip), %rax
+ call *%rax
+
+.global __wrap_foo
+__wrap_foo:
+ ret
+
+.data
+.global ref_foo
+.p2align 3
+ref_foo:
+ .quad __real_foo
+
+.globl _pei386_runtime_relocator
+_pei386_runtime_relocator:
+ movl __RUNTIME_PSEUDO_RELOC_LIST__(%rip), %eax
+ movl __RUNTIME_PSEUDO_RELOC_LIST_END__(%rip), %eax
+
+.weak __real_foo
+.addrsig
+.addrsig_sym __real_foo
+.addrsig_sym ref_foo
+
+#--- other.s
+.global foo
+
+foo:
+ ret
diff --git a/lld/test/COFF/wrap-dllimport.s b/lld/test/COFF/wrap-dllimport.s
index d7662b29fdc78..a1b4521fd75fa 100644
--- a/lld/test/COFF/wrap-dllimport.s
+++ b/lld/test/COFF/wrap-dllimport.s
@@ -1,42 +1,59 @@
// REQUIRES: x86
// Check that we can wrap a dllimported symbol, so that references to
-// __imp_<symbol> gets redirected to a defined local import instead.
+// __imp_<symbol> gets redirected to a symbol already exists or a defined local import instead
// RUN: split-file %s %t.dir
// RUN: llvm-mc -filetype=obj -triple=i686-win32-gnu %t.dir/main.s -o %t.main.obj
// RUN: llvm-mc -filetype=obj -triple=i686-win32-gnu %t.dir/other.s -o %t.other.obj
-// RUN: lld-link -dll -out:%t.dll %t.other.obj -noentry -safeseh:no -export:foo -implib:%t.lib
-// RUN: lld-link -out:%t.exe %t.main.obj %t.lib -entry:entry -subsystem:console -debug:symtab -safeseh:no -wrap:foo -lldmap:%t.map
+// RUN: lld-link -dll -out:%t.dll %t.other.obj -noentry -safeseh:no -export:foo -export:bar -implib:%t.lib
+// RUN: lld-link -out:%t.exe %t.main.obj %t.lib -entry:entry -subsystem:console -debug:symtab -safeseh:no -wrap:foo -wrap:bar -lldmap:%t.map
// RUN: llvm-objdump -s -d --print-imm-hex %t.exe | FileCheck %s
// CHECK: Contents of section .rdata:
-// CHECK-NEXT: 402000 06104000
+// CHECK-NEXT: 402000 0c104000
// CHECK: Disassembly of section .text:
// CHECK-EMPTY:
// CHECK: 00401000 <_entry>:
// CHECK-NEXT: 401000: ff 25 00 20 40 00 jmpl *0x402000
+// CHECK-NEXT: 401006: ff 25 00 00 00 00 jmpl *0x0
// CHECK-EMPTY:
-// CHECK-NEXT: 00401006 <___wrap_foo>:
-// CHECK-NEXT: 401006: c3 retl
+// CHECK-NEXT: 0040100c <___wrap_foo>:
+// CHECK-NEXT: 40100c: c3 retl
+// CHECK-EMPTY:
+// CHECK-NEXT: 0040100d <___wrap_bar>:
+// CHECK-NEXT: 40100d: c3 retl
-// The jmpl instruction in _entry points at an address in 0x402000,
+// The first jmpl instruction in _entry points at an address in 0x402000,
// which is the first 4 bytes of the .rdata section (above), which is a
// pointer that points at ___wrap_foo.
+// The second jmpl instruction in _entry points the null since the referenced symbol
+// `__imp____wrap_bar` is declared as a weak reference to prevent pull a reference
+// from an external DLL.
+
#--- main.s
.global _entry
_entry:
jmpl *__imp__foo
+ jmpl *__imp__bar
.global ___wrap_foo
___wrap_foo:
ret
+.weak __imp____wrap_bar
+.global ___wrap_bar
+___wrap_bar:
+ ret
+
#--- other.s
.global _foo
-
_foo:
ret
+
+.global _bar
+_bar:
+ ret
diff --git a/lld/test/COFF/wrap-inherit-weakness.ll b/lld/test/COFF/wrap-inherit-weakness.ll
new file mode 100644
index 0000000000000..e050e8fda37f2
--- /dev/null
+++ b/lld/test/COFF/wrap-inherit-weakness.ll
@@ -0,0 +1,16 @@
+; REQUIRES: x86
+
+; Check that 'weak' attribute will be inherited to the wrapped symbols.
+
+; RUN: llc %s -mtriple x86_64-mingw -o %t.o --filetype=obj
+; RUN: ld.lld -m i386pep -shared -o %t.dll %t.o --entry= --wrap fn
+
+declare extern_weak dso_local void @__real_fn() nounwind
+declare dso_local void @fn() nounwind
+declare dso_local void @__wrap_fn() nounwind
+
+define dllexport void @caller() nounwind {
+ call void @__real_fn()
+ call void @fn()
+ ret void
+}
|
Member
|
@llvm/pr-subscribers-lld Author: Tomohiro Kashiwada (kikairoya) ChangesFixes #150739 Full diff: https://github.com/llvm/llvm-project/pull/156214.diff 5 Files Affected:
diff --git a/lld/COFF/Chunks.cpp b/lld/COFF/Chunks.cpp
index 39fc25047f3b1..cb5cba5c414a1 100644
--- a/lld/COFF/Chunks.cpp
+++ b/lld/COFF/Chunks.cpp
@@ -564,7 +564,7 @@ void SectionChunk::getBaserels(std::vector<Baserel> *res) {
if (ty == IMAGE_REL_BASED_ABSOLUTE)
continue;
Symbol *target = file->getSymbol(rel.SymbolTableIndex);
- if (!target || isa<DefinedAbsolute>(target))
+ if (!isa_and_nonnull<Defined>(target) || isa<DefinedAbsolute>(target))
continue;
res->emplace_back(rva + rel.VirtualAddress, ty);
}
diff --git a/lld/COFF/MinGW.cpp b/lld/COFF/MinGW.cpp
index e7117cbea2e88..7c6f4281145ce 100644
--- a/lld/COFF/MinGW.cpp
+++ b/lld/COFF/MinGW.cpp
@@ -233,6 +233,17 @@ void lld::coff::addWrappedSymbols(SymbolTable &symtab,
symtab.addUndefined(mangle("__wrap_" + name, symtab.machine));
v.push_back({sym, real, wrap});
+ if (auto *usym = dyn_cast<Undefined>(sym)) {
+ if (auto *ureal = dyn_cast<Undefined>(real);
+ ureal && ureal->weakAlias && !usym->weakAlias) {
+ usym->weakAlias = ureal->weakAlias;
+ }
+ if (auto *uwrap = dyn_cast<Undefined>(wrap);
+ uwrap && usym->weakAlias && !uwrap->weakAlias) {
+ uwrap->weakAlias = usym->weakAlias;
+ }
+ }
+
// These symbols may seem undefined initially, but don't bail out
// at symtab.reportUnresolvable() due to them, but let wrapSymbols
// below sort things out before checking finally with
@@ -269,10 +280,15 @@ void lld::coff::wrapSymbols(SymbolTable &symtab) {
// (We can't easily distinguish whether any object file actually
// referenced it or not, though.)
if (imp) {
- DefinedLocalImport *wrapimp = make<DefinedLocalImport>(
- symtab.ctx, saver().save("__imp_" + w.wrap->getName()), d);
- symtab.localImportChunks.push_back(wrapimp->getChunk());
- map[imp] = wrapimp;
+ if (Symbol *wrapimp =
+ symtab.find(("__imp_" + w.wrap->getName()).str())) {
+ map[imp] = wrapimp;
+ } else {
+ DefinedLocalImport *localwrapimp = make<DefinedLocalImport>(
+ symtab.ctx, saver().save("__imp_" + w.wrap->getName()), d);
+ symtab.localImportChunks.push_back(localwrapimp->getChunk());
+ map[imp] = localwrapimp;
+ }
}
}
}
diff --git a/lld/test/COFF/reloc-undefined-weak.s b/lld/test/COFF/reloc-undefined-weak.s
new file mode 100644
index 0000000000000..5f9baa7a3dfb0
--- /dev/null
+++ b/lld/test/COFF/reloc-undefined-weak.s
@@ -0,0 +1,52 @@
+// REQUIRES: x86
+
+// Check that base-relocations for unresolved weak symbols will be omitted.
+
+// RUN: rm -rf %t.dir && split-file %s %t.dir && cd %t.dir
+// RUN: llvm-mc -filetype=obj -triple=x86_64-mingw main.s -o main.o
+// RUN: llvm-mc -filetype=obj -triple=x86_64-mingw other.s -o other.o
+
+// RUN: ld.lld -m i386pep -dll -o other.dll other.o -entry= --export-all-symbols --out-implib other.dll.a
+// RUN: ld.lld -m i386pep -o main.exe main.o other.dll.a -e entry --wrap foo --verbose
+// RUN: llvm-readobj --sections --symbols --coff-imports --coff-basereloc main.exe | FileCheck %s --implicit-check-not=other.dll
+
+// CHECK: Number: 4
+// CHECK-NEXT: Name: .data
+// CHECK-NEXT: VirtualSize:
+// CHECK-NEXT: VirtualAddress: 0x[[#%x,SECTOP:0x4000]]
+// CHECK: Name: ref_foo
+// CHECK-NEXT: Value: [[#%d,SYMVAL:]]
+// CHECK: BaseReloc [
+// CHECK-NOT: Address: 0x[[#%x,SECTOP+SYMVAL]]
+
+#--- main.s
+.global entry
+entry:
+ movq ref_foo(%rip), %rax
+ call *%rax
+
+.global __wrap_foo
+__wrap_foo:
+ ret
+
+.data
+.global ref_foo
+.p2align 3
+ref_foo:
+ .quad __real_foo
+
+.globl _pei386_runtime_relocator
+_pei386_runtime_relocator:
+ movl __RUNTIME_PSEUDO_RELOC_LIST__(%rip), %eax
+ movl __RUNTIME_PSEUDO_RELOC_LIST_END__(%rip), %eax
+
+.weak __real_foo
+.addrsig
+.addrsig_sym __real_foo
+.addrsig_sym ref_foo
+
+#--- other.s
+.global foo
+
+foo:
+ ret
diff --git a/lld/test/COFF/wrap-dllimport.s b/lld/test/COFF/wrap-dllimport.s
index d7662b29fdc78..a1b4521fd75fa 100644
--- a/lld/test/COFF/wrap-dllimport.s
+++ b/lld/test/COFF/wrap-dllimport.s
@@ -1,42 +1,59 @@
// REQUIRES: x86
// Check that we can wrap a dllimported symbol, so that references to
-// __imp_<symbol> gets redirected to a defined local import instead.
+// __imp_<symbol> gets redirected to a symbol already exists or a defined local import instead
// RUN: split-file %s %t.dir
// RUN: llvm-mc -filetype=obj -triple=i686-win32-gnu %t.dir/main.s -o %t.main.obj
// RUN: llvm-mc -filetype=obj -triple=i686-win32-gnu %t.dir/other.s -o %t.other.obj
-// RUN: lld-link -dll -out:%t.dll %t.other.obj -noentry -safeseh:no -export:foo -implib:%t.lib
-// RUN: lld-link -out:%t.exe %t.main.obj %t.lib -entry:entry -subsystem:console -debug:symtab -safeseh:no -wrap:foo -lldmap:%t.map
+// RUN: lld-link -dll -out:%t.dll %t.other.obj -noentry -safeseh:no -export:foo -export:bar -implib:%t.lib
+// RUN: lld-link -out:%t.exe %t.main.obj %t.lib -entry:entry -subsystem:console -debug:symtab -safeseh:no -wrap:foo -wrap:bar -lldmap:%t.map
// RUN: llvm-objdump -s -d --print-imm-hex %t.exe | FileCheck %s
// CHECK: Contents of section .rdata:
-// CHECK-NEXT: 402000 06104000
+// CHECK-NEXT: 402000 0c104000
// CHECK: Disassembly of section .text:
// CHECK-EMPTY:
// CHECK: 00401000 <_entry>:
// CHECK-NEXT: 401000: ff 25 00 20 40 00 jmpl *0x402000
+// CHECK-NEXT: 401006: ff 25 00 00 00 00 jmpl *0x0
// CHECK-EMPTY:
-// CHECK-NEXT: 00401006 <___wrap_foo>:
-// CHECK-NEXT: 401006: c3 retl
+// CHECK-NEXT: 0040100c <___wrap_foo>:
+// CHECK-NEXT: 40100c: c3 retl
+// CHECK-EMPTY:
+// CHECK-NEXT: 0040100d <___wrap_bar>:
+// CHECK-NEXT: 40100d: c3 retl
-// The jmpl instruction in _entry points at an address in 0x402000,
+// The first jmpl instruction in _entry points at an address in 0x402000,
// which is the first 4 bytes of the .rdata section (above), which is a
// pointer that points at ___wrap_foo.
+// The second jmpl instruction in _entry points the null since the referenced symbol
+// `__imp____wrap_bar` is declared as a weak reference to prevent pull a reference
+// from an external DLL.
+
#--- main.s
.global _entry
_entry:
jmpl *__imp__foo
+ jmpl *__imp__bar
.global ___wrap_foo
___wrap_foo:
ret
+.weak __imp____wrap_bar
+.global ___wrap_bar
+___wrap_bar:
+ ret
+
#--- other.s
.global _foo
-
_foo:
ret
+
+.global _bar
+_bar:
+ ret
diff --git a/lld/test/COFF/wrap-inherit-weakness.ll b/lld/test/COFF/wrap-inherit-weakness.ll
new file mode 100644
index 0000000000000..e050e8fda37f2
--- /dev/null
+++ b/lld/test/COFF/wrap-inherit-weakness.ll
@@ -0,0 +1,16 @@
+; REQUIRES: x86
+
+; Check that 'weak' attribute will be inherited to the wrapped symbols.
+
+; RUN: llc %s -mtriple x86_64-mingw -o %t.o --filetype=obj
+; RUN: ld.lld -m i386pep -shared -o %t.dll %t.o --entry= --wrap fn
+
+declare extern_weak dso_local void @__real_fn() nounwind
+declare dso_local void @fn() nounwind
+declare dso_local void @__wrap_fn() nounwind
+
+define dllexport void @caller() nounwind {
+ call void @__real_fn()
+ call void @fn()
+ ret void
+}
|
Member
|
@llvm/pr-subscribers-platform-windows Author: Tomohiro Kashiwada (kikairoya) ChangesFixes #150739 Full diff: https://github.com/llvm/llvm-project/pull/156214.diff 5 Files Affected:
diff --git a/lld/COFF/Chunks.cpp b/lld/COFF/Chunks.cpp
index 39fc25047f3b1..cb5cba5c414a1 100644
--- a/lld/COFF/Chunks.cpp
+++ b/lld/COFF/Chunks.cpp
@@ -564,7 +564,7 @@ void SectionChunk::getBaserels(std::vector<Baserel> *res) {
if (ty == IMAGE_REL_BASED_ABSOLUTE)
continue;
Symbol *target = file->getSymbol(rel.SymbolTableIndex);
- if (!target || isa<DefinedAbsolute>(target))
+ if (!isa_and_nonnull<Defined>(target) || isa<DefinedAbsolute>(target))
continue;
res->emplace_back(rva + rel.VirtualAddress, ty);
}
diff --git a/lld/COFF/MinGW.cpp b/lld/COFF/MinGW.cpp
index e7117cbea2e88..7c6f4281145ce 100644
--- a/lld/COFF/MinGW.cpp
+++ b/lld/COFF/MinGW.cpp
@@ -233,6 +233,17 @@ void lld::coff::addWrappedSymbols(SymbolTable &symtab,
symtab.addUndefined(mangle("__wrap_" + name, symtab.machine));
v.push_back({sym, real, wrap});
+ if (auto *usym = dyn_cast<Undefined>(sym)) {
+ if (auto *ureal = dyn_cast<Undefined>(real);
+ ureal && ureal->weakAlias && !usym->weakAlias) {
+ usym->weakAlias = ureal->weakAlias;
+ }
+ if (auto *uwrap = dyn_cast<Undefined>(wrap);
+ uwrap && usym->weakAlias && !uwrap->weakAlias) {
+ uwrap->weakAlias = usym->weakAlias;
+ }
+ }
+
// These symbols may seem undefined initially, but don't bail out
// at symtab.reportUnresolvable() due to them, but let wrapSymbols
// below sort things out before checking finally with
@@ -269,10 +280,15 @@ void lld::coff::wrapSymbols(SymbolTable &symtab) {
// (We can't easily distinguish whether any object file actually
// referenced it or not, though.)
if (imp) {
- DefinedLocalImport *wrapimp = make<DefinedLocalImport>(
- symtab.ctx, saver().save("__imp_" + w.wrap->getName()), d);
- symtab.localImportChunks.push_back(wrapimp->getChunk());
- map[imp] = wrapimp;
+ if (Symbol *wrapimp =
+ symtab.find(("__imp_" + w.wrap->getName()).str())) {
+ map[imp] = wrapimp;
+ } else {
+ DefinedLocalImport *localwrapimp = make<DefinedLocalImport>(
+ symtab.ctx, saver().save("__imp_" + w.wrap->getName()), d);
+ symtab.localImportChunks.push_back(localwrapimp->getChunk());
+ map[imp] = localwrapimp;
+ }
}
}
}
diff --git a/lld/test/COFF/reloc-undefined-weak.s b/lld/test/COFF/reloc-undefined-weak.s
new file mode 100644
index 0000000000000..5f9baa7a3dfb0
--- /dev/null
+++ b/lld/test/COFF/reloc-undefined-weak.s
@@ -0,0 +1,52 @@
+// REQUIRES: x86
+
+// Check that base-relocations for unresolved weak symbols will be omitted.
+
+// RUN: rm -rf %t.dir && split-file %s %t.dir && cd %t.dir
+// RUN: llvm-mc -filetype=obj -triple=x86_64-mingw main.s -o main.o
+// RUN: llvm-mc -filetype=obj -triple=x86_64-mingw other.s -o other.o
+
+// RUN: ld.lld -m i386pep -dll -o other.dll other.o -entry= --export-all-symbols --out-implib other.dll.a
+// RUN: ld.lld -m i386pep -o main.exe main.o other.dll.a -e entry --wrap foo --verbose
+// RUN: llvm-readobj --sections --symbols --coff-imports --coff-basereloc main.exe | FileCheck %s --implicit-check-not=other.dll
+
+// CHECK: Number: 4
+// CHECK-NEXT: Name: .data
+// CHECK-NEXT: VirtualSize:
+// CHECK-NEXT: VirtualAddress: 0x[[#%x,SECTOP:0x4000]]
+// CHECK: Name: ref_foo
+// CHECK-NEXT: Value: [[#%d,SYMVAL:]]
+// CHECK: BaseReloc [
+// CHECK-NOT: Address: 0x[[#%x,SECTOP+SYMVAL]]
+
+#--- main.s
+.global entry
+entry:
+ movq ref_foo(%rip), %rax
+ call *%rax
+
+.global __wrap_foo
+__wrap_foo:
+ ret
+
+.data
+.global ref_foo
+.p2align 3
+ref_foo:
+ .quad __real_foo
+
+.globl _pei386_runtime_relocator
+_pei386_runtime_relocator:
+ movl __RUNTIME_PSEUDO_RELOC_LIST__(%rip), %eax
+ movl __RUNTIME_PSEUDO_RELOC_LIST_END__(%rip), %eax
+
+.weak __real_foo
+.addrsig
+.addrsig_sym __real_foo
+.addrsig_sym ref_foo
+
+#--- other.s
+.global foo
+
+foo:
+ ret
diff --git a/lld/test/COFF/wrap-dllimport.s b/lld/test/COFF/wrap-dllimport.s
index d7662b29fdc78..a1b4521fd75fa 100644
--- a/lld/test/COFF/wrap-dllimport.s
+++ b/lld/test/COFF/wrap-dllimport.s
@@ -1,42 +1,59 @@
// REQUIRES: x86
// Check that we can wrap a dllimported symbol, so that references to
-// __imp_<symbol> gets redirected to a defined local import instead.
+// __imp_<symbol> gets redirected to a symbol already exists or a defined local import instead
// RUN: split-file %s %t.dir
// RUN: llvm-mc -filetype=obj -triple=i686-win32-gnu %t.dir/main.s -o %t.main.obj
// RUN: llvm-mc -filetype=obj -triple=i686-win32-gnu %t.dir/other.s -o %t.other.obj
-// RUN: lld-link -dll -out:%t.dll %t.other.obj -noentry -safeseh:no -export:foo -implib:%t.lib
-// RUN: lld-link -out:%t.exe %t.main.obj %t.lib -entry:entry -subsystem:console -debug:symtab -safeseh:no -wrap:foo -lldmap:%t.map
+// RUN: lld-link -dll -out:%t.dll %t.other.obj -noentry -safeseh:no -export:foo -export:bar -implib:%t.lib
+// RUN: lld-link -out:%t.exe %t.main.obj %t.lib -entry:entry -subsystem:console -debug:symtab -safeseh:no -wrap:foo -wrap:bar -lldmap:%t.map
// RUN: llvm-objdump -s -d --print-imm-hex %t.exe | FileCheck %s
// CHECK: Contents of section .rdata:
-// CHECK-NEXT: 402000 06104000
+// CHECK-NEXT: 402000 0c104000
// CHECK: Disassembly of section .text:
// CHECK-EMPTY:
// CHECK: 00401000 <_entry>:
// CHECK-NEXT: 401000: ff 25 00 20 40 00 jmpl *0x402000
+// CHECK-NEXT: 401006: ff 25 00 00 00 00 jmpl *0x0
// CHECK-EMPTY:
-// CHECK-NEXT: 00401006 <___wrap_foo>:
-// CHECK-NEXT: 401006: c3 retl
+// CHECK-NEXT: 0040100c <___wrap_foo>:
+// CHECK-NEXT: 40100c: c3 retl
+// CHECK-EMPTY:
+// CHECK-NEXT: 0040100d <___wrap_bar>:
+// CHECK-NEXT: 40100d: c3 retl
-// The jmpl instruction in _entry points at an address in 0x402000,
+// The first jmpl instruction in _entry points at an address in 0x402000,
// which is the first 4 bytes of the .rdata section (above), which is a
// pointer that points at ___wrap_foo.
+// The second jmpl instruction in _entry points the null since the referenced symbol
+// `__imp____wrap_bar` is declared as a weak reference to prevent pull a reference
+// from an external DLL.
+
#--- main.s
.global _entry
_entry:
jmpl *__imp__foo
+ jmpl *__imp__bar
.global ___wrap_foo
___wrap_foo:
ret
+.weak __imp____wrap_bar
+.global ___wrap_bar
+___wrap_bar:
+ ret
+
#--- other.s
.global _foo
-
_foo:
ret
+
+.global _bar
+_bar:
+ ret
diff --git a/lld/test/COFF/wrap-inherit-weakness.ll b/lld/test/COFF/wrap-inherit-weakness.ll
new file mode 100644
index 0000000000000..e050e8fda37f2
--- /dev/null
+++ b/lld/test/COFF/wrap-inherit-weakness.ll
@@ -0,0 +1,16 @@
+; REQUIRES: x86
+
+; Check that 'weak' attribute will be inherited to the wrapped symbols.
+
+; RUN: llc %s -mtriple x86_64-mingw -o %t.o --filetype=obj
+; RUN: ld.lld -m i386pep -shared -o %t.dll %t.o --entry= --wrap fn
+
+declare extern_weak dso_local void @__real_fn() nounwind
+declare dso_local void @fn() nounwind
+declare dso_local void @__wrap_fn() nounwind
+
+define dllexport void @caller() nounwind {
+ call void @__real_fn()
+ call void @fn()
+ ret void
+}
|
Contributor
|
It looks like it's still pulling in all the vs this with bfd Test source (no defines)#include <stdio.h>
#include <new>
#include <string>
#ifdef OVERRIDE_NEW_DELETE
#define WIN32_LEAN_AND_MEAN
#include <windows.h>
void *operator new (std::size_t sz)
{
printf ("In new\n");
return LocalAlloc (LMEM_FIXED, sz);
}
void operator delete (void *p)
{
printf ("In delete\n");
LocalFree (p);
}
void operator delete (void *p, std::size_t)
{
printf ("In delete sz\n");
LocalFree (p);
}
#endif
struct alignas(32) foo {};
int main (void)
{
char *p = new char (0); // _Znwm
printf ("%p\n", p);
delete p; // _ZdlPvm
foo *bar = new foo; // _ZnwmSt11align_val_t
printf ("%p\n", bar);
delete bar; // _ZdlPvmSt11align_val_t
{
std::string foo (1024, 'x');
printf ("%p\n", foo.data ());
}
return 0;
} |
Contributor
Author
|
That looks a further different problem. The |
Contributor
|
Hmm. How about LLD: (gdb) b _cygwin_crt0_common
Breakpoint 2 at 0x7ff6f37e12a0: file /usr/src/debug/cygwin-3.7.0-0.287.g3a03874f73db/winsup/cygwin/lib/_cygwin_crt0_common.cc, line 124.
(gdb) r
Thread 1 hit Breakpoint 2, _cygwin_crt0_common (f=0x7ff6d15b1080 <main()>,
u=u@entry=0x0)
at /usr/src/debug/cygwin-3.7.0-0.287.g3a03874f73db/winsup/cygwin/lib/_cygwin_crt0_common.cc:124
124 {
(gdb) p __cygwin_cxx_malloc
$1 = {oper_new = 0x7ff6d15b11d8 <operator new(unsigned long)>,
oper_new__ = 0x0, oper_delete = 0x0, oper_delete__ = 0x0, oper_new_nt = 0x0,
oper_new___nt = 0x0, oper_delete_nt = 0x0, oper_delete___nt = 0x0,
oper_delete_sz = 0x7ff6d15b11e8 <operator delete(void*, unsigned long)>,
oper_delete___sz = 0x0,
oper_new_al = 0x7ff6d15b11d0 <operator new(unsigned long, std::align_val_t)>, oper_new___al = 0x0, oper_delete_al = 0x0, oper_delete___al = 0x0,
oper_delete_sz_al = 0x7ff6d15b11e0 <operator delete(void*, unsigned long, std::align_val_t)>, oper_delete___sz_al = 0x0, oper_new_al_nt = 0x0,
oper_new___al_nt = 0x0, oper_delete_al_nt = 0x0, oper_delete___al_nt = 0x0}BFD: (gdb) b _cygwin_crt0_common
Breakpoint 1 at 0x1004012c0: file /usr/src/debug/cygwin-3.7.0-0.287.g3a03874f73db/winsup/cygwin/lib/_cygwin_crt0_common.cc, line 124.
(gdb) r
Thread 1 hit Breakpoint 1, _cygwin_crt0_common (f=0x100401080 <main()>,
u=u@entry=0x0)
at /usr/src/debug/cygwin-3.7.0-0.287.g3a03874f73db/winsup/cygwin/lib/_cygwin_crt0_common.cc:124
124 {
(gdb) p __cygwin_cxx_malloc
$1 = {oper_new = 0x0, oper_new__ = 0x0, oper_delete = 0x0,
oper_delete__ = 0x0, oper_new_nt = 0x0, oper_new___nt = 0x0,
oper_delete_nt = 0x0, oper_delete___nt = 0x0, oper_delete_sz = 0x0,
oper_delete___sz = 0x0, oper_new_al = 0x0, oper_new___al = 0x0,
oper_delete_al = 0x0, oper_delete___al = 0x0, oper_delete_sz_al = 0x0,
oper_delete___sz_al = 0x0, oper_new_al_nt = 0x0, oper_new___al_nt = 0x0,
oper_delete_al_nt = 0x0, oper_delete___al_nt = 0x0}Meaning, Cygwin will think the exe is overriding these operators, when in fact they are just the stubs from linking cygstdc++-6.dll (I am actually kind of surprised about BFD behavior here, but I guess it makes sense because the unadorned operators are not referenced just the wrapped ones?) |
Contributor
|
But at least it's not crashing anymore, which is definitely an improvement! |
mstorsjo
reviewed
Sep 3, 2025
Member
mstorsjo
left a comment
mstorsjo
left a comment
There was a problem hiding this comment.
The code looks reasonable and kinda ok, but I'm not entirely wrapping my head around all what's happening here, so I have a bunch of questions.
lld/test/COFF/wrap-dllimport.s
Outdated
|
|
||
| // Check that we can wrap a dllimported symbol, so that references to | ||
| // __imp_<symbol> gets redirected to a defined local import instead. | ||
| // __imp_<symbol> gets redirected to a symbol already exists or a defined local import instead |
Member
There was a problem hiding this comment.
This comment seems to lack some word to be proper English - "to a symbol that already exists", or something like that?
lld/test/COFF/wrap-dllimport.s
Outdated
| // which is the first 4 bytes of the .rdata section (above), which is a | ||
| // pointer that points at ___wrap_foo. | ||
|
|
||
| // The second jmpl instruction in _entry points the null since the referenced symbol |
Member
There was a problem hiding this comment.
Please rephrase this sentence, "points the null" also seems to lack a couple words, or has some typos.
lld/test/COFF/reloc-undefined-weak.s
Outdated
| // RUN: llvm-mc -filetype=obj -triple=x86_64-mingw main.s -o main.o | ||
| // RUN: llvm-mc -filetype=obj -triple=x86_64-mingw other.s -o other.o | ||
|
|
||
| // RUN: ld.lld -m i386pep -dll -o other.dll other.o -entry= --export-all-symbols --out-implib other.dll.a |
Member
There was a problem hiding this comment.
We generally don't use the ld.lld interface in lld/test/COFF, but only use the direct lld-link interface - even if it understandably would make more sense to use the GNU ld-like interface for being able to crosscheck things with GNU ld.
| @@ -0,0 +1,16 @@ | |||
| ; REQUIRES: x86 | |||
|
|
|||
| ; Check that 'weak' attribute will be inherited to the wrapped symbols. | |||
Member
There was a problem hiding this comment.
Can you elaborate a bit more about what happens here? I don't quite understand the involved mechanics here.
|
|
||
| declare extern_weak dso_local void @__real_fn() nounwind | ||
| declare dso_local void @fn() nounwind | ||
| declare dso_local void @__wrap_fn() nounwind |
Member
There was a problem hiding this comment.
Is __wrap_fn also treated as weak, as __real_fn is weak, or what's the relation here? Wouldn't we otherwise have undefined references as the reference to fn gets redirected to __wrap_fn?
|
|
||
| define dllexport void @caller() nounwind { | ||
| call void @__real_fn() | ||
| call void @fn() |
Member
There was a problem hiding this comment.
If these symbols are weak, and we expect that they may be null after linking, wouldn't it be better to do the equivalent of this:
if (__real_fn)
__real_fn();
?
This raises the question about how the call to fn() works here; as fn() isn't declared as weak on the C/IR level, wouldn't an if (fn) fn() be considered redundant as fn can be assumed to be non-null, as it isn't declared weak?
Contributor
Author
There was a problem hiding this comment.
I completely missed that problem. Thank you for catching that.
This part of the change is supposed to prevent pulling unused __wrap__Znwm family, but it doesn't work, as @jeremyd2019 said. This is because the unwrapped _Znwm family is always LazyArchiveKind.
Therefore, I'd drop this part.
Contributor
Author
|
Thanks! |
jeremyd2019
pushed a commit
to jeremyd2019/llvm-project
that referenced
this pull request
Sep 9, 2025
…mbols (llvm#156214) When a symbol is imported from a DLL, a base relocation record is normally emitted. However, if the import is pulled in via a wrapped symbol (using `--wrap`) and later dropped because it is only referenced weakly, a dangling base relocation remains in the output. At runtime, this relocation changes the symbol value from null to a garbage pointer. This patch adds checks to avoid emitting relocation records for non-`Defined` symbols, and to prevent creating an auto-import entry if an import entry for the wrapped symbol already exists. Fixes llvm#150739 (cherry picked from commit e976622)
jeremyd2019
pushed a commit
to jeremyd2019/llvm-project
that referenced
this pull request
Sep 12, 2025
…mbols (llvm#156214) When a symbol is imported from a DLL, a base relocation record is normally emitted. However, if the import is pulled in via a wrapped symbol (using `--wrap`) and later dropped because it is only referenced weakly, a dangling base relocation remains in the output. At runtime, this relocation changes the symbol value from null to a garbage pointer. This patch adds checks to avoid emitting relocation records for non-`Defined` symbols, and to prevent creating an auto-import entry if an import entry for the wrapped symbol already exists. Fixes llvm#150739 (cherry picked from commit e976622)
jeremyd2019
pushed a commit
to jeremyd2019/llvm-project
that referenced
this pull request
Sep 23, 2025
…mbols (llvm#156214) When a symbol is imported from a DLL, a base relocation record is normally emitted. However, if the import is pulled in via a wrapped symbol (using `--wrap`) and later dropped because it is only referenced weakly, a dangling base relocation remains in the output. At runtime, this relocation changes the symbol value from null to a garbage pointer. This patch adds checks to avoid emitting relocation records for non-`Defined` symbols, and to prevent creating an auto-import entry if an import entry for the wrapped symbol already exists. Fixes llvm#150739 (cherry picked from commit e976622)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When a symbol is imported from a DLL, a base relocation record is normally emitted.
However, if the import is pulled in via a wrapped symbol (using
--wrap) and later dropped because it is only referenced weakly, a dangling base relocation remains in the output.At runtime, this relocation changes the symbol value from null to a garbage pointer.
This patch adds checks to avoid emitting relocation records for non-
Definedsymbols, and to prevent creating an auto-import entry if an import entry for the wrapped symbol already exists.Fixes #150739