[LLD][COFF] Prevent to emit relocations for discarded weak wrapped symbols (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)

Author: kikairoya

lld/COFF/Chunks.cpp

@@ -570,7 +570,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);
   }

lld/COFF/MinGW.cpp

@@ -269,10 +269,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;
+        }
       }
     }
   }

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.obj
+// RUN: llvm-mc -filetype=obj -triple=x86_64-mingw other.s -o other.obj
+
+// RUN: lld-link -lldmingw -machine:x64 -dll -out:other.dll other.obj -noentry -export:foo -implib:other.lib
+// RUN: lld-link -lldmingw -machine:x64 -out:main.exe main.obj other.lib -entry:entry -wrap:foo -debug:symtab
+// RUN: llvm-readobj --sections --symbols --coff-imports --coff-basereloc main.exe | FileCheck %s --implicit-check-not=other.dll
+
+// CHECK:      Number: 3
+// CHECK-NEXT: Name: .data
+// CHECK-NEXT: VirtualSize:
+// CHECK-NEXT: VirtualAddress: 0x[[#%x,SECTOP:0x3000]]
+// 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

lld/test/COFF/wrap-dllimport.s

@@ -1,42 +1,60 @@
 // 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 that 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 to null because 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