]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/link: fix duplicated "undefined reloc" errors
authorisharipo <iskander.sharipov@intel.com>
Mon, 21 May 2018 18:00:01 +0000 (21:00 +0300)
committerIan Lance Taylor <iant@golang.org>
Tue, 5 Jun 2018 16:49:07 +0000 (16:49 +0000)
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
src/cmd/link/internal/ld/ld_test.go [new file with mode: 0644]
src/cmd/link/internal/ld/lib.go
src/cmd/link/internal/ld/testdata/issue10978/main.go [new file with mode: 0644]
src/cmd/link/internal/ld/testdata/issue10978/main.s [new file with mode: 0644]

index 6cc5c544f5a0113ea550219b2e5ca29b245150a6..1cdb27970789387d6f3ccbd9500c926074ae4b49 100644 (file)
@@ -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
                        }
                }
diff --git a/src/cmd/link/internal/ld/ld_test.go b/src/cmd/link/internal/ld/ld_test.go
new file mode 100644 (file)
index 0000000..4884a07
--- /dev/null
@@ -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)
+       }
+}
index 816c867fa8f8a74ba2e7681dacf665619fe44512..e6682606b62ed5ad2efd68fa71b228d2db36a0fe 100644 (file)
@@ -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)
        }
diff --git a/src/cmd/link/internal/ld/testdata/issue10978/main.go b/src/cmd/link/internal/ld/testdata/issue10978/main.go
new file mode 100644 (file)
index 0000000..5e8c097
--- /dev/null
@@ -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.
diff --git a/src/cmd/link/internal/ld/testdata/issue10978/main.s b/src/cmd/link/internal/ld/testdata/issue10978/main.s
new file mode 100644 (file)
index 0000000..1d00e76
--- /dev/null
@@ -0,0 +1 @@
+// This file is needed to make "go build" work for package with external functions.\r