From 1ea60c136a5782eadb9abf92064e3c1985fe1cdd Mon Sep 17 00:00:00 2001 From: Keith Randall Date: Fri, 2 Dec 2016 15:17:52 -0800 Subject: [PATCH] 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 --- src/runtime/asm_amd64.s | 2 +- src/runtime/stack.go | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/runtime/asm_amd64.s b/src/runtime/asm_amd64.s index 9ffd297d84..0070e9d203 100644 --- 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 ea9a69aa1e..b77a3119c3 100644 --- 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) {