[release-branch.go1.12] runtime: scan defer closure in stack scan

With stack objects, when we scan the stack, it scans defers with
tracebackdefers, but it seems to me that tracebackdefers doesn't
include the func value itself, which could be a stack allocated
closure. Scan it explicitly.

Alternatively, we can change tracebackdefers to include the func
value, which in turn needs to change the type of stkframe.

Updates #30453.
Fixes #30470.

Change-Id: I55a6e43264d6952ab2fa5c638bebb89fdc410e2b
Reviewed-on: https://go-review.googlesource.com/c/164118
Reviewed-by: Keith Randall <khr@golang.org>
(cherry picked from commit 4f4c2a79d4f952b96d58aec2926b4c894245071b)
Reviewed-on: https://go-review.googlesource.com/c/164629
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

diff --git a/src/runtime/mgcmark.go b/src/runtime/mgcmark.go
index 022cc8d7d76754ee0d0d64160826b87aa85e12a5..cc4e7d06d30e7ecc36af1a8dd61784fd0c30d2b2 100644 (file)
--- a/src/runtime/mgcmark.go
+++ b/src/runtime/mgcmark.go
@@ -713,6 +713,13 @@ func scanstack(gp *g, gcw *gcWork) {
        // Find additional pointers that point into the stack from the heap.
        // Currently this includes defers and panics. See also function copystack.
        tracebackdefers(gp, scanframe, nil)
+       for d := gp._defer; d != nil; d = d.link {
+               // tracebackdefers above does not scan the func value, which could
+               // be a stack allocated closure. See issue 30453.
+               if d.fn != nil {
+                       scanblock(uintptr(unsafe.Pointer(&d.fn)), sys.PtrSize, &oneptrmask[0], gcw, &state)
+               }
+       }
        if gp._panic != nil {
                state.putPtr(uintptr(unsafe.Pointer(gp._panic)))
        }

diff --git a/src/runtime/stack_test.go b/src/runtime/stack_test.go
index f52381710dff622f89cddefbb65ca80ba9bc027a..7bc63967bba074a7fdef175e7c64a07d86073e8e 100644 (file)
--- a/src/runtime/stack_test.go
+++ b/src/runtime/stack_test.go
@@ -787,3 +787,11 @@ func TestTracebackAncestors(t *testing.T) {
                }
        }
 }
+
+// Test that defer closure is correctly scanned when the stack is scanned.
+func TestDeferLiveness(t *testing.T) {
+       output := runTestProg(t, "testprog", "DeferLiveness", "GODEBUG=clobberfree=1")
+       if output != "" {
+               t.Errorf("output:\n%s\n\nwant no output", output)
+       }
+}

diff --git a/src/runtime/testdata/testprog/gc.go b/src/runtime/testdata/testprog/gc.go
index fdf08be7e91714c9bf3e858aa82ee091cd8e34f9..ea6604f13214a79c0deb6afbadeaf0ab60dabe9a 100644 (file)
--- a/src/runtime/testdata/testprog/gc.go
+++ b/src/runtime/testdata/testprog/gc.go
@@ -18,6 +18,7 @@ func init() {
        register("GCFairness2", GCFairness2)
        register("GCSys", GCSys)
        register("GCPhys", GCPhys)
+       register("DeferLiveness", DeferLiveness)
 }
 
 func GCSys() {
@@ -207,3 +208,25 @@ func GCPhys() {
        fmt.Println("OK")
        runtime.KeepAlive(saved)
 }
+
+// Test that defer closure is correctly scanned when the stack is scanned.
+func DeferLiveness() {
+       var x [10]int
+       escape(&x)
+       fn := func() {
+               if x[0] != 42 {
+                       panic("FAIL")
+               }
+       }
+       defer fn()
+
+       x[0] = 42
+       runtime.GC()
+       runtime.GC()
+       runtime.GC()
+}
+
+//go:noinline
+func escape(x interface{}) { sink2 = x; sink2 = nil }
+
+var sink2 interface{}