]> Cypherpunks repositories - gostls13.git/commitdiff
[release-branch.go1.3] runtime: fix panic stack during runtime.Goexit during panic
authorRuss Cox <rsc@golang.org>
Sat, 7 Jun 2014 02:07:32 +0000 (22:07 -0400)
committerRuss Cox <rsc@golang.org>
Sat, 7 Jun 2014 02:07:32 +0000 (22:07 -0400)
««« CL 102220043 / 00224712f89e
runtime: fix panic stack during runtime.Goexit during panic

A runtime.Goexit during a panic-invoked deferred call
left the panic stack intact even though all the stack frames
are gone when the goroutine is torn down.
The next goroutine to reuse that struct will have a
bogus panic stack and can cause the traceback routines
to walk into garbage.

Most likely to happen during tests, because t.Fatal might
be called during a deferred func and uses runtime.Goexit.

This "not enough cleared in Goexit" failure mode has
happened to us multiple times now. Clear all the pointers
that don't make sense to keep, not just gp->panic.

Fixes #8158.

LGTM=iant, dvyukov
R=iant, dvyukov
CC=golang-codereviews
https://golang.org/cl/102220043
»»»

LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews, r
https://golang.org/cl/108780044

src/pkg/runtime/proc.c
test/fixedbugs/issue8158.go [new file with mode: 0644]

index da2e0f9fa4a62272d0e4f1d1854af3f8a136d7b7..914a02e0bf5492cd9907c5c1b235aea779f2a8e1 100644 (file)
@@ -1459,6 +1459,12 @@ goexit0(G *gp)
        gp->m = nil;
        gp->lockedm = nil;
        gp->paniconfault = 0;
+       gp->defer = nil; // should be true already but just in case.
+       gp->panic = nil; // non-nil for Goexit during panic. points at stack-allocated data.
+       gp->writenbuf = 0;
+       gp->writebuf = nil;
+       gp->waitreason = nil;
+       gp->param = nil;
        m->curg = nil;
        m->lockedg = nil;
        if(m->locked & ~LockExternal) {
diff --git a/test/fixedbugs/issue8158.go b/test/fixedbugs/issue8158.go
new file mode 100644 (file)
index 0000000..b110de1
--- /dev/null
@@ -0,0 +1,41 @@
+// 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.
+
+package main
+
+import (
+       "runtime"
+       "time"
+)
+
+func main() {
+       c := make(chan bool, 1)
+       go f1(c)
+       <-c
+       time.Sleep(10 * time.Millisecond)
+       go f2(c)
+       <-c
+}
+
+func f1(done chan bool) {
+       defer func() {
+               recover()
+               done <- true
+               runtime.Goexit() // left stack-allocated Panic struct on gp->panic stack
+       }()
+       panic("p")
+}
+
+func f2(done chan bool) {
+       defer func() {
+               recover()
+               done <- true
+               runtime.Goexit()
+       }()
+       time.Sleep(10 * time.Millisecond) // overwrote Panic struct with Timer struct
+       runtime.GC()                      // walked gp->panic list, found mangled Panic struct, crashed
+       panic("p")
+}