Skip to content

Commit

Permalink
runtime: get map of args of unstarted goroutines like we do for defers
Browse files Browse the repository at this point in the history
Normally, reflect.makeFuncStub records the context value at a known
point in the stack frame, so that the runtime can get the argument map
for reflect.makeFuncStub from that known location.

This doesn't work for defers or goroutines that haven't started yet,
because they haven't allocated a frame or run an instruction yet. The
argument map must be extracted from the context value. We already do
this for defers (the non-nil ctxt arg to getArgInfo), we just need to
do it for unstarted goroutines as well.

When we traceback a goroutine, remember the context value from
g.sched.  Use it for the first frame we find.

(We never need it for deeper frames, because we normally don't stop at
 the start of reflect.makeFuncStub, as it is nosplit. With this CL we
 could allow makeFuncStub to no longer be nosplit.)

Fixes #25897

Change-Id: I427abf332a741a80728cdc0b8412aa8f37e7c418
Reviewed-on: https://go-review.googlesource.com/c/go/+/180258
Reviewed-by: Cherry Zhang <cherryyz@google.com>
  • Loading branch information
randall77 committed Jun 3, 2019
1 parent 2ce643d commit 38c129b
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 6 deletions.
16 changes: 10 additions & 6 deletions src/runtime/traceback.go
Expand Up @@ -119,6 +119,8 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
}
level, _, _ := gotraceback()

var ctxt *funcval // Context pointer for unstarted goroutines. See issue #25897.

if pc0 == ^uintptr(0) && sp0 == ^uintptr(0) { // Signal to fetch saved values from gp.
if gp.syscallsp != 0 {
pc0 = gp.syscallpc
Expand All @@ -132,6 +134,7 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
if usesLR {
lr0 = gp.sched.lr
}
ctxt = (*funcval)(gp.sched.ctxt)
}
}

Expand Down Expand Up @@ -300,9 +303,10 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
var ok bool
frame.arglen, frame.argmap, ok = getArgInfoFast(f, callback != nil)
if !ok {
frame.arglen, frame.argmap = getArgInfo(&frame, f, callback != nil, nil)
frame.arglen, frame.argmap = getArgInfo(&frame, f, callback != nil, ctxt)
}
}
ctxt = nil // ctxt is only needed to get arg maps for the topmost frame

// Determine frame's 'continuation PC', where it can continue.
// Normally this is the return address on the stack, but if sigpanic
Expand Down Expand Up @@ -593,10 +597,10 @@ func getArgInfoFast(f funcInfo, needArgMap bool) (arglen uintptr, argmap *bitvec
// 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
// deferred calls or goroutines 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.
// the active stack frame). If this is a deferred call or unstarted goroutine,
// ctxt must be the function object that was deferred or go'd.
func getArgInfo(frame *stkframe, f funcInfo, needArgMap bool, ctxt *funcval) (arglen uintptr, argmap *bitvector) {
arglen = uintptr(f.args)
if needArgMap && f.args == _ArgsSizeUnknown {
Expand All @@ -609,8 +613,8 @@ func getArgInfo(frame *stkframe, f funcInfo, needArgMap bool, ctxt *funcval) (ar
var retValid bool
if ctxt != nil {
// This is not an actual call, but a
// deferred call. The function value
// is itself the *reflect.methodValue.
// deferred call or an unstarted goroutine.
// The function value is itself the *reflect.methodValue.
mv = (*reflectMethodValue)(unsafe.Pointer(ctxt))
} else {
// This is a real call that took the
Expand Down
34 changes: 34 additions & 0 deletions test/fixedbugs/issue25897a.go
@@ -0,0 +1,34 @@
// run

// Copyright 2019 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.

// Make sure the runtime can scan args of an unstarted goroutine
// which starts with a reflect-generated function.

package main

import (
"reflect"
"runtime"
)

const N = 100

func main() {
runtime.GOMAXPROCS(1)
c := make(chan bool, N)
for i := 0; i < N; i++ {
f := reflect.MakeFunc(reflect.TypeOf(((func(*int))(nil))),
func(args []reflect.Value) []reflect.Value {
c <- true
return nil
}).Interface().(func(*int))
go f(nil)
}
runtime.GC()
for i := 0; i < N; i++ {
<-c
}
}
38 changes: 38 additions & 0 deletions test/fixedbugs/issue25897b.go
@@ -0,0 +1,38 @@
// run

// Copyright 2019 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.

// Make sure the runtime can scan args of an unstarted goroutine
// which starts with a reflect-generated function.

package main

import (
"reflect"
"runtime"
)

const N = 100

type T struct {
}

func (t *T) Foo(c chan bool) {
c <- true
}

func main() {
t := &T{}
runtime.GOMAXPROCS(1)
c := make(chan bool, N)
for i := 0; i < N; i++ {
f := reflect.ValueOf(t).MethodByName("Foo").Interface().(func(chan bool))
go f(c)
}
runtime.GC()
for i := 0; i < N; i++ {
<-c
}
}

0 comments on commit 38c129b

Please sign in to comment.