[release-branch.go1.14] runtime: fix problem with repeated panic/reco…

…ver/re-panics and open-coded defers

In the open-code defer implementation, we add defer struct entries to the defer
chain on-the-fly at panic time to represent stack frames that contain open-coded
defers. This allows us to process non-open-coded and open-coded defers in the
correct order. Also, we need somewhere to be able to store the 'started' state of
open-coded defers. However, if a recover succeeds, defers will now be processed
inline again (unless another panic happens). Any defer entry representing a frame
with open-coded defers will become stale once we run the corresponding defers
inline and exit the associated stack frame. So, we need to remove all entries for
open-coded defers at recover time.

The current code was only removing the top-most open-coded defer from the defer
chain during recovery. However, with recursive functions that do repeated
panic-recover-repanic, multiple stale entries can accumulate on the chain. So, we
just adjust the loop to process the entire chain. Since this is at panic/recover
case, it is fine to scan through the entire chain (which should usually have few
elements in it, since most defers are open-coded).

The added test fails with a SEGV without the fix, because it tries to run a stale
open-code defer entry (and the stack has changed).

Updates #37664.
Fixes #37782.

Change-Id: I8e3da5d610b5e607411451b66881dea887f7484d
Reviewed-on: https://go-review.googlesource.com/c/go/+/222420
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit fae87a2)
Reviewed-on: https://go-review.googlesource.com/c/go/+/222818
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>

src/runtime/defer_test.go

@@ -6,6 +6,7 @@ package runtime_test
 
 import (
  "fmt"
+ "os"
  "reflect"
  "runtime"
  "testing"
@@ -281,3 +282,56 @@ func TestDeferForFuncWithNoExit(t *testing.T) {
  for {
  }
 }
+
+// Test case approximating issue #37664, where a recursive function (interpreter)
+// may do repeated recovers/re-panics until it reaches the frame where the panic
+// can actually be handled. The recurseFnPanicRec() function is testing that there
+// are no stale defer structs on the defer chain after the interpreter() sequence,
+// by writing a bunch of 0xffffffffs into several recursive stack frames, and then
+// doing a single panic-recover which would invoke any such stale defer structs.
+func TestDeferWithRepeatedRepanics(t *testing.T) {
+ interpreter(0, 6, 2)
+ recurseFnPanicRec(0, 10)
+ interpreter(0, 5, 1)
+ recurseFnPanicRec(0, 10)
+ interpreter(0, 6, 3)
+ recurseFnPanicRec(0, 10)
+}
+
+func interpreter(level int, maxlevel int, rec int) {
+ defer func() {
+ e := recover()
+ if e == nil {
+ return
+ }
+ if level != e.(int) {
+ //fmt.Fprintln(os.Stderr, "re-panicing, level", level)
+ panic(e)
+ }
+ //fmt.Fprintln(os.Stderr, "Recovered, level", level)
+ }()
+ if level+1 < maxlevel {
+ interpreter(level+1, maxlevel, rec)
+ } else {
+ //fmt.Fprintln(os.Stderr, "Initiating panic")
+ panic(rec)
+ }
+}
+
+func recurseFnPanicRec(level int, maxlevel int) {
+ defer func() {
+ recover()
+ }()
+ recurseFn(level, maxlevel)
+}
+
+func recurseFn(level int, maxlevel int) {
+ a := [40]uint32{0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff, 0xffffffff}
+ if level+1 < maxlevel {
+ // Need this print statement to keep a around. '_ = a[4]' doesn't do it.
+ fmt.Fprintln(os.Stderr, "recurseFn", level, a[4])
+ recurseFn(level+1, maxlevel)
+ } else {
+ panic("recurseFn panic")
+ }
+}

src/runtime/panic.go

@@ -1003,11 +1003,12 @@ func gopanic(e interface{}) {
  atomic.Xadd(&runningPanicDefers, -1)
 
  if done {
- // Remove any remaining non-started, open-coded defer
- // entry after a recover (there's at most one, if we just
- // ran a non-open-coded defer), since the entry will
- // become out-dated and the defer will be executed
- // normally.
+ // Remove any remaining non-started, open-coded
+ // defer entries after a recover, since the
+ // corresponding defers will be executed normally
+ // (inline). Any such entry will become stale once
+ // we run the corresponding defers inline and exit
+ // the associated stack frame.
  d := gp._defer
  var prev *_defer
  for d != nil {
@@ -1025,8 +1026,9 @@ func gopanic(e interface{}) {
  } else {
  prev.link = d.link
  }
+ newd := d.link
  freedefer(d)
- break
+ d = newd
  } else {
  prev = d
  d = d.link