diff --git a/src/pkg/runtime/traceback_arm.c b/src/pkg/runtime/traceback_arm.c index 8acd143a5c..8d1fc54266 100644 --- a/src/pkg/runtime/traceback_arm.c +++ b/src/pkg/runtime/traceback_arm.c @@ -261,11 +261,20 @@ runtime·gentraceback(uintptr pc0, uintptr sp0, uintptr lr0, G *gp, int32 skip, if(pcbuf == nil && callback == nil) n = nprint; - if(callback != nil && n < max && (defer != nil || panic != nil && panic->defer != nil)) { + // For rationale, see long comment in traceback_x86.c. + if(callback != nil && n < max && defer != nil) { if(defer != nil) runtime·printf("runtime: g%D: leftover defer argp=%p pc=%p\n", gp->goid, defer->argp, defer->pc); - if(panic != nil && panic->defer != nil) + if(panic != nil) runtime·printf("runtime: g%D: leftover panic argp=%p pc=%p\n", gp->goid, panic->defer->argp, panic->defer->pc); + for(defer = gp->defer; defer != nil; defer = defer->link) + runtime·printf("\tdefer %p argp=%p pc=%p\n", defer, defer->argp, defer->pc); + for(panic = gp->panic; panic != nil; panic = panic->link) { + runtime·printf("\tpanic %p defer %p", panic, panic->defer); + if(panic->defer != nil) + runtime·printf(" argp=%p pc=%p", panic->defer->argp, panic->defer->pc); + runtime·printf("\n"); + } runtime·throw("traceback has leftover defers or panics"); } diff --git a/src/pkg/runtime/traceback_x86.c b/src/pkg/runtime/traceback_x86.c index 0ecaa0fd77..851504f529 100644 --- a/src/pkg/runtime/traceback_x86.c +++ b/src/pkg/runtime/traceback_x86.c @@ -304,11 +304,68 @@ runtime·gentraceback(uintptr pc0, uintptr sp0, uintptr lr0, G *gp, int32 skip, if(pcbuf == nil && callback == nil) n = nprint; - if(callback != nil && n < max && (defer != nil || panic != nil)) { + // If callback != nil, we're being called to gather stack information during + // garbage collection or stack growth. In that context, require that we used + // up the entire defer stack. If not, then there is a bug somewhere and the + // garbage collection or stack growth may not have seen the correct picture + // of the stack. Crash now instead of silently executing the garbage collection + // or stack copy incorrectly and setting up for a mysterious crash later. + // + // Note that panic != nil is okay here: there can be leftover panics, + // because the defers on the panic stack do not nest in frame order as + // they do on the defer stack. If you have: + // + // frame 1 defers d1 + // frame 2 defers d2 + // frame 3 defers d3 + // frame 4 panics + // frame 4's panic starts running defers + // frame 5, running d3, defers d4 + // frame 5 panics + // frame 5's panic starts running defers + // frame 6, running d4, garbage collects + // frame 6, running d2, garbage collects + // + // During the execution of d4, the panic stack is d4 -> d3, which + // is nested properly, and we'll treat frame 3 as resumable, because we + // can find d3. (And in fact frame 3 is resumable. If d4 recovers + // and frame 5 continues running, d3, d3 can recover and we'll + // resume execution in (returning from) frame 3.) + // + // During the execution of d2, however, the panic stack is d2 -> d3, + // which is inverted. The scan will match d2 to frame 2 but having + // d2 on the stack until then means it will not match d3 to frame 3. + // This is okay: if we're running d2, then all the defers after d2 have + // completed and their corresponding frames are dead. Not finding d3 + // for frame 3 means we'll set frame 3's continpc == 0, which is correct + // (frame 3 is dead). At the end of the walk the panic stack can thus + // contain defers (d3 in this case) for dead frames. The inversion here + // always indicates a dead frame, and the effect of the inversion on the + // scan is to hide those dead frames, so the scan is still okay: + // what's left on the panic stack are exactly (and only) the dead frames. + // + // We require callback != nil here because only when callback != nil + // do we know that gentraceback is being called in a "must be correct" + // context as opposed to a "best effort" context. The tracebacks with + // callbacks only happen when everything is stopped nicely. + // At other times, such as when gathering a stack for a profiling signal + // or when printing a traceback during a crash, everything may not be + // stopped nicely, and the stack walk may not be able to complete. + // It's okay in those situations not to use up the entire defer stack: + // incomplete information then is still better than nothing. + if(callback != nil && n < max && defer != nil) { if(defer != nil) runtime·printf("runtime: g%D: leftover defer argp=%p pc=%p\n", gp->goid, defer->argp, defer->pc); if(panic != nil) runtime·printf("runtime: g%D: leftover panic argp=%p pc=%p\n", gp->goid, panic->defer->argp, panic->defer->pc); + for(defer = gp->defer; defer != nil; defer = defer->link) + runtime·printf("\tdefer %p argp=%p pc=%p\n", defer, defer->argp, defer->pc); + for(panic = gp->panic; panic != nil; panic = panic->link) { + runtime·printf("\tpanic %p defer %p", panic, panic->defer); + if(panic->defer != nil) + runtime·printf(" argp=%p pc=%p", panic->defer->argp, panic->defer->pc); + runtime·printf("\n"); + } runtime·throw("traceback has leftover defers or panics"); } diff --git a/test/fixedbugs/issue8132.go b/test/fixedbugs/issue8132.go new file mode 100644 index 0000000000..52f5d39c2f --- /dev/null +++ b/test/fixedbugs/issue8132.go @@ -0,0 +1,32 @@ +// run + +// Copyright 2014 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. + +// issue 8132. stack walk handling of panic stack was confused +// about what was legal. + +package main + +import "runtime" + +var p *int + +func main() { + func() { + defer func() { + runtime.GC() + recover() + }() + var x [8192]byte + func(x [8192]byte) { + defer func() { + if err := recover(); err != nil { + println(*p) + } + }() + println(*p) + }(x) + }() +}