runtime: improve handling of panic during deferred function
authorIan Lance Taylor <ian@gcc.gnu.org>
Fri, 23 Jun 2017 13:45:36 +0000 (13:45 +0000)
committerIan Lance Taylor <ian@gcc.gnu.org>
Fri, 23 Jun 2017 13:45:36 +0000 (13:45 +0000)
    When a panic occurs while processing a deferred function that
    recovered an earlier panic, we shouldn't report the recovered panic
    in the panic stack trace. Stop doing so by keeping track of the panic
    that triggered a defer, marking it as aborted if we see the defer again,
    and discarding aborted panics when a panic is recovered. This is what
    the gc runtime does.

    The test for this is TestRecursivePanic in runtime/crash_test.go.
    We don't run that test yet, but we will soon.

    Reviewed-on: https://go-review.googlesource.com/46461

From-SVN: r249590

gcc/go/gofrontend/MERGE
libgo/go/runtime/panic.go
libgo/go/runtime/runtime2.go

index b6037a6..d82e012 100644 (file)
@@ -1,4 +1,4 @@
-385efb8947af70b8425c833a1ab68ba5f357dfae
+c4adba240f9d5af8ab0534316d6b05bd988c432c
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
index de3c79f..e3d0358 100644 (file)
@@ -91,6 +91,9 @@ func throwinit() {
 // arg is a value to pass to pfn.
 func deferproc(frame *bool, pfn uintptr, arg unsafe.Pointer) {
        d := newdefer()
+       if d._panic != nil {
+               throw("deferproc: d.panic != nil after newdefer")
+       }
        d.frame = frame
        d.panicStack = getg()._panic
        d.pfn = pfn
@@ -338,17 +341,28 @@ func Goexit() {
                if d == nil {
                        break
                }
-               gp._defer = d.link
 
                pfn := d.pfn
+               if pfn == 0 {
+                       if d._panic != nil {
+                               d._panic.aborted = true
+                               d._panic = nil
+                       }
+                       gp._defer = d.link
+                       freedefer(d)
+                       continue
+               }
                d.pfn = 0
 
-               if pfn != 0 {
-                       var fn func(unsafe.Pointer)
-                       *(*uintptr)(unsafe.Pointer(&fn)) = uintptr(unsafe.Pointer(&pfn))
-                       fn(d.arg)
-               }
+               var fn func(unsafe.Pointer)
+               *(*uintptr)(unsafe.Pointer(&fn)) = uintptr(unsafe.Pointer(&pfn))
+               fn(d.arg)
 
+               if gp._defer != d {
+                       throw("bad defer entry in Goexit")
+               }
+               d._panic = nil
+               gp._defer = d.link
                freedefer(d)
                // Note: we ignore recovers here because Goexit isn't a panic
        }
@@ -442,39 +456,71 @@ func gopanic(e interface{}) {
                }
 
                pfn := d.pfn
+
+               // If defer was started by earlier panic or Goexit (and, since we're back here, that triggered a new panic),
+               // take defer off list. The earlier panic or Goexit will not continue running.
+               if pfn == 0 {
+                       if d._panic != nil {
+                               d._panic.aborted = true
+                       }
+                       d._panic = nil
+                       gp._defer = d.link
+                       freedefer(d)
+                       continue
+               }
                d.pfn = 0
 
-               if pfn != 0 {
-                       var fn func(unsafe.Pointer)
-                       *(*uintptr)(unsafe.Pointer(&fn)) = uintptr(unsafe.Pointer(&pfn))
-                       fn(d.arg)
+               // Record the panic that is running the defer.
+               // If there is a new panic during the deferred call, that panic
+               // will find d in the list and will mark d._panic (this panic) aborted.
+               d._panic = p
 
-                       if p.recovered {
-                               // Some deferred function called recover.
-                               // Stop running this panic.
-                               gp._panic = p.link
-
-                               // Unwind the stack by throwing an exception.
-                               // The compiler has arranged to create
-                               // exception handlers in each function
-                               // that uses a defer statement.  These
-                               // exception handlers will check whether
-                               // the entry on the top of the defer stack
-                               // is from the current function.  If it is,
-                               // we have unwound the stack far enough.
-                               unwindStack()
-
-                               throw("unwindStack returned")
+               var fn func(unsafe.Pointer)
+               *(*uintptr)(unsafe.Pointer(&fn)) = uintptr(unsafe.Pointer(&pfn))
+               fn(d.arg)
+
+               if gp._defer != d {
+                       throw("bad defer entry in panic")
+               }
+               d._panic = nil
+
+               if p.recovered {
+                       // Some deferred function called recover.
+                       // Stop running this panic.
+                       gp._panic = p.link
+
+                       // Aborted panics are marked but remain on the g.panic list.
+                       // Remove them from the list.
+                       for gp._panic != nil && gp._panic.aborted {
+                               gp._panic = gp._panic.link
+                       }
+                       if gp._panic == nil { // must be done with signal
+                               gp.sig = 0
                        }
 
-                       // Because we executed that defer function by a panic,
-                       // and it did not call recover, we know that we are
-                       // not returning from the calling function--we are
-                       // panicking through it.
-                       *d.frame = false
+                       // Unwind the stack by throwing an exception.
+                       // The compiler has arranged to create
+                       // exception handlers in each function
+                       // that uses a defer statement.  These
+                       // exception handlers will check whether
+                       // the entry on the top of the defer stack
+                       // is from the current function.  If it is,
+                       // we have unwound the stack far enough.
+                       unwindStack()
+
+                       throw("unwindStack returned")
                }
 
+               // Because we executed that defer function by a panic,
+               // and it did not call recover, we know that we are
+               // not returning from the calling function--we are
+               // panicking through it.
+               *d.frame = false
+
+               // Deferred function did not panic. Remove d.
+               // In the p.recovered case, d will be removed by checkdefer.
                gp._defer = d.link
+
                freedefer(d)
        }
 
index 96a4edb..cdd3fcc 100644 (file)
@@ -700,6 +700,10 @@ type _defer struct {
        // has a defer statement itself.
        panicStack *_panic
 
+       // The panic that caused the defer to run. This is used to
+       // discard panics that have already been handled.
+       _panic *_panic
+
        // The function to call.
        pfn uintptr
 
@@ -733,6 +737,10 @@ type _panic struct {
        // Whether this panic was pushed on the stack because of an
        // exception thrown in some other language.
        isforeign bool
+
+       // Whether this panic was already seen by a deferred function
+       // which called panic again.
+       aborted bool
 }
 
 const (