Skip to content

Commit 16e82be

Browse files
derekparkerlaboger
derekparker
authored and
laboger
committed
runtime: fix crash during VDSO calls on PowerPC
This patch reinstates a fix for PowerPC with regard to making VDSO calls while receiving a signal, and subsequently crashing. The crash happens because certain VDSO calls can modify the r30 register, which is where g is stored. This change was reverted for PowerPC because r30 is supposed to be a non-volatile register. This is true, but that only makes a guarantee across function calls, but not "within" a function call. This patch was seemingly fine before because the Linux kernel still had hand rolled assembly VDSO function calls, however with a recent change to C function calls it seems the compiler used can generate instructions which temporarily clobber r30. This means that when we receive a signal during one of these calls the value of r30 will not be the g as the runtime expects, causing a segfault. You can see from this assembly dump how the register is clobbered during the call: (the following is from a 5.13rc2 kernel) ``` Dump of assembler code for function __cvdso_clock_gettime_data: 0x00007ffff7ff0700 <+0>: cmplwi r4,15 0x00007ffff7ff0704 <+4>: bgt 0x7ffff7ff07f0 <__cvdso_clock_gettime_data+240> 0x00007ffff7ff0708 <+8>: li r9,1 0x00007ffff7ff070c <+12>: slw r9,r9,r4 0x00007ffff7ff0710 <+16>: andi. r10,r9,2179 0x00007ffff7ff0714 <+20>: beq 0x7ffff7ff0810 <__cvdso_clock_gettime_data+272> 0x00007ffff7ff0718 <+24>: rldicr r10,r4,4,59 0x00007ffff7ff071c <+28>: lis r9,32767 0x00007ffff7ff0720 <+32>: std r30,-16(r1) 0x00007ffff7ff0724 <+36>: std r31,-8(r1) 0x00007ffff7ff0728 <+40>: add r6,r3,r10 0x00007ffff7ff072c <+44>: ori r4,r9,65535 0x00007ffff7ff0730 <+48>: lwz r8,0(r3) 0x00007ffff7ff0734 <+52>: andi. r9,r8,1 0x00007ffff7ff0738 <+56>: bne 0x7ffff7ff07d0 <__cvdso_clock_gettime_data+208> 0x00007ffff7ff073c <+60>: lwsync 0x00007ffff7ff0740 <+64>: mftb r30 <---- RIGHT HERE => 0x00007ffff7ff0744 <+68>: ld r12,40(r6) ``` What I believe is happening is that the kernel changed the PowerPC VDSO calls to use standard C calls instead of using hand rolled assembly. The hand rolled assembly calls never touched r30, so this change was safe to roll back. That does not seem to be the case anymore as on the 5.13rc2 kernel the compiler *is* generating assembly which modifies r30, making this change again unsafe and causing a crash when the program receives a signal during these calls (which will happen often due to async preempt). This change happened here: https://lwn.net/ml/linux-kernel/235e5571959cfa89ced081d7e838ed5ff38447d2.1601365870.git.christophe.leroy@csgroup.eu/. I realize this was reverted due to unexplained hangs in PowerPC builders, but I think we should reinstate this change and investigate those issues separately: f4ca3c1 Fixes #46803 Change-Id: Ib18d7bbfc80a1a9cb558f0098878d41081324b52 GitHub-Last-Rev: c3002bc GitHub-Pull-Request: #46767 Reviewed-on: https://go-review.googlesource.com/c/go/+/328110 Run-TryBot: Lynn Boger <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Cherry Mui <[email protected]> Trust: Lynn Boger <[email protected]>
1 parent 2e542c3 commit 16e82be

File tree

2 files changed

+74
-14
lines changed

2 files changed

+74
-14
lines changed

‎src/runtime/signal_unix.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ func preemptM(mp *m) {
382382
//go:nosplit
383383
func sigFetchG(c *sigctxt) *g {
384384
switch GOARCH {
385-
case "arm", "arm64":
385+
case "arm", "arm64", "ppc64", "ppc64le":
386386
if !iscgo && inVDSOPage(c.sigpc()) {
387387
// When using cgo, we save the g on TLS and load it from there
388388
// in sigtramp. Just use that.

‎src/runtime/sys_linux_ppc64x.s

Lines changed: 73 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -216,15 +216,45 @@ TEXT runtime·walltime(SB),NOSPLIT,$16-12
216216
MOVD (g_sched+gobuf_sp)(R7), R1 // Set SP to g0 stack
217217

218218
noswitch:
219-
SUB $16, R1 // Space for results
220-
RLDICR $0, R1, $59, R1 // Align for C code
219+
SUB $16, R1 // Space for results
220+
RLDICR $0, R1, $59, R1 // Align for C code
221221
MOVD R12, CTR
222222
MOVD R1, R4
223-
BL (CTR) // Call from VDSO
224-
MOVD $0, R0 // Restore R0
225-
MOVD 0(R1), R3 // sec
226-
MOVD 8(R1), R5 // nsec
227-
MOVD R15, R1 // Restore SP
223+
224+
// Store g on gsignal's stack, so if we receive a signal
225+
// during VDSO code we can find the g.
226+
// If we don't have a signal stack, we won't receive signal,
227+
// so don't bother saving g.
228+
// When using cgo, we already saved g on TLS, also don't save
229+
// g here.
230+
// Also don't save g if we are already on the signal stack.
231+
// We won't get a nested signal.
232+
MOVBZ runtime·iscgo(SB), R22
233+
CMP R22, $0
234+
BNE nosaveg
235+
MOVD m_gsignal(R21), R22 // g.m.gsignal
236+
CMP R22, $0
237+
BEQ nosaveg
238+
239+
CMP g, R22
240+
BEQ nosaveg
241+
MOVD (g_stack+stack_lo)(R22), R22 // g.m.gsignal.stack.lo
242+
MOVD g, (R22)
243+
244+
BL (CTR) // Call from VDSO
245+
246+
MOVD $0, (R22) // clear g slot, R22 is unchanged by C code
247+
248+
JMP finish
249+
250+
nosaveg:
251+
BL (CTR) // Call from VDSO
252+
253+
finish:
254+
MOVD $0, R0 // Restore R0
255+
MOVD 0(R1), R3 // sec
256+
MOVD 8(R1), R5 // nsec
257+
MOVD R15, R1 // Restore SP
228258

229259
// Restore vdsoPC, vdsoSP
230260
// We don't worry about being signaled between the two stores.
@@ -236,7 +266,7 @@ noswitch:
236266
MOVD 32(R1), R6
237267
MOVD R6, m_vdsoPC(R21)
238268

239-
finish:
269+
return:
240270
MOVD R3, sec+0(FP)
241271
MOVW R5, nsec+8(FP)
242272
RET
@@ -247,7 +277,7 @@ fallback:
247277
SYSCALL $SYS_clock_gettime
248278
MOVD 32(R1), R3
249279
MOVD 40(R1), R5
250-
JMP finish
280+
JMP return
251281

252282
TEXT runtime·nanotime1(SB),NOSPLIT,$16-8
253283
MOVD $1, R3 // CLOCK_MONOTONIC
@@ -283,7 +313,37 @@ noswitch:
283313
RLDICR $0, R1, $59, R1 // Align for C code
284314
MOVD R12, CTR
285315
MOVD R1, R4
286-
BL (CTR) // Call from VDSO
316+
317+
// Store g on gsignal's stack, so if we receive a signal
318+
// during VDSO code we can find the g.
319+
// If we don't have a signal stack, we won't receive signal,
320+
// so don't bother saving g.
321+
// When using cgo, we already saved g on TLS, also don't save
322+
// g here.
323+
// Also don't save g if we are already on the signal stack.
324+
// We won't get a nested signal.
325+
MOVBZ runtime·iscgo(SB), R22
326+
CMP R22, $0
327+
BNE nosaveg
328+
MOVD m_gsignal(R21), R22 // g.m.gsignal
329+
CMP R22, $0
330+
BEQ nosaveg
331+
332+
CMP g, R22
333+
BEQ nosaveg
334+
MOVD (g_stack+stack_lo)(R22), R22 // g.m.gsignal.stack.lo
335+
MOVD g, (R22)
336+
337+
BL (CTR) // Call from VDSO
338+
339+
MOVD $0, (R22) // clear g slot, R22 is unchanged by C code
340+
341+
JMP finish
342+
343+
nosaveg:
344+
BL (CTR) // Call from VDSO
345+
346+
finish:
287347
MOVD $0, R0 // Restore R0
288348
MOVD 0(R1), R3 // sec
289349
MOVD 8(R1), R5 // nsec
@@ -299,7 +359,7 @@ noswitch:
299359
MOVD 32(R1), R6
300360
MOVD R6, m_vdsoPC(R21)
301361

302-
finish:
362+
return:
303363
// sec is in R3, nsec in R5
304364
// return nsec in R3
305365
MOVD $1000000000, R4
@@ -314,7 +374,7 @@ fallback:
314374
SYSCALL $SYS_clock_gettime
315375
MOVD 32(R1), R3
316376
MOVD 40(R1), R5
317-
JMP finish
377+
JMP return
318378

319379
TEXT runtime·rtsigprocmask(SB),NOSPLIT|NOFRAME,$0-28
320380
MOVW how+0(FP), R3
@@ -469,7 +529,7 @@ TEXT sigtramp<>(SB),NOSPLIT|NOFRAME,$0
469529
// this might be called in external code context,
470530
// where g is not set.
471531
MOVBZ runtime·iscgo(SB), R6
472-
CMP R6, $0
532+
CMP R6, $0
473533
BEQ 2(PC)
474534
BL runtime·load_g(SB)
475535

0 commit comments

Comments
 (0)