cmd/link: fix duplicated "undefined reloc" errors

For given program with 2 undefined relocations (main and undefined):

	package main
	func undefined()
	func defined() int {
		undefined()
		undefined()
		return 0
	}
	var x = defined()

"go tool link" produces these errors:

	main.defined: relocation target main.undefined not defined
	main.defined: relocation target main.undefined not defined
	runtime.main_main·f: relocation target main.main not defined
	main.defined: undefined: "main.undefined"
	main.defined: undefined: "main.undefined"
	runtime.main_main·f: undefined: "main.main"

After this CL is applied:

	main.defined: relocation target main.undefined not defined
	runtime.main_main·f: function main is undeclared in the main package

Fixes #10978
Improved error message for main proposed in #24809.

Change-Id: I4ba8547b1e143bbebeb4d6e29ea05d932124f037
Reviewed-on: https://go-review.googlesource.com/113955
Run-TryBot: Iskander Sharipov <iskander.sharipov@intel.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

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

@@ -110,6 +110,10 @@ func trampoline(ctxt *Link, s *sym.Symbol) {
 
 // resolve relocations in s.
 func relocsym(ctxt *Link, s *sym.Symbol) {
+	// undefinedSyms contains all undefined symbol names.
+	// For successfull builds, it remains nil and does not cause any overhead.
+	var undefinedSyms []string
+
 	for ri := int32(0); ri < int32(len(s.R)); ri++ {
 		r := &s.R[ri]
 		if r.Done {
@@ -128,7 +132,7 @@ func relocsym(ctxt *Link, s *sym.Symbol) {
 			continue
 		}
 
-		if r.Sym != nil && ((r.Sym.Type == 0 && !r.Sym.Attr.VisibilityHidden()) || r.Sym.Type == sym.SXREF) {
+		if r.Sym != nil && ((r.Sym.Type == sym.Sxxx && !r.Sym.Attr.VisibilityHidden()) || r.Sym.Type == sym.SXREF) {
 			// When putting the runtime but not main into a shared library
 			// these symbols are undefined and that's OK.
 			if ctxt.BuildMode == BuildModeShared {
@@ -140,7 +144,22 @@ func relocsym(ctxt *Link, s *sym.Symbol) {
 					continue
 				}
 			} else {
-				Errorf(s, "relocation target %s not defined", r.Sym.Name)
+				reported := false
+				for _, name := range undefinedSyms {
+					if name == r.Sym.Name {
+						reported = true
+						break
+					}
+				}
+				if !reported {
+					// Give a special error message for main symbol (see #24809).
+					if r.Sym.Name == "main.main" {
+						Errorf(s, "function main is undeclared in the main package")
+					} else {
+						Errorf(s, "relocation target %s not defined", r.Sym.Name)
+					}
+					undefinedSyms = append(undefinedSyms, r.Sym.Name)
+				}
 				continue
 			}
 		}

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

@@ -0,0 +1,70 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package ld
+
+import (
+	"internal/testenv"
+	"io/ioutil"
+	"os"
+	"os/exec"
+	"strings"
+	"testing"
+)
+
+func TestUndefinedRelocErrors(t *testing.T) {
+	testenv.MustHaveGoBuild(t)
+	dir, err := ioutil.TempDir("", "go-build")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(dir)
+
+	out, err := exec.Command(testenv.GoToolPath(t), "build", "./testdata/issue10978").CombinedOutput()
+	if err == nil {
+		t.Fatal("expected build to fail")
+	}
+
+	wantErrors := map[string]int{
+		// Main function has dedicated error message.
+		"function main is undeclared in the main package": 1,
+
+		// Single error reporting per each symbol.
+		// This way, duplicated messages are not reported for
+		// multiple relocations with a same name.
+		"main.defined1: relocation target main.undefined not defined": 1,
+		"main.defined2: relocation target main.undefined not defined": 1,
+	}
+	unexpectedErrors := map[string]int{}
+
+	for _, l := range strings.Split(string(out), "\n") {
+		if strings.HasPrefix(l, "#") || l == "" {
+			continue
+		}
+		matched := ""
+		for want := range wantErrors {
+			if strings.Contains(l, want) {
+				matched = want
+				break
+			}
+		}
+		if matched != "" {
+			wantErrors[matched]--
+		} else {
+			unexpectedErrors[l]++
+		}
+	}
+
+	for want, n := range wantErrors {
+		switch {
+		case n > 0:
+			t.Errorf("unmatched error: %s (x%d)", want, n)
+		case n < 0:
+			t.Errorf("extra errors: %s (x%d)", want, -n)
+		}
+	}
+	for unexpected, n := range unexpectedErrors {
+		t.Errorf("unexpected error: %s (x%d)", unexpected, n)
+	}
+}

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

@@ -2199,6 +2199,18 @@ func undefsym(ctxt *Link, s *sym.Symbol) {
 }
 
 func (ctxt *Link) undef() {
+	// undefsym performs checks (almost) identical to checks
+	// that report undefined relocations in relocsym.
+	// Both undefsym and relocsym can report same symbol as undefined,
+	// which results in error message duplication (see #10978).
+	//
+	// The undef is run after Arch.Asmb and could detect some
+	// programming errors there, but if object being linked is already
+	// failed with errors, it is better to avoid duplicated errors.
+	if nerrors > 0 {
+		return
+	}
+
 	for _, s := range ctxt.Textp {
 		undefsym(ctxt, s)
 	}

src/cmd/link/internal/ld/testdata/issue10978/main.go

@@ -0,0 +1,27 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+func undefined()
+
+func defined1() int {
+	// To check multiple errors for a single symbol,
+	// reference undefined more than once.
+	undefined()
+	undefined()
+	return 0
+}
+
+func defined2() {
+	undefined()
+	undefined()
+}
+
+func init() {
+	_ = defined1()
+	defined2()
+}
+
+// The "main" function remains undeclared.

src/cmd/link/internal/ld/testdata/issue10978/main.s

@@ -0,0 +1 @@
+// This file is needed to make "go build" work for package with external functions.