[release-branch.go1.7] runtime: fix getArgInfo for deferred reflection calls

Fixes #18333 (backport)

getArgInfo for reflect.makeFuncStub and reflect.methodValueCall is
necessarily special. These have dynamically determined argument maps
that are stored in their context (that is, their *funcval). These
functions are written to store this context at 0(SP) when called, and
getArgInfo retrieves it from there.

This technique works if getArgInfo is passed an active call frame for
one of these functions. However, getArgInfo is also used in
tracebackdefers, where the "call" is not a true call with an active
stack frame, but a deferred call. In this situation, getArgInfo
currently crashes because tracebackdefers passes a frame with sp set
to 0. However, the entire approach used by getArgInfo is flawed in
this situation because the wrapper has not actually executed, and
hence hasn't saved this metadata to any stack frame.

In the defer case, we know the *funcval from the _defer itself, so we
can fix this by teaching getArgInfo to use the *funcval context
directly when its available, and otherwise get it from the active call
frame.

While we're here, this commit simplifies getArgInfo a bit by making it
play more nicely with the type system. Rather than decoding the
*reflect.methodValue that is the wrapper's context as a *[2]uintptr,
just write out a copy of the reflect.methodValue type in the runtime.

Fixes #16331. Fixes #17471.

Change-Id: I81db4d985179b4a81c68c490cceeccbfc675456a
Reviewed-on: https://go-review.googlesource.com/31138
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-on: https://go-review.googlesource.com/35638
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Austin Clements <austin@google.com>

3 files changed, 86 insertions, 7 deletions

diff --git a/src/reflect/makefunc.go b/src/reflect/makefunc.go
index ad2ebd0de9..a7efeb8262 100644
--- a/src/reflect/makefunc.go
+++ b/src/reflect/makefunc.go
@@ -70,6 +70,8 @@ func MakeFunc(typ Type, fn func(args []Value) (results []Value)) Value {
 // word in the passed-in argument frame.
 func makeFuncStub()
 
+// This type is partially duplicated as runtime.reflectMethodValue.
+// Any changes should be reflected in both.
 type methodValue struct {
 	fn     uintptr
 	stack  *bitVector // stack bitmap for args - offset known to runtime
diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go
index 80a54407b3..36abe1f1f0 100644
--- a/src/runtime/traceback.go
+++ b/src/runtime/traceback.go
@@ -107,7 +107,7 @@ func tracebackdefers(gp *g, callback func(*stkframe, unsafe.Pointer) bool, v uns
 			}
 			frame.fn = f
 			frame.argp = uintptr(deferArgs(d))
-			frame.arglen, frame.argmap = getArgInfo(&frame, f, true)
+			frame.arglen, frame.argmap = getArgInfo(&frame, f, true, fn)
 		}
 		frame.continpc = frame.pc
 		if !callback((*stkframe)(noescape(unsafe.Pointer(&frame))), v) {
@@ -339,7 +339,7 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
 		// metadata recorded by f's caller.
 		if callback != nil || printing {
 			frame.argp = frame.fp + sys.MinFrameSize
-			frame.arglen, frame.argmap = getArgInfo(&frame, f, callback != nil)
+			frame.arglen, frame.argmap = getArgInfo(&frame, f, callback != nil, nil)
 		}
 
 		// Determine frame's 'continuation PC', where it can continue.
@@ -546,19 +546,48 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
 	return n
 }
 
-func getArgInfo(frame *stkframe, f *_func, needArgMap bool) (arglen uintptr, argmap *bitvector) {
+// reflectMethodValue is a partial duplicate of reflect.methodValue.
+type reflectMethodValue struct {
+	fn    uintptr
+	stack *bitvector // args bitmap
+}
+
+// getArgInfo returns the argument frame information for a call to f
+// with call frame frame.
+//
+// This is used for both actual calls with active stack frames and for
+// deferred calls that are not yet executing. If this is an actual
+// call, ctxt must be nil (getArgInfo will retrieve what it needs from
+// the active stack frame). If this is a deferred call, ctxt must be
+// the function object that was deferred.
+func getArgInfo(frame *stkframe, f *_func, needArgMap bool, ctxt *funcval) (arglen uintptr, argmap *bitvector) {
 	arglen = uintptr(f.args)
 	if needArgMap && f.args == _ArgsSizeUnknown {
 		// Extract argument bitmaps for reflect stubs from the calls they made to reflect.
 		switch funcname(f) {
 		case "reflect.makeFuncStub", "reflect.methodValueCall":
-			arg0 := frame.sp + sys.MinFrameSize
-			fn := *(**[2]uintptr)(unsafe.Pointer(arg0))
-			if fn[0] != f.entry {
+			// These take a *reflect.methodValue as their
+			// context register.
+			var mv *reflectMethodValue
+			if ctxt != nil {
+				// This is not an actual call, but a
+				// deferred call. The function value
+				// is itself the *reflect.methodValue.
+				mv = (*reflectMethodValue)(unsafe.Pointer(ctxt))
+			} else {
+				// This is a real call that took the
+				// *reflect.methodValue as its context
+				// register and immediately saved it
+				// to 0(SP). Get the methodValue from
+				// 0(SP).
+				arg0 := frame.sp + sys.MinFrameSize
+				mv = *(**reflectMethodValue)(unsafe.Pointer(arg0))
+			}
+			if mv.fn != f.entry {
 				print("runtime: confused by ", funcname(f), "\n")
 				throw("reflect mismatch")
 			}
-			bv := (*bitvector)(unsafe.Pointer(fn[1]))
+			bv := mv.stack
 			arglen = uintptr(bv.n * sys.PtrSize)
 			argmap = bv
 		}
diff --git a/test/fixedbugs/issue16331.go b/test/fixedbugs/issue16331.go
new file mode 100644
index 0000000000..665e7fc0fd
--- /dev/null
+++ b/test/fixedbugs/issue16331.go
@@ -0,0 +1,48 @@
+// run
+
+// Copyright 2016 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.
+
+// Perform tracebackdefers with a deferred reflection method.
+
+package main
+
+import "reflect"
+
+type T struct{}
+
+func (T) M() {
+}
+
+func F(args []reflect.Value) (results []reflect.Value) {
+	return nil
+}
+
+func main() {
+	done := make(chan bool)
+	go func() {
+		// Test reflect.makeFuncStub.
+		t := reflect.TypeOf((func())(nil))
+		f := reflect.MakeFunc(t, F).Interface().(func())
+		defer f()
+		growstack(10000)
+		done <- true
+	}()
+	<-done
+	go func() {
+		// Test reflect.methodValueCall.
+		f := reflect.ValueOf(T{}).Method(0).Interface().(func())
+		defer f()
+		growstack(10000)
+		done <- true
+	}()
+	<-done
+}
+
+func growstack(x int) {
+	if x == 0 {
+		return
+	}
+	growstack(x - 1)
+}