Skip to content

Commit 2b82a4f

Browse files
committed
runtime: track frame pointer while in syscall
Currently the runtime only tracks the PC and SP upon entering a syscall, but not the FP (BP). This is mainly for historical reasons, and because the tracer (which uses the frame pointer unwinder) does not need it. Until it did, of course, in CL 567076, where the tracer tries to take a stack trace of a goroutine that's in a syscall from afar. It tries to use gp.sched.bp and lots of things go wrong. It *really* should be using the equivalent of gp.syscallbp, which doesn't exist before this CL. This change introduces gp.syscallbp and tracks it. It also introduces getcallerfp which is nice for simplifying some code. Because we now have gp.syscallbp, we can also delete the frame skip count computation in traceLocker.GoSysCall, because it's now the same regardless of whether frame pointer unwinding is used. Fixes #66889. Change-Id: Ib6d761c9566055e0a037134138cb0f81be73ecf7 Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-nocgo Reviewed-on: https://go-review.googlesource.com/c/go/+/580255 Auto-Submit: Michael Knyszek <[email protected]> Reviewed-by: Michael Pratt <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Cherry Mui <[email protected]>
1 parent dcb5de5 commit 2b82a4f

File tree

7 files changed

+54
-40
lines changed

7 files changed

+54
-40
lines changed

src/runtime/cgocall.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,7 @@ func cgocallbackg(fn, frame unsafe.Pointer, ctxt uintptr) {
314314
// save syscall* and let reentersyscall restore them.
315315
savedsp := unsafe.Pointer(gp.syscallsp)
316316
savedpc := gp.syscallpc
317+
savedbp := gp.syscallbp
317318
exitsyscall() // coming out of cgo call
318319
gp.m.incgo = false
319320
if gp.m.isextra {
@@ -345,7 +346,7 @@ func cgocallbackg(fn, frame unsafe.Pointer, ctxt uintptr) {
345346
osPreemptExtEnter(gp.m)
346347

347348
// going back to cgo call
348-
reentersyscall(savedpc, uintptr(savedsp))
349+
reentersyscall(savedpc, uintptr(savedsp), savedbp)
349350

350351
gp.m.winsyscall = winsyscall
351352
}

src/runtime/export_windows_test.go

+1-6
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,6 @@ func NewContextStub() *ContextStub {
3333
var ctx context
3434
ctx.set_ip(getcallerpc())
3535
ctx.set_sp(getcallersp())
36-
fp := getfp()
37-
// getfp is not implemented on windows/386 and windows/arm,
38-
// in which case it returns 0.
39-
if fp != 0 {
40-
ctx.set_fp(*(*uintptr)(unsafe.Pointer(fp)))
41-
}
36+
ctx.set_fp(getcallerfp())
4237
return &ContextStub{ctx}
4338
}

src/runtime/proc.go

+19-11
Original file line numberDiff line numberDiff line change
@@ -4237,7 +4237,7 @@ func gdestroy(gp *g) {
42374237
//
42384238
//go:nosplit
42394239
//go:nowritebarrierrec
4240-
func save(pc, sp uintptr) {
4240+
func save(pc, sp, bp uintptr) {
42414241
gp := getg()
42424242

42434243
if gp == gp.m.g0 || gp == gp.m.gsignal {
@@ -4253,6 +4253,7 @@ func save(pc, sp uintptr) {
42534253
gp.sched.sp = sp
42544254
gp.sched.lr = 0
42554255
gp.sched.ret = 0
4256+
gp.sched.bp = bp
42564257
// We need to ensure ctxt is zero, but can't have a write
42574258
// barrier here. However, it should always already be zero.
42584259
// Assert that.
@@ -4285,7 +4286,7 @@ func save(pc, sp uintptr) {
42854286
// entry point for syscalls, which obtains the SP and PC from the caller.
42864287
//
42874288
//go:nosplit
4288-
func reentersyscall(pc, sp uintptr) {
4289+
func reentersyscall(pc, sp, bp uintptr) {
42894290
trace := traceAcquire()
42904291
gp := getg()
42914292

@@ -4301,14 +4302,15 @@ func reentersyscall(pc, sp uintptr) {
43014302
gp.throwsplit = true
43024303

43034304
// Leave SP around for GC and traceback.
4304-
save(pc, sp)
4305+
save(pc, sp, bp)
43054306
gp.syscallsp = sp
43064307
gp.syscallpc = pc
4308+
gp.syscallbp = bp
43074309
casgstatus(gp, _Grunning, _Gsyscall)
43084310
if staticLockRanking {
43094311
// When doing static lock ranking casgstatus can call
43104312
// systemstack which clobbers g.sched.
4311-
save(pc, sp)
4313+
save(pc, sp, bp)
43124314
}
43134315
if gp.syscallsp < gp.stack.lo || gp.stack.hi < gp.syscallsp {
43144316
systemstack(func() {
@@ -4325,18 +4327,18 @@ func reentersyscall(pc, sp uintptr) {
43254327
// systemstack itself clobbers g.sched.{pc,sp} and we might
43264328
// need them later when the G is genuinely blocked in a
43274329
// syscall
4328-
save(pc, sp)
4330+
save(pc, sp, bp)
43294331
}
43304332

43314333
if sched.sysmonwait.Load() {
43324334
systemstack(entersyscall_sysmon)
4333-
save(pc, sp)
4335+
save(pc, sp, bp)
43344336
}
43354337

43364338
if gp.m.p.ptr().runSafePointFn != 0 {
43374339
// runSafePointFn may stack split if run on this stack
43384340
systemstack(runSafePointFn)
4339-
save(pc, sp)
4341+
save(pc, sp, bp)
43404342
}
43414343

43424344
gp.m.syscalltick = gp.m.p.ptr().syscalltick
@@ -4347,7 +4349,7 @@ func reentersyscall(pc, sp uintptr) {
43474349
atomic.Store(&pp.status, _Psyscall)
43484350
if sched.gcwaiting.Load() {
43494351
systemstack(entersyscall_gcwait)
4350-
save(pc, sp)
4352+
save(pc, sp, bp)
43514353
}
43524354

43534355
gp.m.locks--
@@ -4360,7 +4362,12 @@ func reentersyscall(pc, sp uintptr) {
43604362
//go:nosplit
43614363
//go:linkname entersyscall
43624364
func entersyscall() {
4363-
reentersyscall(getcallerpc(), getcallersp())
4365+
// N.B. getcallerfp cannot be written directly as argument in the call
4366+
// to reentersyscall because it forces spilling the other arguments to
4367+
// the stack. This results in exceeding the nosplit stack requirements
4368+
// on some platforms.
4369+
fp := getcallerfp()
4370+
reentersyscall(getcallerpc(), getcallersp(), fp)
43644371
}
43654372

43664373
func entersyscall_sysmon() {
@@ -4418,7 +4425,8 @@ func entersyscallblock() {
44184425
// Leave SP around for GC and traceback.
44194426
pc := getcallerpc()
44204427
sp := getcallersp()
4421-
save(pc, sp)
4428+
bp := getcallerfp()
4429+
save(pc, sp, bp)
44224430
gp.syscallsp = gp.sched.sp
44234431
gp.syscallpc = gp.sched.pc
44244432
if gp.syscallsp < gp.stack.lo || gp.stack.hi < gp.syscallsp {
@@ -4441,7 +4449,7 @@ func entersyscallblock() {
44414449
systemstack(entersyscallblock_handoff)
44424450

44434451
// Resave for traceback during blocked call.
4444-
save(getcallerpc(), getcallersp())
4452+
save(getcallerpc(), getcallersp(), getcallerfp())
44454453

44464454
gp.m.locks--
44474455
}

src/runtime/runtime2.go

+15
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,7 @@ type g struct {
437437
sched gobuf
438438
syscallsp uintptr // if status==Gsyscall, syscallsp = sched.sp to use during gc
439439
syscallpc uintptr // if status==Gsyscall, syscallpc = sched.pc to use during gc
440+
syscallbp uintptr // if status==Gsyscall, syscallbp = sched.bp to use in fpTraceback
440441
stktopsp uintptr // expected sp at top of stack, to check in traceback
441442
// param is a generic pointer parameter field used to pass
442443
// values in particular contexts where other storage for the
@@ -1263,3 +1264,17 @@ var (
12631264

12641265
// Must agree with internal/buildcfg.FramePointerEnabled.
12651266
const framepointer_enabled = GOARCH == "amd64" || GOARCH == "arm64"
1267+
1268+
// getcallerfp returns the frame pointer of the caller of the caller
1269+
// of this function.
1270+
//
1271+
//go:nosplit
1272+
//go:noinline
1273+
func getcallerfp() uintptr {
1274+
fp := getfp() // This frame's FP.
1275+
if fp != 0 {
1276+
fp = *(*uintptr)(unsafe.Pointer(fp)) // The caller's FP.
1277+
fp = *(*uintptr)(unsafe.Pointer(fp)) // The caller's caller's FP.
1278+
}
1279+
return fp
1280+
}

src/runtime/sizeof_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ func TestSizeof(t *testing.T) {
2020
_32bit uintptr // size on 32bit platforms
2121
_64bit uintptr // size on 64bit platforms
2222
}{
23-
{runtime.G{}, 268, 432}, // g, but exported for testing
23+
{runtime.G{}, 272, 440}, // g, but exported for testing
2424
{runtime.Sudog{}, 56, 88}, // sudog, but exported for testing
2525
}
2626

src/runtime/traceruntime.go

+1-16
Original file line numberDiff line numberDiff line change
@@ -481,25 +481,10 @@ func emitUnblockStatus(w traceWriter, gp *g, gen uintptr) traceWriter {
481481
//
482482
// Must be called with a valid P.
483483
func (tl traceLocker) GoSysCall() {
484-
var skip int
485-
switch {
486-
case tracefpunwindoff():
487-
// Unwind by skipping 1 frame relative to gp.syscallsp which is captured 3
488-
// results by hard coding the number of frames in between our caller and the
489-
// actual syscall, see cases below.
490-
// TODO(felixge): Implement gp.syscallbp to avoid this workaround?
491-
skip = 1
492-
case GOOS == "solaris" || GOOS == "illumos":
493-
// These platforms don't use a libc_read_trampoline.
494-
skip = 3
495-
default:
496-
// Skip the extra trampoline frame used on most systems.
497-
skip = 4
498-
}
499484
// Scribble down the M that the P is currently attached to.
500485
pp := tl.mp.p.ptr()
501486
pp.trace.mSyscallID = int64(tl.mp.procid)
502-
tl.eventWriter(traceGoRunning, traceProcRunning).commit(traceEvGoSyscallBegin, pp.trace.nextSeq(tl.gen), tl.stack(skip))
487+
tl.eventWriter(traceGoRunning, traceProcRunning).commit(traceEvGoSyscallBegin, pp.trace.nextSeq(tl.gen), tl.stack(1))
503488
}
504489

505490
// GoSysExit emits a GoSyscallEnd event, possibly along with a GoSyscallBlocked event

src/runtime/tracestack.go

+15-5
Original file line numberDiff line numberDiff line change
@@ -92,19 +92,29 @@ func traceStack(skip int, gp *g, gen uintptr) uint64 {
9292
if getg() == gp {
9393
nstk += fpTracebackPCs(unsafe.Pointer(getfp()), pcBuf[1:])
9494
} else if gp != nil {
95-
// Two cases:
95+
// Three cases:
9696
//
9797
// (1) We're called on the g0 stack through mcall(fn) or systemstack(fn). To
9898
// behave like gcallers above, we start unwinding from sched.bp, which
9999
// points to the caller frame of the leaf frame on g's stack. The return
100100
// address of the leaf frame is stored in sched.pc, which we manually
101101
// capture here.
102102
//
103-
// (2) We're called against a gp that we're not currently executing on, in
104-
// which case it's currently not executing. gp.sched contains the most up-to-date
103+
// (2) We're called against a gp that we're not currently executing on, but that isn't
104+
// in a syscall, in which case it's currently not executing. gp.sched contains the most
105+
// up-to-date information about where it stopped, and like case (1), we match gcallers
106+
// here.
107+
//
108+
// (3) We're called against a gp that we're not currently executing on, but that is in
109+
// a syscall, in which case gp.syscallsp != 0. gp.syscall* contains the most up-to-date
105110
// information about where it stopped, and like case (1), we match gcallers here.
106-
pcBuf[1] = gp.sched.pc
107-
nstk += 1 + fpTracebackPCs(unsafe.Pointer(gp.sched.bp), pcBuf[2:])
111+
if gp.syscallsp != 0 {
112+
pcBuf[1] = gp.syscallpc
113+
nstk += 1 + fpTracebackPCs(unsafe.Pointer(gp.syscallbp), pcBuf[2:])
114+
} else {
115+
pcBuf[1] = gp.sched.pc
116+
nstk += 1 + fpTracebackPCs(unsafe.Pointer(gp.sched.bp), pcBuf[2:])
117+
}
108118
}
109119
}
110120
if nstk > 0 {

0 commit comments

Comments
 (0)