]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: track frame pointer while in syscall
authorMichael Anthony Knyszek <mknyszek@google.com>
Thu, 18 Apr 2024 20:54:55 +0000 (20:54 +0000)
committerMichael Knyszek <mknyszek@google.com>
Fri, 19 Apr 2024 17:25:00 +0000 (17:25 +0000)
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 <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
src/runtime/cgocall.go
src/runtime/export_windows_test.go
src/runtime/proc.go
src/runtime/runtime2.go
src/runtime/sizeof_test.go
src/runtime/traceruntime.go
src/runtime/tracestack.go

index 19de06fd85ee362cf25af4a1e3213cb73945259d..8f09b6831b5d1ba3760a478342f88fd05a0c89ad 100644 (file)
@@ -314,6 +314,7 @@ func cgocallbackg(fn, frame unsafe.Pointer, ctxt uintptr) {
        // save syscall* and let reentersyscall restore them.
        savedsp := unsafe.Pointer(gp.syscallsp)
        savedpc := gp.syscallpc
+       savedbp := gp.syscallbp
        exitsyscall() // coming out of cgo call
        gp.m.incgo = false
        if gp.m.isextra {
@@ -345,7 +346,7 @@ func cgocallbackg(fn, frame unsafe.Pointer, ctxt uintptr) {
        osPreemptExtEnter(gp.m)
 
        // going back to cgo call
-       reentersyscall(savedpc, uintptr(savedsp))
+       reentersyscall(savedpc, uintptr(savedsp), savedbp)
 
        gp.m.winsyscall = winsyscall
 }
index cf0db576b80babeb0f92da1d892bd0e5f102f6b0..4880e62a550ee09eff82adeca7453c79a96d72ae 100644 (file)
@@ -33,11 +33,6 @@ func NewContextStub() *ContextStub {
        var ctx context
        ctx.set_ip(getcallerpc())
        ctx.set_sp(getcallersp())
-       fp := getfp()
-       // getfp is not implemented on windows/386 and windows/arm,
-       // in which case it returns 0.
-       if fp != 0 {
-               ctx.set_fp(*(*uintptr)(unsafe.Pointer(fp)))
-       }
+       ctx.set_fp(getcallerfp())
        return &ContextStub{ctx}
 }
index 8f5787dbbbe332b9457de39f0a674d5f76c16cab..cb5a80455df0918a1fc72ae354a22832eaf697a6 100644 (file)
@@ -4237,7 +4237,7 @@ func gdestroy(gp *g) {
 //
 //go:nosplit
 //go:nowritebarrierrec
-func save(pc, sp uintptr) {
+func save(pc, sp, bp uintptr) {
        gp := getg()
 
        if gp == gp.m.g0 || gp == gp.m.gsignal {
@@ -4253,6 +4253,7 @@ func save(pc, sp uintptr) {
        gp.sched.sp = sp
        gp.sched.lr = 0
        gp.sched.ret = 0
+       gp.sched.bp = bp
        // We need to ensure ctxt is zero, but can't have a write
        // barrier here. However, it should always already be zero.
        // Assert that.
@@ -4285,7 +4286,7 @@ func save(pc, sp uintptr) {
 // entry point for syscalls, which obtains the SP and PC from the caller.
 //
 //go:nosplit
-func reentersyscall(pc, sp uintptr) {
+func reentersyscall(pc, sp, bp uintptr) {
        trace := traceAcquire()
        gp := getg()
 
@@ -4301,14 +4302,15 @@ func reentersyscall(pc, sp uintptr) {
        gp.throwsplit = true
 
        // Leave SP around for GC and traceback.
-       save(pc, sp)
+       save(pc, sp, bp)
        gp.syscallsp = sp
        gp.syscallpc = pc
+       gp.syscallbp = bp
        casgstatus(gp, _Grunning, _Gsyscall)
        if staticLockRanking {
                // When doing static lock ranking casgstatus can call
                // systemstack which clobbers g.sched.
-               save(pc, sp)
+               save(pc, sp, bp)
        }
        if gp.syscallsp < gp.stack.lo || gp.stack.hi < gp.syscallsp {
                systemstack(func() {
@@ -4325,18 +4327,18 @@ func reentersyscall(pc, sp uintptr) {
                // systemstack itself clobbers g.sched.{pc,sp} and we might
                // need them later when the G is genuinely blocked in a
                // syscall
-               save(pc, sp)
+               save(pc, sp, bp)
        }
 
        if sched.sysmonwait.Load() {
                systemstack(entersyscall_sysmon)
-               save(pc, sp)
+               save(pc, sp, bp)
        }
 
        if gp.m.p.ptr().runSafePointFn != 0 {
                // runSafePointFn may stack split if run on this stack
                systemstack(runSafePointFn)
-               save(pc, sp)
+               save(pc, sp, bp)
        }
 
        gp.m.syscalltick = gp.m.p.ptr().syscalltick
@@ -4347,7 +4349,7 @@ func reentersyscall(pc, sp uintptr) {
        atomic.Store(&pp.status, _Psyscall)
        if sched.gcwaiting.Load() {
                systemstack(entersyscall_gcwait)
-               save(pc, sp)
+               save(pc, sp, bp)
        }
 
        gp.m.locks--
@@ -4360,7 +4362,12 @@ func reentersyscall(pc, sp uintptr) {
 //go:nosplit
 //go:linkname entersyscall
 func entersyscall() {
-       reentersyscall(getcallerpc(), getcallersp())
+       // N.B. getcallerfp cannot be written directly as argument in the call
+       // to reentersyscall because it forces spilling the other arguments to
+       // the stack. This results in exceeding the nosplit stack requirements
+       // on some platforms.
+       fp := getcallerfp()
+       reentersyscall(getcallerpc(), getcallersp(), fp)
 }
 
 func entersyscall_sysmon() {
@@ -4418,7 +4425,8 @@ func entersyscallblock() {
        // Leave SP around for GC and traceback.
        pc := getcallerpc()
        sp := getcallersp()
-       save(pc, sp)
+       bp := getcallerfp()
+       save(pc, sp, bp)
        gp.syscallsp = gp.sched.sp
        gp.syscallpc = gp.sched.pc
        if gp.syscallsp < gp.stack.lo || gp.stack.hi < gp.syscallsp {
@@ -4441,7 +4449,7 @@ func entersyscallblock() {
        systemstack(entersyscallblock_handoff)
 
        // Resave for traceback during blocked call.
-       save(getcallerpc(), getcallersp())
+       save(getcallerpc(), getcallersp(), getcallerfp())
 
        gp.m.locks--
 }
index 83252abb441a28f4936b6eebac9ea7dd570a4d72..b58255f279bf052a673ac64286d05848923cc075 100644 (file)
@@ -437,6 +437,7 @@ type g struct {
        sched     gobuf
        syscallsp uintptr // if status==Gsyscall, syscallsp = sched.sp to use during gc
        syscallpc uintptr // if status==Gsyscall, syscallpc = sched.pc to use during gc
+       syscallbp uintptr // if status==Gsyscall, syscallbp = sched.bp to use in fpTraceback
        stktopsp  uintptr // expected sp at top of stack, to check in traceback
        // param is a generic pointer parameter field used to pass
        // values in particular contexts where other storage for the
@@ -1263,3 +1264,17 @@ var (
 
 // Must agree with internal/buildcfg.FramePointerEnabled.
 const framepointer_enabled = GOARCH == "amd64" || GOARCH == "arm64"
+
+// getcallerfp returns the frame pointer of the caller of the caller
+// of this function.
+//
+//go:nosplit
+//go:noinline
+func getcallerfp() uintptr {
+       fp := getfp() // This frame's FP.
+       if fp != 0 {
+               fp = *(*uintptr)(unsafe.Pointer(fp)) // The caller's FP.
+               fp = *(*uintptr)(unsafe.Pointer(fp)) // The caller's caller's FP.
+       }
+       return fp
+}
index 0ef916b044faa945f55140e346183e927e61f6d6..d235d6a3f80d207679a8d8430407623eb3bde09e 100644 (file)
@@ -20,7 +20,7 @@ func TestSizeof(t *testing.T) {
                _32bit uintptr // size on 32bit platforms
                _64bit uintptr // size on 64bit platforms
        }{
-               {runtime.G{}, 268, 432},   // g, but exported for testing
+               {runtime.G{}, 272, 440},   // g, but exported for testing
                {runtime.Sudog{}, 56, 88}, // sudog, but exported for testing
        }
 
index dcaea364e25dd9ad9ef42972337f5ec66bd077b7..3e0e3b3a76a2196e6456d5a987db48ea434ae53d 100644 (file)
@@ -481,25 +481,10 @@ func emitUnblockStatus(w traceWriter, gp *g, gen uintptr) traceWriter {
 //
 // Must be called with a valid P.
 func (tl traceLocker) GoSysCall() {
-       var skip int
-       switch {
-       case tracefpunwindoff():
-               // Unwind by skipping 1 frame relative to gp.syscallsp which is captured 3
-               // results by hard coding the number of frames in between our caller and the
-               // actual syscall, see cases below.
-               // TODO(felixge): Implement gp.syscallbp to avoid this workaround?
-               skip = 1
-       case GOOS == "solaris" || GOOS == "illumos":
-               // These platforms don't use a libc_read_trampoline.
-               skip = 3
-       default:
-               // Skip the extra trampoline frame used on most systems.
-               skip = 4
-       }
        // Scribble down the M that the P is currently attached to.
        pp := tl.mp.p.ptr()
        pp.trace.mSyscallID = int64(tl.mp.procid)
-       tl.eventWriter(traceGoRunning, traceProcRunning).commit(traceEvGoSyscallBegin, pp.trace.nextSeq(tl.gen), tl.stack(skip))
+       tl.eventWriter(traceGoRunning, traceProcRunning).commit(traceEvGoSyscallBegin, pp.trace.nextSeq(tl.gen), tl.stack(1))
 }
 
 // GoSysExit emits a GoSyscallEnd event, possibly along with a GoSyscallBlocked event
index f651a1fca97bbe16bde63556e357fde18c3452db..04b935a2c9a383e2a5146f5df2f37f87153c2932 100644 (file)
@@ -92,7 +92,7 @@ func traceStack(skip int, gp *g, gen uintptr) uint64 {
                if getg() == gp {
                        nstk += fpTracebackPCs(unsafe.Pointer(getfp()), pcBuf[1:])
                } else if gp != nil {
-                       // Two cases:
+                       // Three cases:
                        //
                        // (1) We're called on the g0 stack through mcall(fn) or systemstack(fn). To
                        // behave like gcallers above, we start unwinding from sched.bp, which
@@ -100,11 +100,21 @@ func traceStack(skip int, gp *g, gen uintptr) uint64 {
                        // address of the leaf frame is stored in sched.pc, which we manually
                        // capture here.
                        //
-                       // (2) We're called against a gp that we're not currently executing on, in
-                       // which case it's currently not executing. gp.sched contains the most up-to-date
+                       // (2) We're called against a gp that we're not currently executing on, but that isn't
+                       // in a syscall, in which case it's currently not executing. gp.sched contains the most
+                       // up-to-date information about where it stopped, and like case (1), we match gcallers
+                       // here.
+                       //
+                       // (3) We're called against a gp that we're not currently executing on, but that is in
+                       // a syscall, in which case gp.syscallsp != 0. gp.syscall* contains the most up-to-date
                        // information about where it stopped, and like case (1), we match gcallers here.
-                       pcBuf[1] = gp.sched.pc
-                       nstk += 1 + fpTracebackPCs(unsafe.Pointer(gp.sched.bp), pcBuf[2:])
+                       if gp.syscallsp != 0 {
+                               pcBuf[1] = gp.syscallpc
+                               nstk += 1 + fpTracebackPCs(unsafe.Pointer(gp.syscallbp), pcBuf[2:])
+                       } else {
+                               pcBuf[1] = gp.sched.pc
+                               nstk += 1 + fpTracebackPCs(unsafe.Pointer(gp.sched.bp), pcBuf[2:])
+                       }
                }
        }
        if nstk > 0 {