From cb14e673ec62f09f1216c3d40b03a460785a931e Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Mon, 26 Jul 2021 10:54:57 -0400 Subject: [PATCH] [dev.typeparams] runtime: don't keep stack uintptr across potential stack move Currently, deferproc stores the caller SP as a uintptr in a local variable across a call to newdefer, but newdefer could grow the stack and invalidate this saved SP, causing deferproc to store a stale SP in the defer record. This would lead to us later failing to match that defer to its calling frame, and we wouldn't run the defer at the right time (or possibly at all). It turns out this isn't crashing horribly right now only because the compiler happens to only materialize the result of getcallersp when this variable is used, *after* the call to newdefer. But this is clearly on thin ice, so this CL moves the getcallersp() to the place where we actually need the result. Change-Id: Iae8ab226e03e4482f16acfb965885f0bd83a13b0 Reviewed-on: https://go-review.googlesource.com/c/go/+/337649 Trust: Austin Clements Run-TryBot: Austin Clements TryBot-Result: Go Bot Reviewed-by: Cherry Mui --- src/runtime/panic.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/runtime/panic.go b/src/runtime/panic.go index abf76537b05..85d39b92508 100644 --- a/src/runtime/panic.go +++ b/src/runtime/panic.go @@ -234,9 +234,6 @@ func deferproc(fn *funcval) { // TODO: Make deferproc just take a func(). throw("defer on system stack") } - sp := getcallersp() - callerpc := getcallerpc() - d := newdefer() if d._panic != nil { throw("deferproc: d.panic != nil after newdefer") @@ -244,8 +241,11 @@ func deferproc(fn *funcval) { // TODO: Make deferproc just take a func(). d.link = gp._defer gp._defer = d d.fn = fn - d.pc = callerpc - d.sp = sp + d.pc = getcallerpc() + // We must not be preempted between calling getcallersp and + // storing it to d.sp because getcallersp's result is a + // uintptr stack pointer. + d.sp = getcallersp() // deferproc returns 0 normally. // a deferred func that stops a panic