cmd/link: fix confusing error on unresolved symbol

Currently, if an assembly file includes a static reference to an
undefined symbol, and another package also has an undefined reference
to that symbol, the linker can report an error like:

  x: relocation target zero not defined for ABI0 (but is defined for ABI0)

Since the symbol is referenced in another package, the code in
ErrorUnresolved that looks for alternative ABI symbols finds that
symbol in the symbol table, but doesn't check that it's actually
defined, which is where the "but is defined for ABI0" comes from. The
"not defined for ABI0" is because ErrorUnresolved failed to turn the
static symbol's version back into an ABI, and it happened to print the
zero value for an ABI.

This CL fixes both of these problems. It explicitly maps the
relocation version back to an ABI and detects if it can't be mapped
back (e.g., because it's a static reference). Then, if it finds a
symbol with a different ABI in the symbol table, it checks to make
sure it's a definition, and not simply an unresolved reference.

Fixes #29852.

Change-Id: Ice45cc41c1907919ce5750f74588e8047eaa888c
Reviewed-on: https://go-review.googlesource.com/c/159518
Run-TryBot: Austin Clements <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>

Author: aclements

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

@@ -113,15 +113,16 @@ func (ctxt *Link) ErrorUnresolved(s *sym.Symbol, r *sym.Reloc) {
 		// Try to find symbol under another ABI.
 		var reqABI, haveABI obj.ABI
 		haveABI = ^obj.ABI(0)
-		for abi := obj.ABI(0); abi < obj.ABICount; abi++ {
-			v := sym.ABIToVersion(abi)
-			if v == -1 {
-				continue
-			}
-			if v == int(r.Sym.Version) {
-				reqABI = abi
-			} else if ctxt.Syms.ROLookup(r.Sym.Name, v) != nil {
-				haveABI = abi
+		reqABI, ok := sym.VersionToABI(int(r.Sym.Version))
+		if ok {
+			for abi := obj.ABI(0); abi < obj.ABICount; abi++ {
+				v := sym.ABIToVersion(abi)
+				if v == -1 {
+					continue
+				}
+				if rs := ctxt.Syms.ROLookup(r.Sym.Name, v); rs != nil && rs.Type != sym.Sxxx {
+					haveABI = abi
+				}
 			}
 		}
 

src/cmd/link/internal/sym/symbol.go

@@ -68,6 +68,16 @@ func ABIToVersion(abi obj.ABI) int {
 	return -1
 }
 
+func VersionToABI(v int) (obj.ABI, bool) {
+	switch v {
+	case SymVerABI0:
+		return obj.ABI0, true
+	case SymVerABIInternal:
+		return obj.ABIInternal, true
+	}
+	return ^obj.ABI(0), false
+}
+
 func (s *Symbol) String() string {
 	if s.Version == 0 {
 		return s.Name

src/cmd/link/link_test.go

@@ -6,6 +6,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"testing"
 )
@@ -116,3 +117,57 @@ func TestIssue28429(t *testing.T) {
 	// to compile the extra section.
 	runGo("tool", "link", "main.a")
 }
+
+func TestUnresolved(t *testing.T) {
+	testenv.MustHaveGoBuild(t)
+
+	tmpdir, err := ioutil.TempDir("", "unresolved-")
+	if err != nil {
+		t.Fatalf("failed to create temp dir: %v", err)
+	}
+	defer os.RemoveAll(tmpdir)
+
+	write := func(name, content string) {
+		err := ioutil.WriteFile(filepath.Join(tmpdir, name), []byte(content), 0666)
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	// Test various undefined references. Because of issue #29852,
+	// this used to give confusing error messages because the
+	// linker would find an undefined reference to "zero" created
+	// by the runtime package.
+
+	write("main.go", `package main
+
+func main() {
+        x()
+}
+
+func x()
+`)
+	write("main.s", `
+TEXT ·x(SB),0,$0
+        MOVD zero<>(SB), AX
+        MOVD zero(SB), AX
+        MOVD ·zero(SB), AX
+        RET
+`)
+	cmd := exec.Command(testenv.GoToolPath(t), "build")
+	cmd.Dir = tmpdir
+	cmd.Env = append(os.Environ(), []string{"GOARCH=amd64", "GOOS=linux"}...)
+	out, err := cmd.CombinedOutput()
+	if err == nil {
+		t.Fatalf("expected build to fail, but it succeeded")
+	}
+	out = regexp.MustCompile("(?m)^#.*\n").ReplaceAll(out, nil)
+	got := string(out)
+	want := `main.x: relocation target zero not defined
+main.x: relocation target zero not defined
+main.x: relocation target main.zero not defined
+`
+	if want != got {
+		t.Fatalf("want:\n%sgot:\n%s", want, got)
+	}
+}