qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-10-02 06:28:33 -06:00
Code Releases Activity

runtime: avoid bad unwinding from sigpanic in C code

Browse Source 
Currently, if a sigpanic call is injected into C code, it's possible
for preparePanic to leave the stack in a state where traceback can't
unwind correctly past the sigpanic.

Specifically, shouldPushPanic sniffs the stack to decide where to put
the PC from the signal context. In the cgo case, it will find that
!findfunc(pc).valid() because pc is in C code, and then it will check
if the top of the stack looks like a Go PC. However, this stack slot
is just in a C frame, so it could be uninitialized and contain
anything, including what looks like a valid Go PC. For example, in
https://build.golang.org/log/c601a18e2af24794e6c0899e05dddbb08caefc17,
it sees 1c02c23a <runtime.newproc1+682>. When this condition is met,
it skips putting the signal PC on the stack at all. As a result, when
we later unwind from the sigpanic, we'll "successfully" but
incorrectly unwind to whatever PC was in this uninitialized slot and
go who knows where from there.

Fix this by making shouldPushPanic assume that the signal PC is always
usable if we're running C code, so we always make it appear like
sigpanic's caller.

This lets us be pickier again about unexpected return PCs in
gentraceback.

Updates #23640.

Change-Id: I1e8ade24b031bd905d48e92d5e60c982e8edf160
Reviewed-on: https://go-review.googlesource.com/91137
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This commit is contained in:
Austin Clements 2018-01-31 12:05:24 -05:00
parent 615d44c287
commit ddb503be96
2 changed files with 7 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/runtime/panic.go
View File

@ -805,7 +805,12 @@ func shouldPushSigpanic(gp *g, pc, lr uintptr) bool {
// the link register as code, then this assumes the panic was // the link register as code, then this assumes the panic was
// caused by a call to non-code. In this case, we want to // caused by a call to non-code. In this case, we want to
// ignore this call to make unwinding show the context. // ignore this call to make unwinding show the context.
if findfunc(pc).valid() { //
// If we running C code, we're not going to recognize pc as a
// Go function, so just assume it's good. Otherwise, traceback
// may try to read a stale LR that looks like a Go code
// pointer and wander into the woods.
if gp.m.incgo || findfunc(pc).valid() {
// This wasn't a bad call, so use PC as sigpanic's // This wasn't a bad call, so use PC as sigpanic's
// return PC. // return PC.
return true return true

2
src/runtime/traceback.go
View File

@ -287,7 +287,7 @@ func gentraceback(pc0, sp0, lr0 uintptr, gp *g, skip int, pcbuf *uintptr, max in
// But if callback is set, we're doing a garbage collection and must // But if callback is set, we're doing a garbage collection and must
// get everything, so crash loudly. // get everything, so crash loudly.
doPrint := printing doPrint := printing
if doPrint && gp.m.incgo { if doPrint && gp.m.incgo && f.entry == sigpanicPC {
// We can inject sigpanic // We can inject sigpanic
// calls directly into C code, // calls directly into C code,
// in which case we'll see a C // in which case we'll see a C