Skip to content
Merged
2 changes: 1 addition & 1 deletion lld/COFF/Chunks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
24 changes: 20 additions & 4 deletions lld/COFF/MinGW.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
}
}
}
Expand Down
52 changes: 52 additions & 0 deletions lld/test/COFF/reloc-undefined-weak.s
Original file line number Diff line number Diff line change
@@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// 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
33 changes: 25 additions & 8 deletions lld/test/COFF/wrap-dllimport.s
Original file line number Diff line number Diff line change
@@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment seems to lack some word to be proper English - "to a symbol that already exists", or something like that?


// 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rephrase this sentence, "points the null" also seems to lack a couple words, or has some typos.

// `__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
16 changes: 16 additions & 0 deletions lld/test/COFF/wrap-inherit-weakness.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
; REQUIRES: x86

; Check that 'weak' attribute will be inherited to the wrapped symbols.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate a bit more about what happens here? I don't quite understand the involved mechanics here.


; 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

ret void
}