[go] Revert "runtime: remove the pc field of _defer struct"
4 views
Skip to first unread message
Keith Randall (Gerrit)
Nov 5, 2025, 9:05:47 PM (5 days ago) Nov 5
to David Chase, Youlin Feng, Keith Randall, Michael Knyszek, Go LUCI, goph...@pubsubhelper.golang.org, Derek Parker, Keith Randall, golang-co...@googlegroups.com
Attention needed from David Chase, Keith Randall, Michael Knyszek and Youlin Feng
Keith Randall has uploaded the change for review![]( "Open in Gerrit")
Keith Randall would like David Chase, Youlin Feng, Keith Randall, Michael Knyszek and Go LUCI to review this change.
Commit message
Revert "runtime: remove the pc field of _defer struct"
This reverts commit 361d51a6b58bccaab0559e06737c918018a7a5fa.
Reason for revert: Breaks some tests inside Google (on arm64?)
Change-Id: Iaea45fdcf9b4f9d36553687ca7f479750fe559da
Change diff
diff --git a/src/cmd/compile/internal/ssagen/ssa.go b/src/cmd/compile/internal/ssagen/ssa.go
index db2ffb5..ae7d575 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 = 3
+const deferStructFnField = 4
var deferType *types.Type
@@ -7817,6 +7817,7 @@
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 3a8c374..3671c65 100644
--- a/src/runtime/heapdump.go
+++ b/src/runtime/heapdump.go
@@ -382,6 +382,7 @@
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 aee1cd0..175452f 100644
--- a/src/runtime/panic.go
+++ b/src/runtime/panic.go
@@ -354,6 +354,7 @@
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.
@@ -457,6 +458,7 @@
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.
@@ -516,6 +518,7 @@
}
for d1 := d; ; d1 = d1.link {
d1.sp = d0.sp
+ d1.pc = d0.pc
if d1.link == nil {
d1.link = tail
break
@@ -544,6 +547,7 @@
d.heap = false
d.rangefunc = false
d.sp = sys.GetCallerSP()
+ d.pc = sys.GetCallerPC()
// The lines below implement:
// d.panic = nil
// d.fd = nil
@@ -973,6 +977,8 @@
fn := d.fn
+ p.retpc = d.pc
+
// Unlink and free.
popDefer(gp)
@@ -1012,12 +1018,6 @@
// 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,6 +1273,15 @@
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.
@@ -1295,7 +1304,7 @@
// With how subtle defer handling is, this might not actually be
// worthwhile though.
if p.goexit {
- pc, sp = p.startPC, uintptr(p.startSP)
+ gotoPc, sp = p.startPC, uintptr(p.startSP)
saveOpenDeferState = false // goexit is unwinding the stack anyway
break
}
@@ -1358,7 +1367,7 @@
// branch directly to the deferreturn
gp.sched.sp = sp
- gp.sched.pc = pc
+ gp.sched.pc = gotoPc
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 3672b19..b346337 100644
--- a/src/runtime/runtime2.go
+++ b/src/runtime/runtime2.go
@@ -1090,6 +1090,7 @@
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, 21 insertions(+), 9 deletions(-)
Related details
Attention is currently required from:
- David Chase
- Keith Randall
- Michael Knyszek
- 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)
Nov 5, 2025, 9:08:03 PM (5 days ago) Nov 5
to Keith Randall, goph...@pubsubhelper.golang.org, David Chase, Youlin Feng, Michael Knyszek, Go LUCI, Derek Parker, golang-co...@googlegroups.com
Attention needed from David Chase, Keith Randall, Michael Knyszek and Youlin Feng
Keith Randall added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 1 (Latest):
Keith Randall . resolved
Failures look like:
```
=== RUN TestNonSwissMapType
fatal error: no deferreturn
runtime stack:
runtime.throw({0x2b9be8?, 0x118d8?})
third_party/go/builds/gc_tip/go/src/runtime/panic.go:1227 +0x38 fp=0xffff637fd840 sp=0xffff637fd810 pc=0xc2928
runtime.(*_panic).nextFrame.func1()
third_party/go/builds/gc_tip/go/src/runtime/panic.go:1017 +0x160 fp=0xffff637fd900 sp=0xffff637fd840 pc=0x88be0
runtime.systemstack(0x0)
third_party/go/builds/gc_tip/go/src/runtime/asm_arm64.s:366 +0x68 fp=0xffff637fd910 sp=0xffff637fd900 pc=0xc7e08
goroutine 11 gp=0xc0001841c0 m=4 mp=0xc000051808 [running]:
runtime.systemstack_switch()
third_party/go/builds/gc_tip/go/src/runtime/asm_arm64.s:321 +0x8 fp=0xc000196d00 sp=0xc000196cf0 pc=0xc7d88
runtime.(*_panic).nextFrame(0x0?)
third_party/go/builds/gc_tip/go/src/runtime/panic.go:995 +0x58 fp=0xc000196d40 sp=0xc000196d00 pc=0x88a58
runtime.(*_panic).start(0x466360?, 0x26cd80?, 0xc000196d01?)
third_party/go/builds/gc_tip/go/src/runtime/panic.go:918 +0x12c fp=0xc000196d60 sp=0xc000196d40 pc=0x8884c
panic({0x276520?, 0x47dc30?})
third_party/go/builds/gc_tip/go/src/runtime/panic.go:846 +0x11c fp=0xc000196e10 sp=0xc000196d60 pc=0xc25ec
runtime.panicmem(...)
third_party/go/builds/gc_tip/go/src/runtime/panic.go:336
runtime.sigpanic()
third_party/go/builds/gc_tip/go/src/runtime/signal_unix.go:925 +0x2e8 fp=0xc000196e70 sp=0xc000196e10 pc=0xc4978
internal/abi.(*MapType).NeedKeyUpdate(0xc000196e98?)
third_party/go/builds/gc_tip/go/src/internal/abi/map.go:54 fp=0xc000196e80 sp=0xc000196e80 pc=0x3ef60
```
Cherry suggests that this may be the issue:
```
I think it changes semantics: in the old code the pc field is the PC of the defer statement, whereas in the new code retpc is the PC of deferreturn, which is different.
```
Related details
Attention is currently required from:
- David Chase
- Keith Randall
- Michael Knyszek
- 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. |
Youlin Feng (Gerrit)
Nov 5, 2025, 9:08:11 PM (5 days ago) Nov 5
to Keith Randall, goph...@pubsubhelper.golang.org, David Chase, Keith Randall, Michael Knyszek, Go LUCI, Derek Parker, golang-co...@googlegroups.com
Attention needed from David Chase, Keith Randall and Michael Knyszek
Youlin Feng voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- David Chase
- Keith Randall
- Michael Knyszek
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)
Nov 5, 2025, 9:13:30 PM (5 days ago) Nov 5
to Keith Randall, goph...@pubsubhelper.golang.org, Youlin Feng, David Chase, Keith Randall, Michael Knyszek, Go LUCI, Derek Parker, golang-co...@googlegroups.com
Attention needed from David Chase and Michael Knyszek
Keith Randall voted![]( "Open in Gerrit")
Attention is currently required from:
- David Chase
- Michael Knyszek
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)
Nov 5, 2025, 9:14:18 PM (5 days ago) Nov 5
to Keith Randall, goph...@pubsubhelper.golang.org, Youlin Feng, David Chase, Michael Knyszek, Go LUCI, Derek Parker, golang-co...@googlegroups.com
Attention needed from David Chase and Michael Knyszek
Keith Randall voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Youlin Feng (Gerrit)
Nov 5, 2025, 9:17:00 PM (5 days ago) Nov 5
to Keith Randall, goph...@pubsubhelper.golang.org, Keith Randall, David Chase, Michael Knyszek, Go LUCI, Derek Parker, golang-co...@googlegroups.com
Attention needed from David Chase, Keith Randall and Michael Knyszek
Youlin Feng added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Youlin Feng . resolved
How can I reproduce this problem? I want to try to fix it and resubmit this CL.
Related details
Attention is currently required from:
- David Chase
- Keith Randall
- Michael Knyszek
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. |
Cuong Manh Le (Gerrit)
Nov 5, 2025, 9:32:25 PM (5 days ago) Nov 5
to Keith Randall, goph...@pubsubhelper.golang.org, Keith Randall, Youlin Feng, David Chase, Michael Knyszek, Go LUCI, Derek Parker, golang-co...@googlegroups.com
Attention needed from David Chase, Keith Randall and Michael Knyszek
Cuong Manh Le voted Code-Review+2![]( "Open in Gerrit")
| Code-Review | +2 |
Related details
Attention is currently required from:
- David Chase
- Keith Randall
- Michael Knyszek
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)
Nov 6, 2025, 12:55:22 AM (5 days ago) Nov 6
to Keith Randall, goph...@pubsubhelper.golang.org, Go LUCI, Cuong Manh Le, Keith Randall, Youlin Feng, David Chase, Michael Knyszek, Derek Parker, golang-co...@googlegroups.com
Attention needed from David Chase, Michael Knyszek and Youlin Feng
Keith Randall added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Youlin Feng . resolved
How can I reproduce this problem? I want to try to fix it and resubmit this CL.
Keith Randall
Unfortunately there isn't a easily extracted reproducer.
The only tidbit of information is that it is probably arm64 specific (or at least, doesn't happen on amd64).
Related details
Attention is currently required from:
- David Chase
- Michael Knyszek
- 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. |
David Chase (Gerrit)
Nov 6, 2025, 7:59:36 AM (5 days ago) Nov 6
to Keith Randall, goph...@pubsubhelper.golang.org, Go LUCI, Cuong Manh Le, Keith Randall, Youlin Feng, Michael Knyszek, Derek Parker, golang-co...@googlegroups.com
Attention needed from Keith Randall, Michael Knyszek and Youlin Feng
David Chase voted Code-Review+2![]( "Open in Gerrit")
Attention is currently required from:
- Keith Randall
- Michael Knyszek
- 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. |
David Chase (Gerrit)
Nov 6, 2025, 8:00:32 AM (5 days ago) Nov 6
to Keith Randall, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Go LUCI, Cuong Manh Le, Keith Randall, Youlin Feng, Michael Knyszek, Derek Parker, golang-co...@googlegroups.com
David Chase submitted the change![]( "Open in Gerrit")
Change information
Commit message:
Revert "runtime: remove the pc field of _defer struct"
This reverts commit 361d51a6b58bccaab0559e06737c918018a7a5fa.
Reason for revert: Breaks some tests inside Google (on arm64?)
Change-Id: Iaea45fdcf9b4f9d36553687ca7f479750fe559da
Reviewed-on: https://go-review.googlesource.com/c/go/+/718066
Auto-Submit: Keith Randall <k...@golang.org>
Reviewed-by: Cuong Manh Le <cuong.m...@gmail.com>
Reviewed-by: Youlin Feng <fengy...@live.com>
Reviewed-by: Keith Randall <k...@google.com>
LUCI-TryBot-Result: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drc...@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, 21 insertions(+), 9 deletions(-)
Branch: refs/heads/master
Submit Requirements:
Code-Review: +1 by Youlin Feng, +2 by David Chase, +1 by Keith Randall, +2 by Cuong Manh Le
TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nicolas Hillegeer (Gerrit)
Nov 6, 2025, 8:45:16 AM (5 days ago) Nov 6
to Keith Randall, David Chase, goph...@pubsubhelper.golang.org, Go LUCI, Cuong Manh Le, Keith Randall, Youlin Feng, Derek Parker, golang-co...@googlegroups.com
Attention needed from Keith Randall
Nicolas Hillegeer added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Keith RandallHow can I reproduce this problem? I want to try to fix it and resubmit this CL.
Unfortunately there isn't a easily extracted reproducer.
The only tidbit of information is that it is probably arm64 specific (or at least, doesn't happen on amd64).
Nicolas Hillegeer
Here's a mildly redacted stack trace:
```
fatal error: no deferreturn
runtime stack:
runtime.throw({0x2b9be8?, 0x118d8?})
/home/aktau/go/src/runtime/panic.go:1227 +0x38 fp=0xffff637fd840 sp=0xffff637fd810 pc=0xc2928
runtime.(*_panic).nextFrame.func1()
/home/aktau/go/src/runtime/panic.go:1017 +0x160 fp=0xffff637fd900 sp=0xffff637fd840 pc=0x88be0
runtime.systemstack(0x0)
/home/aktau/go/src/runtime/asm_arm64.s:366 +0x68 fp=0xffff637fd910 sp=0xffff637fd900 pc=0xc7e08
goroutine 11 gp=0xc0001841c0 m=4 mp=0xc000051808 [running]:
runtime.systemstack_switch()
/home/aktau/go/src/runtime/asm_arm64.s:321 +0x8 fp=0xc000196d00 sp=0xc000196cf0 pc=0xc7d88
runtime.(*_panic).nextFrame(0x0?)
/home/aktau/go/src/runtime/panic.go:995 +0x58 fp=0xc000196d40 sp=0xc000196d00 pc=0x88a58
runtime.(*_panic).start(0x466360?, 0x26cd80?, 0xc000196d01?)
/home/aktau/go/src/runtime/panic.go:918 +0x12c fp=0xc000196d60 sp=0xc000196d40 pc=0x8884c
panic({0x276520?, 0x47dc30?})
/home/aktau/go/src/runtime/panic.go:846 +0x11c fp=0xc000196e10 sp=0xc000196d60 pc=0xc25ec
runtime.panicmem(...)
/home/aktau/go/src/runtime/panic.go:336
runtime.sigpanic()
/home/aktau/go/src/runtime/signal_unix.go:925 +0x2e8 fp=0xc000196e70 sp=0xc000196e10 pc=0xc4978
internal/abi.(*MapType).NeedKeyUpdate(0xc000196e98?)
/home/aktau/go/src/internal/abi/map.go:54 fp=0xc000196e80 sp=0xc000196e80 pc=0x3ef60
/home/aktua/expiriments/tags_test.TestNonSwissMapType(0xc00018a6c8?)
third_party/go/builds/tags/nonswissmaptype_test.go:18 +0x3c fp=0xc000196ed0 sp=0xc000196e80 pc=0x1d0e0c
testing.tRunner(0xc00018a6c8, 0x2c8d58)
/home/aktau/go/src/testing/testing.go:2029 +0x168 fp=0xc000196fa0 sp=0xc000196ed0 pc=0x16b358
testing.(*T).Run.gowrap1()
/home/aktau/go/src/testing/testing.go:2094 +0x38 fp=0xc000196fd0 sp=0xc000196fa0 pc=0x16c988
runtime.goexit({})
/home/aktau/go/src/runtime/asm_arm64.s:1389 +0x4 fp=0xc000196fd0 sp=0xc000196fd0 pc=0xc9f44
created by testing.(*T).Run in goroutine 1
/home/aktau/go/src/testing/testing.go:2094 +0x7c0
goroutine 1 gp=0xc0000021c0 m=nil [chan receive]:
runtime.gopark(0x3001860a0?, 0xffffb04c9170?, 0x8?, 0x1?, 0x18?)
/home/aktau/go/src/runtime/proc.go:461 +0xc0 fp=0xc0000f5630 sp=0xc0000f5610 pc=0xc2a50
runtime.chanrecv(0xc000076200, 0xc0000f573e, 0x1)
/home/aktau/go/src/runtime/chan.go:667 +0x410 fp=0xc0000f56c0 sp=0xc0000f5630 pc=0x54d90
runtime.chanrecv1(0x489ea0?, 0x269fe0?)
/home/aktau/go/src/runtime/chan.go:509 +0x14 fp=0xc0000f56f0 sp=0xc0000f56c0 pc=0x54944
testing.(*T).Run(0xc00018a008, {0x2bb51b, 0x13}, 0x2c8d58)
/home/aktau/go/src/testing/testing.go:2102 +0x7dc fp=0xc0000f5840 sp=0xc0000f56f0 pc=0x16c7ac
testing.runTests.func1(0xc00018a008)
/home/aktau/go/src/testing/testing.go:2580 +0x7c fp=0xc0000f5890 sp=0xc0000f5840 pc=0x16fd9c
testing.tRunner(0xc00018a008, 0xc0000f5aa8)
/home/aktau/go/src/testing/testing.go:2029 +0x168 fp=0xc0000f5960 sp=0xc0000f5890 pc=0x16b358
testing.runTests({0x0, 0x0}, {0x2c3f2a, 0x2c}, 0xc000010018, {0x4842e0, 0x7, 0x7}, {0xc0000f5b28?, 0xd5494?, ...})
/home/aktau/go/src/testing/testing.go:2578 +0x7a4 fp=0xc0000f5ad0 sp=0xc0000f5960 pc=0x16fc24
testing.(*M).Run(0xc000186000)
/home/aktau/go/src/testing/testing.go:2438 +0xb54 fp=0xc0000f5e40 sp=0xc0000f5ad0 pc=0x16db74
main.main()
gen_tags_test_main.go:61 +0x240 fp=0xc0000f5f30 sp=0xc0000f5e40 pc=0x249850
runtime.main()
/home/aktau/go/src/runtime/proc.go:289 +0x2b4 fp=0xc0000f5fd0 sp=0xc0000f5f30 pc=0x8c734
runtime.goexit({})
/home/aktau/go/src/runtime/asm_arm64.s:1389 +0x4 fp=0xc0000f5fd0 sp=0xc0000f5fd0 pc=0xc9f44
```
The error is always `fatal error: no deferreturn`. It doesn't happen for all ARM64 tests.
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:00:47 PM (4 days ago) Nov 6
to Keith Randall, David Chase, goph...@pubsubhelper.golang.org, Nicolas Hillegeer, Go LUCI, Cuong Manh Le, Keith Randall, Derek Parker, golang-co...@googlegroups.com
Attention needed from Keith Randall
Youlin Feng added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Youlin Feng
Thanks. @ak...@google.com If it's convenient for you, could you please test the [CL 718360](https://go.dev/cl/718360)?
Reply all
Reply to author
Forward
0 new messages