cmd/compile: fix method set computation for shadowed methods

In expandmeth, we call expand1/expand0 to build a list of all
candidate methods to promote, and then we use dotpath to prune down
which names actually resolve to a promoted method and how.

However, previously we still computed "followsptr" based on the
expand1/expand0 traversal (which is depth-first), rather than
dotpath (which is breadth-first). The result is that we could
sometimes end up miscomputing whether a particular promoted method
involves a pointer traversal, which could result in bad code
generation for method trampolines.

Fixes #24547.

Change-Id: I57dc014466d81c165b05d78b98610dc3765b7a90
Reviewed-on: https://go-review.googlesource.com/102618
Reviewed-by: Robert Griesemer <gri@golang.org>

diff --git a/src/cmd/compile/internal/gc/subr.go b/src/cmd/compile/internal/gc/subr.go
index a6231963cdfba54b759ab70e7f382235a63cbae7..96c1fc1cca0973dfe8e927f33b0857790bd73a2b 100644 (file)
--- a/src/cmd/compile/internal/gc/subr.go
+++ b/src/cmd/compile/internal/gc/subr.go
@@ -1516,25 +1516,20 @@ func adddot(n *Node) *Node {
        return n
 }
 
-// code to help generate trampoline
-// functions for methods on embedded
-// subtypes.
-// these are approx the same as
-// the corresponding adddot routines
-// except that they expect to be called
-// with unique tasks and they return
-// the actual methods.
+// Code to help generate trampoline functions for methods on embedded
+// types. These are approx the same as the corresponding adddot
+// routines except that they expect to be called with unique tasks and
+// they return the actual methods.
+
 type Symlink struct {
-       field     *types.Field
-       followptr bool
+       field *types.Field
 }
 
 var slist []Symlink
 
-func expand0(t *types.Type, followptr bool) {
+func expand0(t *types.Type) {
        u := t
        if u.IsPtr() {
-               followptr = true
                u = u.Elem()
        }
 
@@ -1544,7 +1539,7 @@ func expand0(t *types.Type, followptr bool) {
                                continue
                        }
                        f.Sym.SetUniq(true)
-                       slist = append(slist, Symlink{field: f, followptr: followptr})
+                       slist = append(slist, Symlink{field: f})
                }
 
                return
@@ -1557,24 +1552,23 @@ func expand0(t *types.Type, followptr bool) {
                                continue
                        }
                        f.Sym.SetUniq(true)
-                       slist = append(slist, Symlink{field: f, followptr: followptr})
+                       slist = append(slist, Symlink{field: f})
                }
        }
 }
 
-func expand1(t *types.Type, top, followptr bool) {
+func expand1(t *types.Type, top bool) {
        if t.Recur() {
                return
        }
        t.SetRecur(true)
 
        if !top {
-               expand0(t, followptr)
+               expand0(t)
        }
 
        u := t
        if u.IsPtr() {
-               followptr = true
                u = u.Elem()
        }
 
@@ -1586,7 +1580,7 @@ func expand1(t *types.Type, top, followptr bool) {
                        if f.Sym == nil {
                                continue
                        }
-                       expand1(f.Type, false, followptr)
+                       expand1(f.Type, false)
                }
        }
 
@@ -1606,7 +1600,7 @@ func expandmeth(t *types.Type) {
 
        // generate all reachable methods
        slist = slist[:0]
-       expand1(t, true, false)
+       expand1(t, true)
 
        // check each method to be uniquely reachable
        var ms []*types.Field
@@ -1615,7 +1609,8 @@ func expandmeth(t *types.Type) {
                sl.field.Sym.SetUniq(false)
 
                var f *types.Field
-               if path, _ := dotpath(sl.field.Sym, t, &f, false); path == nil {
+               path, _ := dotpath(sl.field.Sym, t, &f, false)
+               if path == nil {
                        continue
                }
 
@@ -1627,8 +1622,11 @@ func expandmeth(t *types.Type) {
                // add it to the base type method list
                f = f.Copy()
                f.Embedded = 1 // needs a trampoline
-               if sl.followptr {
-                       f.Embedded = 2
+               for _, d := range path {
+                       if d.field.Type.IsPtr() {
+                               f.Embedded = 2
+                               break
+                       }
                }
                ms = append(ms, f)
        }

diff --git a/test/fixedbugs/issue24547.go b/test/fixedbugs/issue24547.go
new file mode 100644 (file)
index 0000000..47d94a9
--- /dev/null
+++ b/test/fixedbugs/issue24547.go
@@ -0,0 +1,46 @@
+// run
+
+// 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.
+
+// When computing method sets with shadowed methods, make sure we
+// compute whether a method promotion involved a pointer traversal
+// based on the promoted method, not the shadowed method.
+
+package main
+
+import (
+       "bytes"
+       "fmt"
+)
+
+type mystruct struct {
+       f int
+}
+
+func (t mystruct) String() string {
+       return "FAIL"
+}
+
+func main() {
+       type deep struct {
+               mystruct
+       }
+       s := struct {
+               deep
+               *bytes.Buffer
+       }{
+               deep{},
+               bytes.NewBufferString("ok"),
+       }
+
+       if got := s.String(); got != "ok" {
+               panic(got)
+       }
+
+       var i fmt.Stringer = s
+       if got := i.String(); got != "ok" {
+               panic(got)
+       }
+}