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/1840Reviewed-by: Ian Lance Taylor <iant@golang.org>

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...)