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/45777
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>

diff --git a/src/cmd/compile/internal/gc/walk.go b/src/cmd/compile/internal/gc/walk.go
index 15108e6e5737a50fdf8daf3a1f2b29a15284ce09..76031d160abb9ada73b7e84399a16374169a0d7b 100644 (file)
--- a/src/cmd/compile/internal/gc/walk.go
+++ b/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
-       }
 
-       Curfn.Func.SetReflectMethod(true)
+       // 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) {

diff --git a/test/fixedbugs/issue19028.dir/a.go b/test/fixedbugs/issue19028.dir/a.go
new file mode 100644 (file)
index 0000000..361251d
--- /dev/null
+++ b/test/fixedbugs/issue19028.dir/a.go
@@ -0,0 +1,9 @@
+// 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

diff --git a/test/fixedbugs/issue19028.dir/main.go b/test/fixedbugs/issue19028.dir/main.go
new file mode 100644 (file)
index 0000000..627e926
--- /dev/null
+++ b/test/fixedbugs/issue19028.dir/main.go
@@ -0,0 +1,26 @@
+// 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
+        }
+}

diff --git a/test/fixedbugs/issue19028.go b/test/fixedbugs/issue19028.go
new file mode 100644 (file)
index 0000000..8d934d2
--- /dev/null
+++ b/test/fixedbugs/issue19028.go
@@ -0,0 +1,13 @@
+// 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