Skip to content

Commit 2aaa388

Browse files
dr2chasemknyszek
authored andcommitted
[release-branch.go1.23] cmd/compile, runtime: use deferreturn as target PC for recover from deferrangefunc
The existing code for recover from deferrangefunc was broken in several ways. 1. the code following a deferrangefunc call did not check the return value for an out-of-band value indicating "return now" (i.e., recover was called) 2. the returned value was delivered using a bespoke ABI that happened to match on register-ABI platforms, but not on older stack-based ABI. 3. the returned value was the wrong width (1 word versus 2) and type/value(integer 1, not a pointer to anything) for deferrangefunc's any-typed return value (in practice, the OOB value check could catch this, but still, it's sketchy). This -- using the deferreturn lookup method already in place for open-coded defers -- turned out to be a much-less-ugly way of obtaining the desired transfer of control for recover(). TODO: we also could do this for regular defer, and delete some code. Fixes #71839 Change-Id: If7d7ea789ad4320821aab3b443759a7d71647ff0 Reviewed-on: https://go-review.googlesource.com/c/go/+/650476 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Cherry Mui <[email protected]> Reviewed-by: Cuong Manh Le <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/go/+/651496
1 parent 22fdd35 commit 2aaa388

File tree

5 files changed

+134
-7
lines changed

5 files changed

+134
-7
lines changed

src/cmd/compile/internal/ssa/func.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,12 @@ type Func struct {
4141
ABISelf *abi.ABIConfig // ABI for function being compiled
4242
ABIDefault *abi.ABIConfig // ABI for rtcall and other no-parsed-signature/pragma functions.
4343

44-
scheduled bool // Values in Blocks are in final order
45-
laidout bool // Blocks are ordered
46-
NoSplit bool // true if function is marked as nosplit. Used by schedule check pass.
47-
dumpFileSeq uint8 // the sequence numbers of dump file. (%s_%02d__%s.dump", funcname, dumpFileSeq, phaseName)
48-
IsPgoHot bool
44+
scheduled bool // Values in Blocks are in final order
45+
laidout bool // Blocks are ordered
46+
NoSplit bool // true if function is marked as nosplit. Used by schedule check pass.
47+
dumpFileSeq uint8 // the sequence numbers of dump file. (%s_%02d__%s.dump", funcname, dumpFileSeq, phaseName)
48+
IsPgoHot bool
49+
HasDeferRangeFunc bool // if true, needs a deferreturn so deferrangefunc can use it for recover() return PC
4950

5051
// when register allocation is done, maps value ids to locations
5152
RegAlloc []Location

src/cmd/compile/internal/ssagen/ssa.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5390,6 +5390,9 @@ func (s *state) call(n *ir.CallExpr, k callKind, returnResultAddr bool, deferExt
53905390
callABI = s.f.ABI1
53915391
}
53925392
}
5393+
if fn := n.Fun.Sym().Name; n.Fun.Sym().Pkg == ir.Pkgs.Runtime && fn == "deferrangefunc" {
5394+
s.f.HasDeferRangeFunc = true
5395+
}
53935396
break
53945397
}
53955398
closure = s.expr(fn)
@@ -7513,10 +7516,13 @@ func genssa(f *ssa.Func, pp *objw.Progs) {
75137516
// nop (which will never execute) after the call.
75147517
Arch.Ginsnop(s.pp)
75157518
}
7516-
if openDeferInfo != nil {
7519+
if openDeferInfo != nil || f.HasDeferRangeFunc {
75177520
// When doing open-coded defers, generate a disconnected call to
75187521
// deferreturn and a return. This will be used to during panic
75197522
// recovery to unwind the stack and return back to the runtime.
7523+
//
7524+
// deferrangefunc needs to be sure that at least one of these exists;
7525+
// if all returns are dead-code eliminated, there might not be.
75207526
s.pp.NextLive = s.livenessMap.DeferReturn
75217527
p := s.pp.Prog(obj.ACALL)
75227528
p.To.Type = obj.TYPE_MEM

src/runtime/panic.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -391,10 +391,16 @@ func deferrangefunc() any {
391391
throw("defer on system stack")
392392
}
393393

394+
fn := findfunc(getcallerpc())
395+
if fn.deferreturn == 0 {
396+
throw("no deferreturn")
397+
}
398+
394399
d := newdefer()
395400
d.link = gp._defer
396401
gp._defer = d
397-
d.pc = getcallerpc()
402+
403+
d.pc = fn.entry() + uintptr(fn.deferreturn)
398404
// We must not be preempted between calling getcallersp and
399405
// storing it to d.sp because getcallersp's result is a
400406
// uintptr stack pointer.
@@ -1215,6 +1221,8 @@ func recovery(gp *g) {
12151221
// only gets us to the caller's fp.
12161222
gp.sched.bp = sp - goarch.PtrSize
12171223
}
1224+
// The value in ret is delivered IN A REGISTER, even if there is a
1225+
// stack ABI.
12181226
gp.sched.ret = 1
12191227
gogo(&gp.sched)
12201228
}

test/fixedbugs/issue71675.go

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
// run
2+
// Copyright 2025 The Go Authors. All rights reserved.
3+
// Use of this source code is governed by a BSD-style
4+
// license that can be found in the LICENSE file.
5+
package main
6+
7+
//go:noinline
8+
func i() {
9+
for range yieldInts {
10+
defer func() {
11+
println("I")
12+
recover()
13+
}()
14+
}
15+
// This panic causes dead code elimination of the return block.
16+
// The compiler should nonetheless emit a deferreturn.
17+
panic("i panic")
18+
}
19+
20+
//go:noinline
21+
func h() {
22+
defer func() {
23+
println("H first")
24+
}()
25+
for range yieldInts {
26+
defer func() {
27+
println("H second")
28+
}()
29+
}
30+
defer func() {
31+
println("H third")
32+
}()
33+
for range yieldIntsPanic {
34+
defer func() {
35+
println("h recover:called")
36+
recover()
37+
}()
38+
}
39+
}
40+
41+
//go:noinline
42+
func yieldInts(yield func(int) bool) {
43+
if !yield(0) {
44+
return
45+
}
46+
}
47+
48+
//go:noinline
49+
func g() {
50+
defer func() {
51+
println("G first")
52+
}()
53+
for range yieldIntsPanic {
54+
defer func() {
55+
println("g recover:called")
56+
recover()
57+
}()
58+
}
59+
}
60+
61+
//go:noinline
62+
func yieldIntsPanic(yield func(int) bool) {
63+
if !yield(0) {
64+
return
65+
}
66+
panic("yield stop")
67+
}
68+
69+
//go:noinline
70+
func next(i int) int {
71+
if i == 0 {
72+
panic("next stop")
73+
}
74+
return i + 1
75+
}
76+
77+
//go:noinline
78+
func f() {
79+
defer func() {
80+
println("F first")
81+
}()
82+
for i := 0; i < 1; i = next(i) {
83+
defer func() {
84+
println("f recover:called")
85+
recover()
86+
}()
87+
}
88+
}
89+
func main() {
90+
f()
91+
println("f returned")
92+
g()
93+
println("g returned")
94+
h()
95+
println("h returned")
96+
i()
97+
println("i returned")
98+
99+
}

test/fixedbugs/issue71675.out

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
f recover:called
2+
F first
3+
f returned
4+
g recover:called
5+
G first
6+
g returned
7+
h recover:called
8+
H third
9+
H second
10+
H first
11+
h returned
12+
I
13+
i returned

0 commit comments

Comments
 (0)