cmd/go: be more careful when linking a test exe with gccgo

Previously, we ended up passing two compiled objects for the package
being tested when linking the test executable.  Somewhat by luck, this
worked most of the time but occasionally it did not.  This changes the
linking code to not pass two objects for the same ImportPath and to
always pass the object for the test version of the package and removes
some unecessary nil checks.

Change-Id: I7bbd3fc708f14672ee2cc6aed3397421fceb8a38
Reviewed-on: https://go-review.googlesource.com/1840
Reviewed-by: Ian Lance Taylor <iant@golang.org>

diff --git a/src/cmd/go/build.go b/src/cmd/go/build.go
index cd17aba7370c5d0c63286d795c534cbd046b8df9..e201f29e766576cb38acbbc924af2881fe359064 100644 (file)
--- a/src/cmd/go/build.go
+++ b/src/cmd/go/build.go
@@ -1899,7 +1899,6 @@ func (gccgoToolchain) pack(b *builder, p *Package, objDir, afile string, ofiles
 func (tools gccgoToolchain) ld(b *builder, p *Package, out string, allactions []*action, mainpkg string, ofiles []string) error {
        // gccgo needs explicit linking with all package dependencies,
        // and all LDFLAGS from cgo dependencies.
-       apackagesSeen := make(map[*Package]bool)
        afiles := []string{}
        ldflags := b.gccArchArgs()
        cgoldflags := []string{}
@@ -1907,40 +1906,42 @@ func (tools gccgoToolchain) ld(b *builder, p *Package, out string, allactions []
        cxx := len(p.CXXFiles) > 0
        objc := len(p.MFiles) > 0
 
-       // Prefer the output of an install action to the output of a build action,
-       // because the install action will delete the output of the build action.
-       // Iterate over the list backward (reverse dependency order) so that we
-       // always see the install before the build.
+       // For a given package import path:
+       //   1) prefer a test package (created by (*builder).test) to a non-test package
+       //   2) prefer the output of an install action to the output of a build action
+       //      because the install action will delete the output of the build
+       //      action
+       // Iterating over the list backwards (reverse dependency order) ensures that we
+       // always see an install before a build.
+       importPathsSeen := make(map[string]bool)
        for i := len(allactions) - 1; i >= 0; i-- {
                a := allactions[i]
-               if !a.p.Standard {
-                       if a.p != nil && !apackagesSeen[a.p] {
-                               apackagesSeen[a.p] = true
-                               if a.p.fake {
-                                       // move _test files to the top of the link order
-                                       afiles = append([]string{a.target}, afiles...)
-                               } else {
-                                       afiles = append(afiles, a.target)
-                               }
-                       }
+               if a.p.fake && !importPathsSeen[a.p.ImportPath] {
+                       importPathsSeen[a.p.ImportPath] = true
+                       afiles = append(afiles, a.target)
+               }
+       }
+       for i := len(allactions) - 1; i >= 0; i-- {
+               a := allactions[i]
+               if !a.p.Standard && !importPathsSeen[a.p.ImportPath] {
+                       importPathsSeen[a.p.ImportPath] = true
+                       afiles = append(afiles, a.target)
                }
        }
 
        for _, a := range allactions {
-               if a.p != nil {
-                       cgoldflags = append(cgoldflags, a.p.CgoLDFLAGS...)
-                       if len(a.p.CgoFiles) > 0 {
-                               usesCgo = true
-                       }
-                       if a.p.usesSwig() {
-                               usesCgo = true
-                       }
-                       if len(a.p.CXXFiles) > 0 {
-                               cxx = true
-                       }
-                       if len(a.p.MFiles) > 0 {
-                               objc = true
-                       }
+               cgoldflags = append(cgoldflags, a.p.CgoLDFLAGS...)
+               if len(a.p.CgoFiles) > 0 {
+                       usesCgo = true
+               }
+               if a.p.usesSwig() {
+                       usesCgo = true
+               }
+               if len(a.p.CXXFiles) > 0 {
+                       cxx = true
+               }
+               if len(a.p.MFiles) > 0 {
+                       objc = true
                }
        }
        ldflags = append(ldflags, afiles...)