From 7b177b1a03c2f519bb5e52eb4471a1e2580f6db9 Mon Sep 17 00:00:00 2001
From: Matthew Dempsky <mdempsky@google.com>
Date: Tue, 27 Mar 2018 01:05:48 -0700
Subject: [PATCH] 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>
---
 src/cmd/compile/internal/gc/subr.go | 42 +++++++++++++-------------
 test/fixedbugs/issue24547.go        | 46 +++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 22 deletions(-)
 create mode 100644 test/fixedbugs/issue24547.go

diff --git a/src/cmd/compile/internal/gc/subr.go b/src/cmd/compile/internal/gc/subr.go
index a6231963cd..96c1fc1cca 100644
--- 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
index 0000000000..47d94a9f9f
--- /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)
+	}
+}