runtime: save/fetch g register during VDSO on ARM and ARM64

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>

diff --git a/src/os/signal/signal_test.go b/src/os/signal/signal_test.go
index d50e595d84d826a921af71ef3470de965e1395c0..7aa3d7805b28f0b1ef30b0480765d361e65f4993 100644 (file)
--- a/src/os/signal/signal_test.go
+++ b/src/os/signal/signal_test.go
@@ -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)
+}

diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index 7d2ff2748bf0f1b4f7a5f299e6bdf2aa90452712..4814a5fc8777d3f2718860030df81f33c6ae476a 100644 (file)
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -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
 }

diff --git a/src/runtime/signal_unix.go b/src/runtime/signal_unix.go
index d5a04b6d48cbe7a3864d0ceda856bb2a00f454df..cea65282e0a4e32dc4a17d21403a9f3faaf2b7e1 100644 (file)
--- a/src/runtime/signal_unix.go
+++ b/src/runtime/signal_unix.go
@@ -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
                }
        }

diff --git a/src/runtime/sys_linux_arm.s b/src/runtime/sys_linux_arm.s
index 23a66554ab4758f1d334224dee5371d30aed0e97..9a9e1c92c75119d9806b767dcf8608470dc5b821 100644 (file)
--- a/src/runtime/sys_linux_arm.s
+++ b/src/runtime/sys_linux_arm.s
@@ -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:
@@ -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:

diff --git a/src/runtime/sys_linux_arm64.s b/src/runtime/sys_linux_arm64.s
index 5514a6be62d4d9aa7dcb4666f879ac57f5381e53..a77be98739fc6937d4d1f961b1eaa65cd47dbed6 100644 (file)
--- a/src/runtime/sys_linux_arm64.s
+++ b/src/runtime/sys_linux_arm64.s
@@ -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:
@@ -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: