cmd/link: revise DLL import symbol handling

This patch reworks the handling of DLL import symbols in the PE host
object loader to ensure that the Go linker can deal with them properly
during internal linking.

Prior to this point the strategy was to immediately treat an import
symbol reference of the form "__imp__XXX" as if it were a reference to
the corresponding DYNIMPORT symbol XXX, except for certain special
cases. This worked for the most part, but ran into problems in
situations where the target ("XXX") wasn't a previously created
DYNIMPORT symbol (and when these problems happened, the root cause was
not always easy to see).

The new strategy is to not do any renaming or forwarding immediately,
but to delay handling until host object loading is complete. At that
point we make a scan through the newly introduced text+data sections
looking at the relocations that target import symbols, forwarding
the references to the corresponding DYNIMPORT sym where appropriate
and where there are direct refs to the DYNIMPORT syms, tagging them
for stub generation later on.

Updates #35006.
Updates #53540.

Change-Id: I2d42b39141ae150a9f82ecc334001749ae8a3b4a
Reviewed-on: https://go-review.googlesource.com/c/go/+/451738
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>

Author: thanm

src/cmd/link/internal/ld/ar.go

@@ -32,6 +32,7 @@ package ld
 
 import (
 	"cmd/internal/bio"
+	"cmd/link/internal/loader"
 	"cmd/link/internal/sym"
 	"encoding/binary"
 	"fmt"
@@ -61,6 +62,39 @@ type ArHdr struct {
 	fmag string
 }
 
+// pruneUndefsForWindows trims the list "undefs" of currently
+// outstanding unresolved symbols to remove references to DLL import
+// symbols (e.g. "__imp_XXX"). In older versions of the linker, we
+// would just immediately forward references from the import sym
+// (__imp_XXX) to the DLL sym (XXX), but with newer compilers this
+// strategy falls down in certain cases. We instead now do this
+// forwarding later on as a post-processing step, and meaning that
+// during the middle part of host object loading we can see a lot of
+// unresolved (SXREF) import symbols. We do not, however, want to
+// trigger the inclusion of an object from a host archive if the
+// reference is going to be eventually forwarded to the corresponding
+// SDYNIMPORT symbol, so here we strip out such refs from the undefs
+// list.
+func pruneUndefsForWindows(ldr *loader.Loader, undefs, froms []loader.Sym) ([]loader.Sym, []loader.Sym) {
+	var newundefs []loader.Sym
+	var newfroms []loader.Sym
+	for _, s := range undefs {
+		sname := ldr.SymName(s)
+		if strings.HasPrefix(sname, "__imp_") {
+			dname := sname[len("__imp_"):]
+			ds := ldr.Lookup(dname, 0)
+			if ds != 0 && ldr.SymType(ds) == sym.SDYNIMPORT {
+				// Don't try to pull things out of a host archive to
+				// satisfy this symbol.
+				continue
+			}
+		}
+		newundefs = append(newundefs, s)
+		newfroms = append(newfroms, s)
+	}
+	return newundefs, newfroms
+}
+
 // hostArchive reads an archive file holding host objects and links in
 // required objects. The general format is the same as a Go archive
 // file, but it has an armap listing symbols and the objects that
@@ -111,6 +145,9 @@ func hostArchive(ctxt *Link, name string) {
 		var load []uint64
 		returnAllUndefs := -1
 		undefs, froms := ctxt.loader.UndefinedRelocTargets(returnAllUndefs)
+		if buildcfg.GOOS == "windows" {
+			undefs, froms = pruneUndefsForWindows(ctxt.loader, undefs, froms)
+		}
 		for k, symIdx := range undefs {
 			sname := ctxt.loader.SymName(symIdx)
 			if off := armap[sname]; off != 0 && !loaded[off] {

src/cmd/link/internal/ld/data.go

@@ -37,6 +37,7 @@ import (
 	"cmd/internal/objabi"
 	"cmd/internal/sys"
 	"cmd/link/internal/loader"
+	"cmd/link/internal/loadpe"
 	"cmd/link/internal/sym"
 	"compress/zlib"
 	"debug/elf"
@@ -747,7 +748,38 @@ func (ctxt *Link) makeRelocSymState() *relocSymState {
 	}
 }
 
-func windynrelocsym(ctxt *Link, rel *loader.SymbolBuilder, s loader.Sym) {
+// windynrelocsym examines a text symbol 's' and looks for relocations
+// from it that correspond to references to symbols defined in DLLs,
+// then fixes up those relocations as needed. A reference to a symbol
+// XYZ from some DLL will fall into one of two categories: an indirect
+// ref via "__imp_XYZ", or a direct ref to "XYZ". Here's an example of
+// an indirect ref (this is an excerpt from objdump -ldr):
+//
+//	     1c1: 48 89 c6                     	movq	%rax, %rsi
+//	     1c4: ff 15 00 00 00 00            	callq	*(%rip)
+//			00000000000001c6:  IMAGE_REL_AMD64_REL32	__imp__errno
+//
+// In the assembly above, the code loads up the value of __imp_errno
+// and then does an indirect call to that value.
+//
+// Here is what a direct reference might look like:
+//
+//	     137: e9 20 06 00 00               	jmp	0x75c <pow+0x75c>
+//	     13c: e8 00 00 00 00               	callq	0x141 <pow+0x141>
+//			000000000000013d:  IMAGE_REL_AMD64_REL32	_errno
+//
+// The assembly below dispenses with the import symbol and just makes
+// a direct call to _errno.
+//
+// The code below handles indirect refs by redirecting the target of
+// the relocation from "__imp_XYZ" to "XYZ" (since the latter symbol
+// is what the Windows loader is expected to resolve). For direct refs
+// the call is redirected to a stub, where the stub first loads the
+// symbol and then direct an indirect call to that value.
+//
+// Note that for a given symbol (as above) it is perfectly legal to
+// have both direct and indirect references.
+func windynrelocsym(ctxt *Link, rel *loader.SymbolBuilder, s loader.Sym) error {
 	var su *loader.SymbolBuilder
 	relocs := ctxt.loader.Relocs(s)
 	for ri := 0; ri < relocs.Count(); ri++ {
@@ -763,13 +795,43 @@ func windynrelocsym(ctxt *Link, rel *loader.SymbolBuilder, s loader.Sym) {
 			if r.Weak() {
 				continue
 			}
-			ctxt.Errorf(s, "dynamic relocation to unreachable symbol %s",
+			return fmt.Errorf("dynamic relocation to unreachable symbol %s",
 				ctxt.loader.SymName(targ))
 		}
+		tgot := ctxt.loader.SymGot(targ)
+		if tgot == loadpe.RedirectToDynImportGotToken {
+
+			// Consistency check: name should be __imp_X
+			sname := ctxt.loader.SymName(targ)
+			if !strings.HasPrefix(sname, "__imp_") {
+				return fmt.Errorf("internal error in windynrelocsym: redirect GOT token applied to non-import symbol %s", sname)
+			}
+
+			// Locate underlying symbol (which originally had type
+			// SDYNIMPORT but has since been retyped to SWINDOWS).
+			ds, err := loadpe.LookupBaseFromImport(targ, ctxt.loader, ctxt.Arch)
+			if err != nil {
+				return err
+			}
+			dstyp := ctxt.loader.SymType(ds)
+			if dstyp != sym.SWINDOWS {
+				return fmt.Errorf("internal error in windynrelocsym: underlying sym for %q has wrong type %s", sname, dstyp.String())
+			}
+
+			// Redirect relocation to the dynimport.
+			r.SetSym(ds)
+			continue
+		}
 
 		tplt := ctxt.loader.SymPlt(targ)
-		tgot := ctxt.loader.SymGot(targ)
-		if tplt == -2 && tgot != -2 { // make dynimport JMP table for PE object files.
+		if tplt == loadpe.CreateImportStubPltToken {
+
+			// Consistency check: don't want to see both PLT and GOT tokens.
+			if tgot != -1 {
+				return fmt.Errorf("internal error in windynrelocsym: invalid GOT setting %d for reloc to %s", tgot, ctxt.loader.SymName(targ))
+			}
+
+			// make dynimport JMP table for PE object files.
 			tplt := int32(rel.Size())
 			ctxt.loader.SetPlt(targ, tplt)
 
@@ -782,8 +844,7 @@ func windynrelocsym(ctxt *Link, rel *loader.SymbolBuilder, s loader.Sym) {
 			// jmp *addr
 			switch ctxt.Arch.Family {
 			default:
-				ctxt.Errorf(s, "unsupported arch %v", ctxt.Arch.Family)
-				return
+				return fmt.Errorf("internal error in windynrelocsym: unsupported arch %v", ctxt.Arch.Family)
 			case sys.I386:
 				rel.AddUint8(0xff)
 				rel.AddUint8(0x25)
@@ -805,6 +866,7 @@ func windynrelocsym(ctxt *Link, rel *loader.SymbolBuilder, s loader.Sym) {
 			r.SetAdd(int64(tplt))
 		}
 	}
+	return nil
 }
 
 // windynrelocsyms generates jump table to C library functions that will be
@@ -818,7 +880,9 @@ func (ctxt *Link) windynrelocsyms() {
 	rel.SetType(sym.STEXT)
 
 	for _, s := range ctxt.Textp {
-		windynrelocsym(ctxt, rel, s)
+		if err := windynrelocsym(ctxt, rel, s); err != nil {
+			ctxt.Errorf(s, "%v", err)
+		}
 	}
 
 	ctxt.Textp = append(ctxt.Textp, rel.Sym())

src/cmd/link/internal/ld/lib.go

@@ -711,6 +711,13 @@ func loadWindowsHostArchives(ctxt *Link) {
 			ctxt.loader.SetAttrSpecial(sb.Sym(), true)
 		}
 	}
+
+	// Fix up references to DLL import symbols now that we're done
+	// pulling in new objects.
+	if err := loadpe.PostProcessImports(); err != nil {
+		Errorf(nil, "%v", err)
+	}
+
 	// TODO: maybe do something similar to peimporteddlls to collect
 	// all lib names and try link them all to final exe just like
 	// libmingwex.a and libmingw32.a: