cmd/compile: fix detection of calls to reflect.Method

The existing code used Type.String() to obtain the name of a type;
specifically type reflect.Method in this case. However, Type.String()
formatting is intended for error messages and uses the format
pkgpath.name instead of pkgname.name if a package (in this case
package reflect) is imported multiple times. As a result, the
reflect.Method type detection failed under peculiar circumstances
(see the included test case).

Thanks to https://github.com/ericlagergren for tracking down
an easy way to make the bug disappear (which in turn directly
led to the underlying cause).

Fixes #19028.

Change-Id: I1b9c5dfd183260a9be74969fe916a94146fc36da
Reviewed-on: https://go-review.googlesource.com/45777Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>

src/cmd/compile/internal/gc/walk.go

@@ -3613,7 +3613,7 @@ func bounded(n *Node, max int64) bool {
 	return false
 }
 
-// usemethod check interface method calls for uses of reflect.Type.Method.
+// usemethod checks interface method calls for uses of reflect.Type.Method.
 func usemethod(n *Node) {
 	t := n.Left.Type
 
@@ -3648,11 +3648,12 @@ func usemethod(n *Node) {
 			return
 		}
 	}
-	if res0.Type.String() != "reflect.Method" {
-		return
-	}
 
+	// Note: Don't rely on res0.Type.String() since its formatting depends on multiple factors
+	//       (including global variables such as numImports - was issue #19028).
+	if s := res0.Type.Sym; s != nil && s.Name == "Method" && s.Pkg != nil && s.Pkg.Path == "reflect" {
 		Curfn.Func.SetReflectMethod(true)
+	}
 }
 
 func usefield(n *Node) {

test/fixedbugs/issue19028.dir/a.go

+// Copyright 2017 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 reflect
+
+import "reflect"
+
+type Type reflect.Type

test/fixedbugs/issue19028.dir/main.go

+// Copyright 2017 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
+
+import (
+        "reflect"
+        fake "./reflect" // 2nd package with name "reflect"
+)
+
+type T struct {
+        _ fake.Type
+}
+
+func (T) f()            {}
+func (T) G() (_ int)    { return }
+func (T) H() (_, _ int) { return }
+
+func main() {
+        var x T
+        typ := reflect.TypeOf(x)
+        for i := 0; i < typ.NumMethod(); i++ {
+                _ = typ.Method(i) // must not crash
+        }
+}

test/fixedbugs/issue19028.go

+// rundir
+
+// Copyright 2017 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.
+
+// This test failed when the compiler didn't use the
+// correct code to identify the type reflect.Method.
+// The failing code relied on Type.String() which had
+// formatting that depended on whether a package (in
+// this case "reflect") was imported more than once.
+
+package ignored