[go] runtime: remove the pc field of _defer struct
10 views
Skip to first unread message
Youlin Feng (Gerrit)
Oct 30, 2025, 11:08:46 PM (11 days ago) Oct 30
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Youlin Feng has uploaded the change for review![]( "Open in Gerrit")
Commit message
runtime: remove the pc field of _defer struct
Since we always can get the address of `CALL runtime.deferreturn(SB)`
from the unwinder, so it is not necessary to record the caller's pc
in the _defer struct. For the stack allocated _defer, this CL makes
the frame smaller.
Change-Id: I0fd347e4bc07cf8a9b954816323df30fc52552b6
Change diff
diff --git a/src/cmd/compile/internal/ssagen/ssa.go b/src/cmd/compile/internal/ssagen/ssa.go
index ae7d575..db2ffb5 100644
--- a/src/cmd/compile/internal/ssagen/ssa.go
+++ b/src/cmd/compile/internal/ssagen/ssa.go
@@ -7797,7 +7797,7 @@
}
// deferStructFnField is the field index of _defer.fn.
-const deferStructFnField = 4
+const deferStructFnField = 3
var deferType *types.Type
@@ -7817,7 +7817,6 @@
makefield("heap", types.Types[types.TBOOL]),
makefield("rangefunc", types.Types[types.TBOOL]),
makefield("sp", types.Types[types.TUINTPTR]),
- makefield("pc", types.Types[types.TUINTPTR]),
// Note: the types here don't really matter. Defer structures
// are always scanned explicitly during stack copying and GC,
// so we make them uintptr type even though they are real pointers.
diff --git a/src/runtime/heapdump.go b/src/runtime/heapdump.go
index 3671c65..3a8c374 100644
--- a/src/runtime/heapdump.go
+++ b/src/runtime/heapdump.go
@@ -382,7 +382,6 @@
dumpint(uint64(uintptr(unsafe.Pointer(d))))
dumpint(uint64(uintptr(unsafe.Pointer(gp))))
dumpint(uint64(d.sp))
- dumpint(uint64(d.pc))
fn := *(**funcval)(unsafe.Pointer(&d.fn))
dumpint(uint64(uintptr(unsafe.Pointer(fn))))
if d.fn == nil {
diff --git a/src/runtime/panic.go b/src/runtime/panic.go
index 175452f..aee1cd0 100644
--- a/src/runtime/panic.go
+++ b/src/runtime/panic.go
@@ -354,7 +354,6 @@
d.link = gp._defer
gp._defer = d
d.fn = fn
- d.pc = sys.GetCallerPC()
// We must not be preempted between calling GetCallerSP and
// storing it to d.sp because GetCallerSP's result is a
// uintptr stack pointer.
@@ -458,7 +457,6 @@
d := newdefer()
d.link = gp._defer
gp._defer = d
- d.pc = sys.GetCallerPC()
// We must not be preempted between calling GetCallerSP and
// storing it to d.sp because GetCallerSP's result is a
// uintptr stack pointer.
@@ -518,7 +516,6 @@
}
for d1 := d; ; d1 = d1.link {
d1.sp = d0.sp
- d1.pc = d0.pc
if d1.link == nil {
d1.link = tail
break
@@ -547,7 +544,6 @@
d.heap = false
d.rangefunc = false
d.sp = sys.GetCallerSP()
- d.pc = sys.GetCallerPC()
// The lines below implement:
// d.panic = nil
// d.fd = nil
@@ -977,8 +973,6 @@
fn := d.fn
- p.retpc = d.pc
-
// Unlink and free.
popDefer(gp)
@@ -1018,6 +1012,12 @@
// it's non-zero.
if u.frame.sp == limit {
+ f := u.frame.fn
+ if f.deferreturn == 0 {
+ throw("no deferreturn")
+ }
+ p.retpc = f.entry() + uintptr(f.deferreturn)
+
break // found a frame with linked defers
}
@@ -1273,15 +1273,6 @@
pc, sp, fp := p.retpc, uintptr(p.sp), uintptr(p.fp)
p0, saveOpenDeferState := p, p.deferBitsPtr != nil && *p.deferBitsPtr != 0
- // The linker records the f-relative address of a call to deferreturn in f's funcInfo.
- // Assuming a "normal" call to recover() inside one of f's deferred functions
- // invoked for a panic, that is the desired PC for exiting f.
- f := findfunc(pc)
- if f.deferreturn == 0 {
- throw("no deferreturn")
- }
- gotoPc := f.entry() + uintptr(f.deferreturn)
-
// Unwind the panic stack.
for ; p != nil && uintptr(p.startSP) < sp; p = p.link {
// Don't allow jumping past a pending Goexit.
@@ -1304,7 +1295,7 @@
// With how subtle defer handling is, this might not actually be
// worthwhile though.
if p.goexit {
- gotoPc, sp = p.startPC, uintptr(p.startSP)
+ pc, sp = p.startPC, uintptr(p.startSP)
saveOpenDeferState = false // goexit is unwinding the stack anyway
break
}
@@ -1367,7 +1358,7 @@
// branch directly to the deferreturn
gp.sched.sp = sp
- gp.sched.pc = gotoPc
+ gp.sched.pc = pc
gp.sched.lr = 0
// Restore the bp on platforms that support frame pointers.
// N.B. It's fine to not set anything for platforms that don't
diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go
index b346337..3672b19 100644
--- a/src/runtime/runtime2.go
+++ b/src/runtime/runtime2.go
@@ -1090,7 +1090,6 @@
heap bool
rangefunc bool // true for rangefunc list
sp uintptr // sp at time of defer
- pc uintptr // pc at time of defer
fn func() // can be nil for open-coded defers
link *_defer // next defer on G; can point to either heap or stack!
Change information
Files:
- M src/cmd/compile/internal/ssagen/ssa.go
- M src/runtime/heapdump.go
- M src/runtime/panic.go
- M src/runtime/runtime2.go
Change size: S
Delta: 4 files changed, 9 insertions(+), 21 deletions(-)
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Youlin Feng (Gerrit)
Oct 30, 2025, 11:10:18 PM (11 days ago) Oct 30
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Youlin Feng voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Keith Randall (Gerrit)
Oct 31, 2025, 6:04:58 PM (10 days ago) Oct 31
to Youlin Feng, goph...@pubsubhelper.golang.org, Keith Randall, Derek Parker, Go LUCI, David Chase, golang-co...@googlegroups.com
Attention needed from David Chase and Youlin Feng
Keith Randall voted and added 1 comment![]( "Open in Gerrit")
Votes added by Keith Randall
| Code-Review | +2 |
1 comment
Patchset-level comments
File-level comment, Patchset 1 (Latest):
Keith Randall . resolved
Derek, does Delve know the layout of defer records?
Related details
Attention is currently required from:
- David Chase
- Youlin Feng
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Keith Randall (Gerrit)
Oct 31, 2025, 6:05:26 PM (10 days ago) Oct 31
to Youlin Feng, goph...@pubsubhelper.golang.org, Keith Randall, Derek Parker, Go LUCI, David Chase, golang-co...@googlegroups.com
Attention needed from David Chase and Youlin Feng
Keith Randall voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- David Chase
- Youlin Feng
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Knyszek (Gerrit)
Nov 3, 2025, 10:12:35 AM (8 days ago) Nov 3
to Youlin Feng, goph...@pubsubhelper.golang.org, Keith Randall, Keith Randall, Derek Parker, Go LUCI, David Chase, golang-co...@googlegroups.com
Attention needed from David Chase and Youlin Feng
Michael Knyszek voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- David Chase
- Youlin Feng
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Derek Parker (Gerrit)
Nov 3, 2025, 11:43:33 AM (7 days ago) Nov 3
to Youlin Feng, goph...@pubsubhelper.golang.org, Keith Randall, Keith Randall, Go LUCI, David Chase, golang-co...@googlegroups.com
Attention needed from David Chase and Youlin Feng
Derek Parker added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Keith Randall . resolved
Derek, does Delve know the layout of defer records?
Derek Parker
Yes, we'll have to update some code on our end when this lands. Thanks for the heads up.
Keith Randall (Gerrit)
Nov 3, 2025, 12:26:41 PM (7 days ago) Nov 3
to Keith Randall, Youlin Feng, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Michael Knyszek, Keith Randall, Derek Parker, Go LUCI, David Chase, golang-co...@googlegroups.com
Keith Randall submitted the change![]( "Open in Gerrit")
Change information
Commit message:
runtime: remove the pc field of _defer struct
Since we always can get the address of `CALL runtime.deferreturn(SB)`
from the unwinder, so it is not necessary to record the caller's pc
in the _defer struct. For the stack allocated _defer, this CL makes
the frame smaller.
Change-Id: I0fd347e4bc07cf8a9b954816323df30fc52552b6
Reviewed-on: https://go-review.googlesource.com/c/go/+/716720
Reviewed-by: Keith Randall <k...@golang.org>
LUCI-TryBot-Result: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <k...@google.com>
Reviewed-by: Michael Knyszek <mkny...@google.com>
Files:
- M src/cmd/compile/internal/ssagen/ssa.go
- M src/runtime/heapdump.go
- M src/runtime/panic.go
- M src/runtime/runtime2.go
Change size: S
Delta: 4 files changed, 9 insertions(+), 21 deletions(-)
Branch: refs/heads/master
Submit Requirements:
Code-Review: +2 by Keith Randall, +1 by Michael Knyszek, +1 by Keith Randall
TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Keith Randall (Gerrit)
Nov 5, 2025, 9:05:48 PM (5 days ago) Nov 5
to Keith Randall, Youlin Feng, goph...@pubsubhelper.golang.org, Keith Randall, Derek Parker, Go LUCI, David Chase, golang-co...@googlegroups.com
Keith Randall has created a revert of this change![]( "Open in Gerrit")
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Youlin Feng (Gerrit)
Nov 6, 2025, 1:05:59 AM (5 days ago) Nov 6
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Youlin Feng has uploaded the change for review![]( "Open in Gerrit")
Commit message
runtime: remove the pc field of _defer struct
Since we always can get the address of `CALL runtime.deferreturn(SB)`
from the unwinder, so it is not necessary to record the caller's pc
in the _defer struct. For the stack allocated _defer, this CL makes
the frame smaller.
This is a resubmission of CL 716720, designed to fix the issue that
caused the revert.
Change-Id: Ie89548d1d9963ef4757c7ae087864b08b1635b55
Change diff
index 175452f..36d31bf 100644@@ -1018,6 +1012,13 @@
// it's non-zero.
if u.frame.sp == limit {
+ // This delays the "no deferreturn" fatal error until we
+ // actually perform the recovery operation.
+ retpc := uintptr(u.frame.fn.deferreturn)
+ if retpc != 0 {
+ retpc += u.frame.fn.entry()
+ }
+ p.retpc = retpc
break // found a frame with linked defers
}
@@ -1273,14 +1274,11 @@
pc, sp, fp := p.retpc, uintptr(p.sp), uintptr(p.fp)
p0, saveOpenDeferState := p, p.deferBitsPtr != nil && *p.deferBitsPtr != 0
- // The linker records the f-relative address of a call to deferreturn in f's funcInfo.
- // Assuming a "normal" call to recover() inside one of f's deferred functions
- // invoked for a panic, that is the desired PC for exiting f.
- f := findfunc(pc)
- if f.deferreturn == 0 {
+ // The "no deferreturn" error is delayed until we actually perform the
+ // recovery operation, we won't throw when unwinding.
+ if pc == 0 {
throw("no deferreturn")
}
- gotoPc := f.entry() + uintptr(f.deferreturn)
// Unwind the panic stack.
for ; p != nil && uintptr(p.startSP) < sp; p = p.link {
@@ -1304,7 +1302,7 @@
// With how subtle defer handling is, this might not actually be
// worthwhile though.
if p.goexit {
- gotoPc, sp = p.startPC, uintptr(p.startSP)
+ pc, sp = p.startPC, uintptr(p.startSP)
saveOpenDeferState = false // goexit is unwinding the stack anyway
break
}
@@ -1367,7 +1365,7 @@
// branch directly to the deferreturn
gp.sched.sp = sp
- gp.sched.pc = gotoPc
+ gp.sched.pc = pc
gp.sched.lr = 0
// Restore the bp on platforms that support frame pointers.
// N.B. It's fine to not set anything for platforms that don't
diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go
index b346337..3672b19 100644
--- a/src/runtime/runtime2.go
+++ b/src/runtime/runtime2.go
@@ -1090,7 +1090,6 @@
heap bool
rangefunc bool // true for rangefunc list
sp uintptr // sp at time of defer
- pc uintptr // pc at time of defer
fn func() // can be nil for open-coded defers
link *_defer // next defer on G; can point to either heap or stack!
Change information
Files:
- M src/cmd/compile/internal/ssagen/ssa.go
- M src/runtime/heapdump.go
- M src/runtime/panic.go
- M src/runtime/runtime2.go
Change size: S
Delta: 4 files changed, 13 insertions(+), 18 deletions(-)
Related details
Attention set is empty
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Youlin Feng (Gerrit)
Nov 6, 2025, 1:13:41 AM (5 days ago) Nov 6
to goph...@pubsubhelper.golang.org, Keith Randall, golang-co...@googlegroups.com
Attention needed from Keith Randall
Youlin Feng added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Youlin Feng . unresolved
Hi Keith, cound you help me test this CL? I'd like to know if the issue that caused the revert has been fixed in this CL. @k...@golang.org
Related details
Attention is currently required from:
- Keith Randall
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Youlin Feng (Gerrit)
Nov 6, 2025, 10:06:01 PM (4 days ago) Nov 6
to goph...@pubsubhelper.golang.org, Keith Randall, golang-co...@googlegroups.com
Attention needed from Keith Randall
Youlin Feng voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Attention is currently required from:
- Keith Randall
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Youlin Feng (Gerrit)
8:45 AM (14 hours ago) 8:45 AM
to goph...@pubsubhelper.golang.org, Nicolas Hillegeer, Go LUCI, Keith Randall, golang-co...@googlegroups.com
Attention needed from Keith Randall
Youlin Feng added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Youlin Feng . resolved
PTAL.
Related details
Attention is currently required from:
- Keith Randall
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Keith Randall (Gerrit)
6:44 PM (4 hours ago) 6:44 PM
to Youlin Feng, goph...@pubsubhelper.golang.org, Nicolas Hillegeer, Go LUCI, Keith Randall, golang-co...@googlegroups.com
Attention needed from Youlin Feng
Keith Randall added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Youlin Feng . unresolved
Keith RandallPTAL.
Unfortunately, this CL is still hitting the "no deferreturn" throw in runtime.recover.
(on arm64, amd64 is ok)
Related details
Attention is currently required from:
- Youlin Feng
Submit Requirements:
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages