runtime: scan gp._panic in stack scan

In runtime.gopanic, the _panic object p is stack allocated and
referenced from gp._panic. With stack objects, p on stack is dead
at the point preprintpanics runs. gp._panic points to p, but
stack scan doesn't look at gp. Heap scan of gp does look at
gp._panic, but it stops and ignores the pointer as it points to
the stack. So whatever p points to may be collected and clobbered.
We need to scan gp._panic explicitly during stack scan.

To test it reliably, we introduce a GODEBUG mode "clobberfree",
which clobbers the memory content when the GC frees an object.

Fixes #30150.

Change-Id: I11128298f03a89f817faa221421a9d332b41dced
Reviewed-on: https://go-review.googlesource.com/c/161778
Run-TryBot: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Austin Clements <austin@google.com>

diff --git a/src/runtime/crash_test.go b/src/runtime/crash_test.go
index 6fba4dd91a1daab76788ccfa309ebc05e9f621f1..03ebf022a6ef8d02aa94e4c9ff3be219f88090f5 100644 (file)
--- a/src/runtime/crash_test.go
+++ b/src/runtime/crash_test.go
@@ -728,3 +728,15 @@ func TestG0StackOverflow(t *testing.T) {
 
        runtime.G0StackOverflow()
 }
+
+// Test that panic message is not clobbered.
+// See issue 30150.
+func TestDoublePanic(t *testing.T) {
+       output := runTestProg(t, "testprog", "DoublePanic", "GODEBUG=clobberfree=1")
+       wants := []string{"panic: XXX", "panic: YYY"}
+       for _, want := range wants {
+               if !strings.Contains(output, want) {
+                       t.Errorf("output:\n%s\n\nwant output containing: %s", output, want)
+               }
+       }
+}

diff --git a/src/runtime/extern.go b/src/runtime/extern.go
index af858a331f63cbd8a41a82191d82f9f53d919f8b..437406d9910100eed7225169d25fb6220bac9d9a 100644 (file)
--- a/src/runtime/extern.go
+++ b/src/runtime/extern.go
@@ -27,6 +27,10 @@ It is a comma-separated list of name=val pairs setting these named variables:
        allocfreetrace: setting allocfreetrace=1 causes every allocation to be
        profiled and a stack trace printed on each object's allocation and free.
 
+       clobberfree: setting clobberfree=1 causes the garbage collector to
+       clobber the memory content of an object with bad content when it frees
+       the object.
+
        cgocheck: setting cgocheck=0 disables all checks for packages
        using cgo to incorrectly pass Go pointers to non-Go code.
        Setting cgocheck=1 (the default) enables relatively cheap

diff --git a/src/runtime/mgcmark.go b/src/runtime/mgcmark.go
index 86416caab5288c80cb830fb0e361e7377771132c..022cc8d7d76754ee0d0d64160826b87aa85e12a5 100644 (file)
--- a/src/runtime/mgcmark.go
+++ b/src/runtime/mgcmark.go
@@ -709,7 +709,13 @@ func scanstack(gp *g, gcw *gcWork) {
                return true
        }
        gentraceback(^uintptr(0), ^uintptr(0), 0, gp, 0, nil, 0x7fffffff, scanframe, nil, 0)
+
+       // 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)
+       if gp._panic != nil {
+               state.putPtr(uintptr(unsafe.Pointer(gp._panic)))
+       }
 
        // Find and scan all reachable stack objects.
        state.buildIndex()

diff --git a/src/runtime/mgcsweep.go b/src/runtime/mgcsweep.go
index edb9fcac09026bfb14ab42322eacbd16b7c88f7c..6ac3b031761841af8220b473d20887ba76faaf1a 100644 (file)
--- a/src/runtime/mgcsweep.go
+++ b/src/runtime/mgcsweep.go
@@ -291,7 +291,7 @@ func (s *mspan) sweep(preserve bool) bool {
                }
        }
 
-       if debug.allocfreetrace != 0 || raceenabled || msanenabled {
+       if debug.allocfreetrace != 0 || debug.clobberfree != 0 || raceenabled || msanenabled {
                // Find all newly freed objects. This doesn't have to
                // efficient; allocfreetrace has massive overhead.
                mbits := s.markBitsForBase()
@@ -302,6 +302,9 @@ func (s *mspan) sweep(preserve bool) bool {
                                if debug.allocfreetrace != 0 {
                                        tracefree(unsafe.Pointer(x), size)
                                }
+                               if debug.clobberfree != 0 {
+                                       clobberfree(unsafe.Pointer(x), size)
+                               }
                                if raceenabled {
                                        racefree(unsafe.Pointer(x), size)
                                }
@@ -446,3 +449,12 @@ retry:
                traceGCSweepDone()
        }
 }
+
+// clobberfree sets the memory content at x to bad content, for debugging
+// purposes.
+func clobberfree(x unsafe.Pointer, size uintptr) {
+       // size (span.elemsize) is always a multiple of 4.
+       for i := uintptr(0); i < size; i += 4 {
+               *(*uint32)(add(x, i)) = 0xdeadbeef
+       }
+}

diff --git a/src/runtime/runtime1.go b/src/runtime/runtime1.go
index c5667e73adc4d739bb2a3db684409ccd37da05f1..0c0a31ee6a6644a8807e3e25f211c1f770599990 100644 (file)
--- a/src/runtime/runtime1.go
+++ b/src/runtime/runtime1.go
@@ -301,6 +301,7 @@ type dbgVar struct {
 var debug struct {
        allocfreetrace     int32
        cgocheck           int32
+       clobberfree        int32
        efence             int32
        gccheckmark        int32
        gcpacertrace       int32
@@ -318,6 +319,7 @@ var debug struct {
 
 var dbgvars = []dbgVar{
        {"allocfreetrace", &debug.allocfreetrace},
+       {"clobberfree", &debug.clobberfree},
        {"cgocheck", &debug.cgocheck},
        {"efence", &debug.efence},
        {"gccheckmark", &debug.gccheckmark},

diff --git a/src/runtime/testdata/testprog/crash.go b/src/runtime/testdata/testprog/crash.go
index 4d83132198d27d3d8f4173dc7b701b4bbd9342e9..c4990cdda9a078c1829a8282aab72ca309def9cf 100644 (file)
--- a/src/runtime/testdata/testprog/crash.go
+++ b/src/runtime/testdata/testprog/crash.go
@@ -11,6 +11,7 @@ import (
 
 func init() {
        register("Crash", Crash)
+       register("DoublePanic", DoublePanic)
 }
 
 func test(name string) {
@@ -43,3 +44,23 @@ func Crash() {
        testInNewThread("second-new-thread")
        test("main-again")
 }
+
+type P string
+
+func (p P) String() string {
+       // Try to free the "YYY" string header when the "XXX"
+       // panic is stringified.
+       runtime.GC()
+       runtime.GC()
+       runtime.GC()
+       return string(p)
+}
+
+// Test that panic message is not clobbered.
+// See issue 30150.
+func DoublePanic() {
+       defer func() {
+               panic(P("YYY"))
+       }()
+       panic(P("XXX"))
+}