Skip to content

Commit

Permalink
runtime: save/fetch g register during VDSO on ARM and ARM64
Browse files Browse the repository at this point in the history
On ARM and ARM64, during a VDSO call, the g register may be
temporarily clobbered by the VDSO code. If a signal is received
during the execution of VDSO code, we may not find a valid g
reading the g register. In CL 192937, we conservatively assume
g is nil. But this approach has a problem: we cannot handle
the signal in this case. Further, if the signal is not a
profiling signal, we'll call badsignal, which calls needm, which
wants to get an extra m, but we don't have one in a non-cgo
binary, which cuases the program to hang.

This is even more of a problem with async preemption, where we
will receive more signals than before. I ran into this problem
while working on async preemption support on ARM64.

In this CL, before making a VDSO call, we save the g on the
gsignal stack. When we receive a signal, we will be running on
the gsignal stack, so we can fetch the g from there and move on.

We probably want to do the same for PPC64. Currently we rely on
that the VDSO code doesn't actually clobber the g register, but
this is not guaranteed and we don't have control with.

Idea from discussion with Dan Cross and Austin.

Should fix #34391.

Change-Id: Idbefc5e4c2f4373192c2be797be0140ae08b26e3
Reviewed-on: https://go-review.googlesource.com/c/go/+/202759
Run-TryBot: Cherry Zhang <cherryyz@google.com>
Reviewed-by: Austin Clements <austin@google.com>
  • Loading branch information
cherrymui committed Oct 23, 2019
1 parent 20bba86 commit 758eb02
Show file tree
Hide file tree
Showing 5 changed files with 122 additions and 0 deletions.
49 changes: 49 additions & 0 deletions src/os/signal/signal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -469,3 +469,52 @@ func atomicStopTestProgram() {

os.Exit(0)
}

func TestTime(t *testing.T) {
// Test that signal works fine when we are in a call to get time,
// which on some platforms is using VDSO. See issue #34391.
dur := 3 * time.Second
if testing.Short() {
dur = 100 * time.Millisecond
}
defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(4))
done := make(chan bool)
finished := make(chan bool)
go func() {
sig := make(chan os.Signal, 1)
Notify(sig, syscall.SIGUSR1)
defer Stop(sig)
Loop:
for {
select {
case <-sig:
case <-done:
break Loop
}
}
finished <- true
}()
go func() {
Loop:
for {
select {
case <-done:
break Loop
default:
syscall.Kill(syscall.Getpid(), syscall.SIGUSR1)
runtime.Gosched()
}
}
finished <- true
}()
t0 := time.Now()
for t1 := t0; t1.Sub(t0) < dur; t1 = time.Now() {
} // hammering on getting time
close(done)
<-finished
<-finished
// When run with 'go test -cpu=1,2,4' SIGUSR1 from this test can slip
// into subsequent TestSignal() causing failure.
// Sleep for a while to reduce the possibility of the failure.
time.Sleep(10 * time.Millisecond)
}
3 changes: 3 additions & 0 deletions src/runtime/proc.go
Original file line number Diff line number Diff line change
Expand Up @@ -3420,6 +3420,9 @@ func malg(stacksize int32) *g {
})
newg.stackguard0 = newg.stack.lo + _StackGuard
newg.stackguard1 = ^uintptr(0)
// Clear the bottom word of the stack. We record g
// there on gsignal stack during VDSO on ARM and ARM64.
*(*uintptr)(unsafe.Pointer(newg.stack.lo)) = 0
}
return newg
}
Expand Down
10 changes: 10 additions & 0 deletions src/runtime/signal_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,16 @@ func sigFetchG(c *sigctxt) *g {
switch GOARCH {
case "arm", "arm64":
if inVDSOPage(c.sigpc()) {
// Before making a VDSO call we save the g to the bottom of the
// signal stack. Fetch from there.
// TODO: in efence mode, stack is sysAlloc'd, so this wouldn't
// work.
sp := getcallersp()
s := spanOf(sp)
if s != nil && s.state == mSpanManual && s.base() < sp && sp < s.limit {
gp := *(**g)(unsafe.Pointer(s.base()))
return gp
}
return nil
}
}
Expand Down
32 changes: 32 additions & 0 deletions src/runtime/sys_linux_arm.s
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,23 @@ noswitch:
CMP $0, R11
B.EQ fallback

// Store g on gsignal's stack, so if we receive a signal
// during VDSO code we can find the g.
// If we don't have a signal stack, we won't receive signal,
// so don't bother saving g.
MOVW m_gsignal(R5), R6 // g.m.gsignal
CMP $0, R6
BEQ 3(PC)
MOVW (g_stack+stack_lo)(R6), R6 // g.m.gsignal.stack.lo
MOVW g, (R6)

BL (R11)

CMP $0, R6 // R6 is unchanged by C code
BEQ 3(PC)
MOVW $0, R1
MOVW R1, (R6) // clear g slot

JMP finish

fallback:
Expand Down Expand Up @@ -310,7 +326,23 @@ noswitch:
CMP $0, R11
B.EQ fallback

// Store g on gsignal's stack, so if we receive a signal
// during VDSO code we can find the g.
// If we don't have a signal stack, we won't receive signal,
// so don't bother saving g.
MOVW m_gsignal(R5), R6 // g.m.gsignal
CMP $0, R6
BEQ 3(PC)
MOVW (g_stack+stack_lo)(R6), R6 // g.m.gsignal.stack.lo
MOVW g, (R6)

BL (R11)

CMP $0, R6 // R6 is unchanged by C code
BEQ 3(PC)
MOVW $0, R1
MOVW R1, (R6) // clear g slot

JMP finish

fallback:
Expand Down
28 changes: 28 additions & 0 deletions src/runtime/sys_linux_arm64.s
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,21 @@ noswitch:
MOVW $CLOCK_REALTIME, R0
MOVD runtime·vdsoClockgettimeSym(SB), R2
CBZ R2, fallback

// Store g on gsignal's stack, so if we receive a signal
// during VDSO code we can find the g.
// If we don't have a signal stack, we won't receive signal,
// so don't bother saving g.
MOVD m_gsignal(R21), R22 // g.m.gsignal
CBZ R22, 3(PC)
MOVD (g_stack+stack_lo)(R22), R22 // g.m.gsignal.stack.lo
MOVD g, (R22)

BL (R2)

CBZ R22, 2(PC) // R22 is unchanged by C code
MOVD ZR, (R22) // clear g slot

B finish

fallback:
Expand Down Expand Up @@ -261,7 +275,21 @@ noswitch:
MOVW $CLOCK_MONOTONIC, R0
MOVD runtime·vdsoClockgettimeSym(SB), R2
CBZ R2, fallback

// Store g on gsignal's stack, so if we receive a signal
// during VDSO code we can find the g.
// If we don't have a signal stack, we won't receive signal,
// so don't bother saving g.
MOVD m_gsignal(R21), R22 // g.m.gsignal
CBZ R22, 3(PC)
MOVD (g_stack+stack_lo)(R22), R22 // g.m.gsignal.stack.lo
MOVD g, (R22)

BL (R2)

CBZ R22, 2(PC) // R22 is unchanged by C code
MOVD ZR, (R22) // clear g slot

B finish

fallback:
Expand Down

0 comments on commit 758eb02

Please sign in to comment.