Skip to content

Commit da9e5e1

Browse files
cherrymuijproberts
cherrymui
authored and
jproberts
committed
cmd/internal/obj/arm64: save LR and SP in one instruction for small frames
When we create a thread with signals blocked. But glibc's pthread_sigmask doesn't really allow us to block SIGSETXID. So we may get a signal early on before the signal stack is set. If we get a signal on the current stack, it will clobber anything below the SP. This CL makes it to save LR and decrement SP in a single MOVD.W instruction for small frames, so we don't write below the SP. We used to use a single MOVD.W instruction before CL 379075. CL 379075 changed to use an STP instruction to save the LR and FP, then decrementing the SP. This CL changes it back, just this part (epilogues and large frame prologues are unchanged). For small frames, it is the same number of instructions either way. This decreases the size of a "small" frame from 0x1f0 to 0xf0. For frame sizes in between, it could benefit from using an STP instruction instead of using the prologue for the "large" frame case. We don't bother it for now as this is a stop-gap solution anyway. This only addresses the issue with small frames. Luckily, all functions from thread entry to setting up the signal stack have samll frames. Other possible ideas: - Expand the unwind info metadata, separate SP delta and the location of the return address, so we can express "SP is decremented but the return address is in the LR register". Then we can always create the frame first then write the LR, without writing anything below the SP (except the frame pointer at SP-8, which is minor because it doesn't really affect program execution). - Set up the signal stack immediately in mstart in assembly. For Go 1.19 we do this simple fix. We plan to do the metadata fix in Go 1.20 ( golang#53609 ). Other LR architectures are addressed in CL 413428. Fix golang#53374. Change-Id: I9d6582ab14ccb06ac61ad43852943d9555e22ae5 Reviewed-on: https://go-review.googlesource.com/c/go/+/412474 Run-TryBot: Cherry Mui <cherryyz@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Reviewed-by: Austin Clements <austin@google.com> Reviewed-by: Eric Fang <eric.fang@arm.com>
1 parent e877b49 commit da9e5e1

File tree

3 files changed

+69
-39
lines changed

3 files changed

+69
-39
lines changed

Diff for: ‎misc/cgo/test/cgo_linux_test.go

+8
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,14 @@ func TestSetgid(t *testing.T) {
1515
}
1616
testSetgid(t)
1717
}
18+
19+
func TestSetgidStress(t *testing.T) {
20+
if runtime.GOOS == "android" {
21+
t.Skip("unsupported on Android")
22+
}
23+
testSetgidStress(t)
24+
}
25+
1826
func Test1435(t *testing.T) { test1435(t) }
1927
func Test6997(t *testing.T) { test6997(t) }
2028
func TestBuildID(t *testing.T) { testBuildID(t) }

Diff for: ‎misc/cgo/test/setgid2_linux.go

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
// Copyright 2022 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
// Stress test setgid and thread creation. A thread
6+
// can get a SIGSETXID signal early on at thread
7+
// initialization, causing crash. See issue 53374.
8+
9+
package cgotest
10+
11+
/*
12+
#include <sys/types.h>
13+
#include <unistd.h>
14+
*/
15+
import "C"
16+
17+
import (
18+
"runtime"
19+
"testing"
20+
)
21+
22+
func testSetgidStress(t *testing.T) {
23+
const N = 1000
24+
ch := make(chan int, N)
25+
for i := 0; i < N; i++ {
26+
go func() {
27+
C.setgid(0)
28+
ch <- 1
29+
runtime.LockOSThread() // so every goroutine uses a new thread
30+
}()
31+
}
32+
for i := 0; i < N; i++ {
33+
<-ch
34+
}
35+
}

Diff for: ‎src/cmd/internal/obj/arm64/obj7.go

+26-39
Original file line numberDiff line numberDiff line change
@@ -609,17 +609,17 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) {
609609
var prologueEnd *obj.Prog
610610

611611
aoffset := c.autosize
612-
if aoffset > 0x1f0 {
613-
// LDP offset variant range is -512 to 504, SP should be 16-byte aligned,
614-
// so the maximum aoffset value is 496.
615-
aoffset = 0x1f0
612+
if aoffset > 0xf0 {
613+
// MOVD.W offset variant range is -0x100 to 0xf8, SP should be 16-byte aligned.
614+
// so the maximum aoffset value is 0xf0.
615+
aoffset = 0xf0
616616
}
617617

618618
// Frame is non-empty. Make sure to save link register, even if
619619
// it is a leaf function, so that traceback works.
620620
q = p
621621
if c.autosize > aoffset {
622-
// Frame size is too large for a STP instruction. Store the frame pointer
622+
// Frame size is too large for a MOVD.W instruction. Store the frame pointer
623623
// register and link register before decrementing SP, so if a signal comes
624624
// during the execution of the function prologue, the traceback code will
625625
// not see a half-updated stack frame.
@@ -679,50 +679,37 @@ func preprocess(ctxt *obj.Link, cursym *obj.LSym, newprog obj.ProgAlloc) {
679679
q1.To.Offset = -8
680680
}
681681
} else {
682-
// small frame, save FP and LR with one STP instruction, then update SP.
683-
// Store first, so if a signal comes during the execution of the function
684-
// prologue, the traceback code will not see a half-updated stack frame.
685-
// STP (R29, R30), -aoffset-8(RSP)
682+
// small frame, update SP and save LR in a single MOVD.W instruction.
683+
// So if a signal comes during the execution of the function prologue,
684+
// the traceback code will not see a half-updated stack frame.
685+
// Also, on Linux, in a cgo binary we may get a SIGSETXID signal
686+
// early on before the signal stack is set, as glibc doesn't allow
687+
// us to block SIGSETXID. So it is important that we don't write below
688+
// the SP until the signal stack is set.
689+
// Luckily, all the functions from thread entry to setting the signal
690+
// stack have small frames.
686691
q1 = obj.Appendp(q, c.newprog)
687-
q1.As = ASTP
692+
q1.As = AMOVD
688693
q1.Pos = p.Pos
689-
q1.From.Type = obj.TYPE_REGREG
690-
q1.From.Reg = REGFP
691-
q1.From.Offset = REGLINK
694+
q1.From.Type = obj.TYPE_REG
695+
q1.From.Reg = REGLINK
692696
q1.To.Type = obj.TYPE_MEM
693-
q1.To.Offset = int64(-aoffset - 8)
697+
q1.Scond = C_XPRE
698+
q1.To.Offset = int64(-aoffset)
694699
q1.To.Reg = REGSP
700+
q1.Spadj = aoffset
695701

696702
prologueEnd = q1
697703

698-
q1 = c.ctxt.StartUnsafePoint(q1, c.newprog)
699-
// This instruction is not async preemptible, see the above comment.
700-
// SUB $aoffset, RSP, RSP
704+
// Frame pointer.
701705
q1 = obj.Appendp(q1, c.newprog)
702706
q1.Pos = p.Pos
703-
q1.As = ASUB
704-
q1.From.Type = obj.TYPE_CONST
705-
q1.From.Offset = int64(aoffset)
706-
q1.Reg = REGSP
707-
q1.To.Type = obj.TYPE_REG
707+
q1.As = AMOVD
708+
q1.From.Type = obj.TYPE_REG
709+
q1.From.Reg = REGFP
710+
q1.To.Type = obj.TYPE_MEM
708711
q1.To.Reg = REGSP
709-
q1.Spadj = aoffset
710-
711-
q1 = c.ctxt.EndUnsafePoint(q1, c.newprog, -1)
712-
713-
if buildcfg.GOOS == "ios" {
714-
// See the above comment.
715-
// STP (R29, R30), -8(RSP)
716-
q1 = obj.Appendp(q1, c.newprog)
717-
q1.As = ASTP
718-
q1.Pos = p.Pos
719-
q1.From.Type = obj.TYPE_REGREG
720-
q1.From.Reg = REGFP
721-
q1.From.Offset = REGLINK
722-
q1.To.Type = obj.TYPE_MEM
723-
q1.To.Offset = int64(-8)
724-
q1.To.Reg = REGSP
725-
}
712+
q1.To.Offset = -8
726713
}
727714

728715
prologueEnd.Pos = prologueEnd.Pos.WithXlogue(src.PosPrologueEnd)

0 commit comments

Comments
 (0)