runtime: on stack copy, adjust BP

When we copy the stack, we need to adjust all BPs.
We correctly adjust the ones on the stack, but we also
need to adjust the one that is in g.sched.bp.

Like CL 33754, no test as only kernel-gathered profiles will notice.
Tests will come (in 1.9) with the implementation of #16638.

The invariant should hold that every frame pointer points to
somewhere within its stack.  After this CL, it is mostly true, but
something about cgo breaks it.  The runtime checks are disabled
until I figure that out.

Update #16638
Fixes #18174

Change-Id: I6023ee64adc80574ee3e76491d4f0fa5ede3dbdb
Reviewed-on: https://go-review.googlesource.com/33895
Reviewed-by: Austin Clements <austin@google.com>

diff --git a/src/runtime/asm_amd64.s b/src/runtime/asm_amd64.s
index 9ffd297d842eab9b5ec98059c36b798137888969..0070e9d203845c244e899453d10d9b33fad54ae6 100644 (file)
--- a/src/runtime/asm_amd64.s
+++ b/src/runtime/asm_amd64.s
@@ -742,7 +742,7 @@ havem:
        MOVQ    (g_sched+gobuf_pc)(SI), BX
        MOVQ    BX, -8(DI)
        // Compute the size of the frame, including return PC and, if
-       // GOEXPERIMENT=framepointer, the saved based pointer
+       // GOEXPERIMENT=framepointer, the saved base pointer
        MOVQ    ctxt+24(FP), BX
        LEAQ    fv+0(FP), AX
        SUBQ    SP, AX

diff --git a/src/runtime/stack.go b/src/runtime/stack.go
index ea9a69aa1e2db22c22c80e798fbb94328a8a6465..b77a3119c3e488e51f8ffd8c37af699ad36ee8aa 100644 (file)
--- a/src/runtime/stack.go
+++ b/src/runtime/stack.go
@@ -123,6 +123,9 @@ const (
        stackPoisonCopy  = 0 // fill stack that should not be accessed with garbage, to detect bad dereferences during copy
 
        stackCache = 1
+
+       // check the BP links during traceback.
+       debugCheckBP = false
 )
 
 const (
@@ -688,6 +691,16 @@ func adjustframe(frame *stkframe, arg unsafe.Pointer) bool {
                if stackDebug >= 3 {
                        print("      saved bp\n")
                }
+               if debugCheckBP {
+                       // Frame pointers should always point to the next higher frame on
+                       // the Go stack (or be nil, for the top frame on the stack).
+                       bp := *(*uintptr)(unsafe.Pointer(frame.varp))
+                       if bp != 0 && (bp < adjinfo.old.lo || bp >= adjinfo.old.hi) {
+                               println("runtime: found invalid frame pointer")
+                               print("bp=", hex(bp), " min=", hex(adjinfo.old.lo), " max=", hex(adjinfo.old.hi), "\n")
+                               throw("bad frame pointer")
+                       }
+               }
                adjustpointer(adjinfo, unsafe.Pointer(frame.varp))
        }
 
@@ -719,6 +732,18 @@ func adjustframe(frame *stkframe, arg unsafe.Pointer) bool {
 
 func adjustctxt(gp *g, adjinfo *adjustinfo) {
        adjustpointer(adjinfo, unsafe.Pointer(&gp.sched.ctxt))
+       if !framepointer_enabled {
+               return
+       }
+       if debugCheckBP {
+               bp := gp.sched.bp
+               if bp != 0 && (bp < adjinfo.old.lo || bp >= adjinfo.old.hi) {
+                       println("runtime: found invalid top frame pointer")
+                       print("bp=", hex(bp), " min=", hex(adjinfo.old.lo), " max=", hex(adjinfo.old.hi), "\n")
+                       throw("bad top frame pointer")
+               }
+       }
+       adjustpointer(adjinfo, unsafe.Pointer(&gp.sched.bp))
 }
 
 func adjustdefers(gp *g, adjinfo *adjustinfo) {