Skip to content

Commit b2482e4

Browse files
cherrymuicherrymui
committed
cmd/internal/obj: mark split-stack prologue nonpreemptible
When there are both a synchronous preemption request (by clobbering the stack guard) and an asynchronous one (by signal), the running goroutine may observe the synchronous request first in stack bounds check, and go to the path of calling morestack. If the preemption signal arrives at this point before the call to morestack, the goroutine will be asynchronously preempted, entering the scheduler. When it is resumed, the scheduler clears the preemption request, unclobbers the stack guard. But the resumed goroutine will still call morestack, as it is already on its way. morestack will, as there is no preemption request, double the stack unnecessarily. If this happens multiple times, the stack may grow too big, although only a small amount is actually used. To fix this, we mark the stack bounds check and the call to morestack async-nonpreemptible, starting after the memory instruction (mostly a load, on x86 CMP with memory). Not done for Wasm as it does not support async preemption. Fixes #35470. Change-Id: Ibd7f3d935a3649b80f47539116ec9b9556680cf2 Reviewed-on: https://go-review.googlesource.com/c/go/+/207350 Reviewed-by: David Chase <[email protected]>
1 parent c3f1492 commit b2482e4

File tree

7 files changed

+94
-40
lines changed

7 files changed

+94
-40
lines changed

‎src/cmd/internal/obj/arm/obj5.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -674,6 +674,12 @@ func (c *ctxt5) stacksplit(p *obj.Prog, framesize int32) *obj.Prog {
674674
p.To.Type = obj.TYPE_REG
675675
p.To.Reg = REG_R1
676676

677+
// Mark the stack bound check and morestack call async nonpreemptible.
678+
// If we get preempted here, when resumed the preemption request is
679+
// cleared, but we'll still call morestack, which will double the stack
680+
// unnecessarily. See issue #35470.
681+
p = c.ctxt.StartUnsafePoint(p, c.newprog)
682+
677683
if framesize <= objabi.StackSmall {
678684
// small stack: SP < stackguard
679685
// CMP stackguard, SP
@@ -757,6 +763,8 @@ func (c *ctxt5) stacksplit(p *obj.Prog, framesize int32) *obj.Prog {
757763
bls.As = ABLS
758764
bls.To.Type = obj.TYPE_BRANCH
759765

766+
end := c.ctxt.EndUnsafePoint(bls, c.newprog, -1)
767+
760768
var last *obj.Prog
761769
for last = c.cursym.Func.Text; last.Link != nil; last = last.Link {
762770
}
@@ -768,7 +776,8 @@ func (c *ctxt5) stacksplit(p *obj.Prog, framesize int32) *obj.Prog {
768776
spfix.As = obj.ANOP
769777
spfix.Spadj = -framesize
770778

771-
pcdata := c.ctxt.EmitEntryLiveness(c.cursym, spfix, c.newprog)
779+
pcdata := c.ctxt.EmitEntryStackMap(c.cursym, spfix, c.newprog)
780+
pcdata = c.ctxt.StartUnsafePoint(pcdata, c.newprog)
772781

773782
// MOVW LR, R3
774783
movw := obj.Appendp(pcdata, c.newprog)
@@ -793,14 +802,16 @@ func (c *ctxt5) stacksplit(p *obj.Prog, framesize int32) *obj.Prog {
793802
}
794803
call.To.Sym = c.ctxt.Lookup(morestack)
795804

805+
pcdata = c.ctxt.EndUnsafePoint(call, c.newprog, -1)
806+
796807
// B start
797-
b := obj.Appendp(call, c.newprog)
808+
b := obj.Appendp(pcdata, c.newprog)
798809
b.As = obj.AJMP
799810
b.To.Type = obj.TYPE_BRANCH
800811
b.Pcond = c.cursym.Func.Text.Link
801812
b.Spadj = +framesize
802813

803-
return bls
814+
return end
804815
}
805816

806817
var unaryDst = map[obj.As]bool{

‎src/cmd/internal/obj/arm64/obj7.go

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,12 @@ func (c *ctxt7) stacksplit(p *obj.Prog, framesize int32) *obj.Prog {
6262
p.To.Type = obj.TYPE_REG
6363
p.To.Reg = REG_R1
6464

65+
// Mark the stack bound check and morestack call async nonpreemptible.
66+
// If we get preempted here, when resumed the preemption request is
67+
// cleared, but we'll still call morestack, which will double the stack
68+
// unnecessarily. See issue #35470.
69+
p = c.ctxt.StartUnsafePoint(p, c.newprog)
70+
6571
q := (*obj.Prog)(nil)
6672
if framesize <= objabi.StackSmall {
6773
// small stack: SP < stackguard
@@ -156,6 +162,8 @@ func (c *ctxt7) stacksplit(p *obj.Prog, framesize int32) *obj.Prog {
156162
bls.As = ABLS
157163
bls.To.Type = obj.TYPE_BRANCH
158164

165+
end := c.ctxt.EndUnsafePoint(bls, c.newprog, -1)
166+
159167
var last *obj.Prog
160168
for last = c.cursym.Func.Text; last.Link != nil; last = last.Link {
161169
}
@@ -167,7 +175,8 @@ func (c *ctxt7) stacksplit(p *obj.Prog, framesize int32) *obj.Prog {
167175
spfix.As = obj.ANOP
168176
spfix.Spadj = -framesize
169177

170-
pcdata := c.ctxt.EmitEntryLiveness(c.cursym, spfix, c.newprog)
178+
pcdata := c.ctxt.EmitEntryStackMap(c.cursym, spfix, c.newprog)
179+
pcdata = c.ctxt.StartUnsafePoint(pcdata, c.newprog)
171180

172181
// MOV LR, R3
173182
movlr := obj.Appendp(pcdata, c.newprog)
@@ -204,18 +213,16 @@ func (c *ctxt7) stacksplit(p *obj.Prog, framesize int32) *obj.Prog {
204213
}
205214
call.To.Sym = c.ctxt.Lookup(morestack)
206215

216+
pcdata = c.ctxt.EndUnsafePoint(call, c.newprog, -1)
217+
207218
// B start
208-
jmp := obj.Appendp(call, c.newprog)
219+
jmp := obj.Appendp(pcdata, c.newprog)
209220
jmp.As = AB
210221
jmp.To.Type = obj.TYPE_BRANCH
211222
jmp.Pcond = c.cursym.Func.Text.Link
212223
jmp.Spadj = +framesize
213224

214-
// placeholder for bls's jump target
215-
// p = obj.Appendp(ctxt, p)
216-
// p.As = obj.ANOP
217-
218-
return bls
225+
return end
219226
}
220227

221228
func progedit(ctxt *obj.Link, p *obj.Prog, newprog obj.ProgAlloc) {

‎src/cmd/internal/obj/mips/obj0.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -677,6 +677,12 @@ func (c *ctxt0) stacksplit(p *obj.Prog, framesize int32) *obj.Prog {
677677
p.To.Type = obj.TYPE_REG
678678
p.To.Reg = REG_R1
679679

680+
// Mark the stack bound check and morestack call async nonpreemptible.
681+
// If we get preempted here, when resumed the preemption request is
682+
// cleared, but we'll still call morestack, which will double the stack
683+
// unnecessarily. See issue #35470.
684+
p = c.ctxt.StartUnsafePoint(p, c.newprog)
685+
680686
var q *obj.Prog
681687
if framesize <= objabi.StackSmall {
682688
// small stack: SP < stackguard
@@ -796,7 +802,7 @@ func (c *ctxt0) stacksplit(p *obj.Prog, framesize int32) *obj.Prog {
796802
p.Mark |= LABEL
797803
}
798804

799-
p = c.ctxt.EmitEntryLiveness(c.cursym, p, c.newprog)
805+
p = c.ctxt.EmitEntryStackMap(c.cursym, p, c.newprog)
800806

801807
// JAL runtime.morestack(SB)
802808
p = obj.Appendp(p, c.newprog)
@@ -812,6 +818,8 @@ func (c *ctxt0) stacksplit(p *obj.Prog, framesize int32) *obj.Prog {
812818
}
813819
p.Mark |= BRANCH
814820

821+
p = c.ctxt.EndUnsafePoint(p, c.newprog, -1)
822+
815823
// JMP start
816824
p = obj.Appendp(p, c.newprog)
817825

‎src/cmd/internal/obj/plist.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,13 @@ func (ctxt *Link) Globl(s *LSym, size int64, flag int) {
187187
// liveness map active at the entry of function s. It returns the last
188188
// Prog generated.
189189
func (ctxt *Link) EmitEntryLiveness(s *LSym, p *Prog, newprog ProgAlloc) *Prog {
190+
pcdata := ctxt.EmitEntryStackMap(s, p, newprog)
191+
pcdata = ctxt.EmitEntryRegMap(s, pcdata, newprog)
192+
return pcdata
193+
}
194+
195+
// Similar to EmitEntryLiveness, but just emit stack map.
196+
func (ctxt *Link) EmitEntryStackMap(s *LSym, p *Prog, newprog ProgAlloc) *Prog {
190197
pcdata := Appendp(p, newprog)
191198
pcdata.Pos = s.Func.Text.Pos
192199
pcdata.As = APCDATA
@@ -195,8 +202,12 @@ func (ctxt *Link) EmitEntryLiveness(s *LSym, p *Prog, newprog ProgAlloc) *Prog {
195202
pcdata.To.Type = TYPE_CONST
196203
pcdata.To.Offset = -1 // pcdata starts at -1 at function entry
197204

198-
// Same, with register map.
199-
pcdata = Appendp(pcdata, newprog)
205+
return pcdata
206+
}
207+
208+
// Similar to EmitEntryLiveness, but just emit register map.
209+
func (ctxt *Link) EmitEntryRegMap(s *LSym, p *Prog, newprog ProgAlloc) *Prog {
210+
pcdata := Appendp(p, newprog)
200211
pcdata.Pos = s.Func.Text.Pos
201212
pcdata.As = APCDATA
202213
pcdata.From.Type = TYPE_CONST

‎src/cmd/internal/obj/ppc64/obj9.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1045,6 +1045,12 @@ func (c *ctxt9) stacksplit(p *obj.Prog, framesize int32) *obj.Prog {
10451045
p.To.Type = obj.TYPE_REG
10461046
p.To.Reg = REG_R3
10471047

1048+
// Mark the stack bound check and morestack call async nonpreemptible.
1049+
// If we get preempted here, when resumed the preemption request is
1050+
// cleared, but we'll still call morestack, which will double the stack
1051+
// unnecessarily. See issue #35470.
1052+
p = c.ctxt.StartUnsafePoint(p, c.newprog)
1053+
10481054
var q *obj.Prog
10491055
if framesize <= objabi.StackSmall {
10501056
// small stack: SP < stackguard
@@ -1153,7 +1159,7 @@ func (c *ctxt9) stacksplit(p *obj.Prog, framesize int32) *obj.Prog {
11531159
q.Pcond = p
11541160
}
11551161

1156-
p = c.ctxt.EmitEntryLiveness(c.cursym, p, c.newprog)
1162+
p = c.ctxt.EmitEntryStackMap(c.cursym, p, c.newprog)
11571163

11581164
var morestacksym *obj.LSym
11591165
if c.cursym.CFunc() {
@@ -1239,6 +1245,8 @@ func (c *ctxt9) stacksplit(p *obj.Prog, framesize int32) *obj.Prog {
12391245
p.To.Reg = REG_R2
12401246
}
12411247

1248+
p = c.ctxt.EndUnsafePoint(p, c.newprog, -1)
1249+
12421250
// BR start
12431251
p = obj.Appendp(p, c.newprog)
12441252
p.As = ABR

‎src/cmd/internal/obj/s390x/objz.go

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,7 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) {
335335
if !p.From.Sym.NoSplit() {
336336
p, pPreempt = c.stacksplitPre(p, autosize) // emit pre part of split check
337337
pPre = p
338+
p = c.ctxt.EndUnsafePoint(p, c.newprog, -1)
338339
wasSplit = true //need post part of split
339340
}
340341

@@ -575,15 +576,16 @@ func (c *ctxtz) stacksplitPre(p *obj.Prog, framesize int32) (*obj.Prog, *obj.Pro
575576
p.To.Type = obj.TYPE_REG
576577
p.To.Reg = REG_R3
577578

579+
// Mark the stack bound check and morestack call async nonpreemptible.
580+
// If we get preempted here, when resumed the preemption request is
581+
// cleared, but we'll still call morestack, which will double the stack
582+
// unnecessarily. See issue #35470.
583+
p = c.ctxt.StartUnsafePoint(p, c.newprog)
584+
578585
q = nil
579586
if framesize <= objabi.StackSmall {
580587
// small stack: SP < stackguard
581-
// CMP stackguard, SP
582-
583-
//p.To.Type = obj.TYPE_REG
584-
//p.To.Reg = REGSP
585-
586-
// q1: BLT done
588+
// CMPUBGE stackguard, SP, label-of-call-to-morestack
587589

588590
p = obj.Appendp(p, c.newprog)
589591
//q1 = p
@@ -592,22 +594,11 @@ func (c *ctxtz) stacksplitPre(p *obj.Prog, framesize int32) (*obj.Prog, *obj.Pro
592594
p.Reg = REGSP
593595
p.As = ACMPUBGE
594596
p.To.Type = obj.TYPE_BRANCH
595-
//p = obj.Appendp(ctxt, p)
596-
597-
//p.As = ACMPU
598-
//p.From.Type = obj.TYPE_REG
599-
//p.From.Reg = REG_R3
600-
//p.To.Type = obj.TYPE_REG
601-
//p.To.Reg = REGSP
602-
603-
//p = obj.Appendp(ctxt, p)
604-
//p.As = ABGE
605-
//p.To.Type = obj.TYPE_BRANCH
606597

607598
} else if framesize <= objabi.StackBig {
608599
// large stack: SP-framesize < stackguard-StackSmall
609600
// ADD $-(framesize-StackSmall), SP, R4
610-
// CMP stackguard, R4
601+
// CMPUBGE stackguard, R4, label-of-call-to-morestack
611602
p = obj.Appendp(p, c.newprog)
612603

613604
p.As = AADD
@@ -639,7 +630,7 @@ func (c *ctxtz) stacksplitPre(p *obj.Prog, framesize int32) (*obj.Prog, *obj.Pro
639630
// ADD $StackGuard, SP, R4
640631
// SUB R3, R4
641632
// MOVD $(framesize+(StackGuard-StackSmall)), TEMP
642-
// CMPUBGE TEMP, R4
633+
// CMPUBGE TEMP, R4, label-of-call-to-morestack
643634
p = obj.Appendp(p, c.newprog)
644635

645636
p.As = ACMP
@@ -694,7 +685,8 @@ func (c *ctxtz) stacksplitPost(p *obj.Prog, pPre *obj.Prog, pPreempt *obj.Prog,
694685
spfix.As = obj.ANOP
695686
spfix.Spadj = -framesize
696687

697-
pcdata := c.ctxt.EmitEntryLiveness(c.cursym, spfix, c.newprog)
688+
pcdata := c.ctxt.EmitEntryStackMap(c.cursym, spfix, c.newprog)
689+
pcdata = c.ctxt.StartUnsafePoint(pcdata, c.newprog)
698690

699691
// MOVD LR, R5
700692
p = obj.Appendp(pcdata, c.newprog)
@@ -721,6 +713,8 @@ func (c *ctxtz) stacksplitPost(p *obj.Prog, pPre *obj.Prog, pPreempt *obj.Prog,
721713
p.To.Sym = c.ctxt.Lookup("runtime.morestack")
722714
}
723715

716+
p = c.ctxt.EndUnsafePoint(p, c.newprog, -1)
717+
724718
// BR start
725719
p = obj.Appendp(p, c.newprog)
726720

‎src/cmd/internal/obj/x86/obj6.go

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -998,6 +998,12 @@ func stacksplit(ctxt *obj.Link, cursym *obj.LSym, p *obj.Prog, newprog obj.ProgA
998998
if cursym.CFunc() {
999999
p.To.Offset = 3 * int64(ctxt.Arch.PtrSize) // G.stackguard1
10001000
}
1001+
1002+
// Mark the stack bound check and morestack call async nonpreemptible.
1003+
// If we get preempted here, when resumed the preemption request is
1004+
// cleared, but we'll still call morestack, which will double the stack
1005+
// unnecessarily. See issue #35470.
1006+
p = ctxt.StartUnsafePoint(p, newprog)
10011007
} else if framesize <= objabi.StackBig {
10021008
// large stack: SP-framesize <= stackguard-StackSmall
10031009
// LEAQ -xxx(SP), AX
@@ -1020,6 +1026,8 @@ func stacksplit(ctxt *obj.Link, cursym *obj.LSym, p *obj.Prog, newprog obj.ProgA
10201026
if cursym.CFunc() {
10211027
p.To.Offset = 3 * int64(ctxt.Arch.PtrSize) // G.stackguard1
10221028
}
1029+
1030+
p = ctxt.StartUnsafePoint(p, newprog) // see the comment above
10231031
} else {
10241032
// Such a large stack we need to protect against wraparound.
10251033
// If SP is close to zero:
@@ -1029,11 +1037,11 @@ func stacksplit(ctxt *obj.Link, cursym *obj.LSym, p *obj.Prog, newprog obj.ProgA
10291037
//
10301038
// Preemption sets stackguard to StackPreempt, a very large value.
10311039
// That breaks the math above, so we have to check for that explicitly.
1032-
// MOVQ stackguard, CX
1033-
// CMPQ CX, $StackPreempt
1040+
// MOVQ stackguard, SI
1041+
// CMPQ SI, $StackPreempt
10341042
// JEQ label-of-call-to-morestack
10351043
// LEAQ StackGuard(SP), AX
1036-
// SUBQ CX, AX
1044+
// SUBQ SI, AX
10371045
// CMPQ AX, $(framesize+(StackGuard-StackSmall))
10381046

10391047
p = obj.Appendp(p, newprog)
@@ -1047,6 +1055,8 @@ func stacksplit(ctxt *obj.Link, cursym *obj.LSym, p *obj.Prog, newprog obj.ProgA
10471055
p.To.Type = obj.TYPE_REG
10481056
p.To.Reg = REG_SI
10491057

1058+
p = ctxt.StartUnsafePoint(p, newprog) // see the comment above
1059+
10501060
p = obj.Appendp(p, newprog)
10511061
p.As = cmp
10521062
p.From.Type = obj.TYPE_REG
@@ -1090,6 +1100,8 @@ func stacksplit(ctxt *obj.Link, cursym *obj.LSym, p *obj.Prog, newprog obj.ProgA
10901100
jls.As = AJLS
10911101
jls.To.Type = obj.TYPE_BRANCH
10921102

1103+
end := ctxt.EndUnsafePoint(jls, newprog, -1)
1104+
10931105
var last *obj.Prog
10941106
for last = cursym.Func.Text; last.Link != nil; last = last.Link {
10951107
}
@@ -1101,7 +1113,8 @@ func stacksplit(ctxt *obj.Link, cursym *obj.LSym, p *obj.Prog, newprog obj.ProgA
11011113
spfix.As = obj.ANOP
11021114
spfix.Spadj = -framesize
11031115

1104-
pcdata := ctxt.EmitEntryLiveness(cursym, spfix, newprog)
1116+
pcdata := ctxt.EmitEntryStackMap(cursym, spfix, newprog)
1117+
pcdata = ctxt.StartUnsafePoint(pcdata, newprog)
11051118

11061119
call := obj.Appendp(pcdata, newprog)
11071120
call.Pos = cursym.Func.Text.Pos
@@ -1126,7 +1139,9 @@ func stacksplit(ctxt *obj.Link, cursym *obj.LSym, p *obj.Prog, newprog obj.ProgA
11261139
progedit(ctxt, callend.Link, newprog)
11271140
}
11281141

1129-
jmp := obj.Appendp(callend, newprog)
1142+
pcdata = ctxt.EndUnsafePoint(callend, newprog, -1)
1143+
1144+
jmp := obj.Appendp(pcdata, newprog)
11301145
jmp.As = obj.AJMP
11311146
jmp.To.Type = obj.TYPE_BRANCH
11321147
jmp.Pcond = cursym.Func.Text.Link
@@ -1137,7 +1152,7 @@ func stacksplit(ctxt *obj.Link, cursym *obj.LSym, p *obj.Prog, newprog obj.ProgA
11371152
q1.Pcond = call
11381153
}
11391154

1140-
return jls
1155+
return end
11411156
}
11421157

11431158
var unaryDst = map[obj.As]bool{

0 commit comments

Comments
 (0)