]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: discard SIGPROF delivered to non-Go threads.
authorAlan Donovan <adonovan@google.com>
Tue, 4 Sep 2012 18:40:49 +0000 (14:40 -0400)
committerAlan Donovan <adonovan@google.com>
Tue, 4 Sep 2012 18:40:49 +0000 (14:40 -0400)
Signal handlers are global resources but many language
environments (Go, C++ at Google, etc) assume they have sole
ownership of a particular handler.  Signal handlers in
mixed-language applications must therefore be robust against
unexpected delivery of certain signals, such as SIGPROF.

The default Go signal handler runtime·sigtramp assumes that it
will never be called on a non-Go thread, but this assumption
is violated by when linking in C++ code that spawns threads.
Specifically, the handler asserts the thread has an associated
"m" (Go scheduler).

This CL is a very simple workaround: discard SIGPROF delivered to non-Go threads.  runtime.badsignal(int32) now receives the signal number; if it returns without panicking (e.g. sig==SIGPROF) the signal is discarded.

I don't think there is any really satisfactory solution to the
problem of signal-based profiling in a mixed-language
application.  It's not only the issue of handler clobbering,
but also that a C++ SIGPROF handler called in a Go thread
can't unwind the Go stack (and vice versa).  The best we can
hope for is not crashing.

Note:
- I've ported this to all POSIX platforms, except ARM-linux which already ignores unexpected signals on m-less threads.
- I've avoided tail-calling runtime.badsignal because AFAICT the 6a/6l don't support it.
- I've avoided hoisting 'push sig' (common to both function calls) because it makes the code harder to read.
- Fixed an (apparently incorrect?) docstring.

R=iant, rsc, minux.ma
CC=golang-dev
https://golang.org/cl/6498057

18 files changed:
src/pkg/runtime/signal_linux_amd64.c
src/pkg/runtime/sys_darwin_386.s
src/pkg/runtime/sys_darwin_amd64.s
src/pkg/runtime/sys_freebsd_386.s
src/pkg/runtime/sys_freebsd_amd64.s
src/pkg/runtime/sys_linux_386.s
src/pkg/runtime/sys_linux_amd64.s
src/pkg/runtime/sys_linux_arm.s
src/pkg/runtime/sys_netbsd_386.s
src/pkg/runtime/sys_netbsd_amd64.s
src/pkg/runtime/sys_openbsd_386.s
src/pkg/runtime/sys_openbsd_amd64.s
src/pkg/runtime/thread_darwin.c
src/pkg/runtime/thread_freebsd.c
src/pkg/runtime/thread_linux.c
src/pkg/runtime/thread_netbsd.c
src/pkg/runtime/thread_openbsd.c
src/pkg/runtime/thread_plan9.c

index 8ff5be7859160980d1d90153113bf8e822a578f8..96088f781d68be2bf44e5e745e1f023ea99d3ca1 100644 (file)
@@ -135,6 +135,8 @@ runtime·setsig(int32 i, void (*fn)(int32, Siginfo*, void*, G*), bool restart)
        if(restart)
                sa.sa_flags |= SA_RESTART;
        sa.sa_mask = ~0ULL;
+       // TODO(adonovan): Linux manpage says "sa_restorer element is
+       // obsolete and should not be used".  Avoid it here, and test.
        sa.sa_restorer = (void*)runtime·sigreturn;
        if(fn == runtime·sighandler)
                fn = (void*)runtime·sigtramp;
index 5f7919dc8cc2cb62b1dc22176cb26f1a7764d6d2..c1652090cbe8039abb1366dd7853de6fac877367 100644 (file)
@@ -213,8 +213,8 @@ TEXT runtime·sigaction(SB),7,$0
 // It is called with the following arguments on the stack:
 //     0(FP)   "return address" - ignored
 //     4(FP)   actual handler
-//     8(FP)   siginfo style - ignored
-//     12(FP)  signal number
+//     8(FP)   signal number
+//     12(FP)  siginfo style
 //     16(FP)  siginfo
 //     20(FP)  context
 TEXT runtime·sigtramp(SB),7,$40
@@ -223,8 +223,11 @@ TEXT runtime·sigtramp(SB),7,$40
        // check that m exists
        MOVL    m(CX), BP
        CMPL    BP, $0
-       JNE     2(PC)
+       JNE     5(PC)
+       MOVL    sig+8(FP), BX
+       MOVL    BX, 0(SP)
        CALL    runtime·badsignal(SB)
+       RET
 
        // save g
        MOVL    g(CX), DI
index 36e49ebf8bc46400d01876364e4d685c36cc1c9f..69207c8d8ade5bc990cadfd825de8566e517bc38 100644 (file)
@@ -173,8 +173,10 @@ TEXT runtime·sigtramp(SB),7,$64
        // check that m exists
        MOVQ    m(BX), BP
        CMPQ    BP, $0
-       JNE     2(PC)
+       JNE     4(PC)
+       MOVL    DX, 0(SP)
        CALL    runtime·badsignal(SB)
+       RET
 
        // save g
        MOVQ    g(BX), R10
index a72d8972b15cdc546e531c1098e7dedb8553663c..2cfce09f44eba5d17408632dcc148bbd0b39f053 100644 (file)
@@ -162,8 +162,11 @@ TEXT runtime·sigtramp(SB),7,$44
        // check that m exists
        MOVL    m(CX), BX
        CMPL    BX, $0
-       JNE     2(PC)
+       JNE     5(PC)
+       MOVL    signo+0(FP), BX
+       MOVL    BX, 0(SP)
        CALL    runtime·badsignal(SB)
+       RET
 
        // save g
        MOVL    g(CX), DI
index 36e034a802298738bc475687dfc022ecd196b9da..3d25db2ce7d4207e3580bfaedfda8a259f86a15e 100644 (file)
@@ -138,8 +138,10 @@ TEXT runtime·sigtramp(SB),7,$64
        // check that m exists
        MOVQ    m(BX), BP
        CMPQ    BP, $0
-       JNE     2(PC)
+       JNE     4(PC)
+       MOVQ    DI, 0(SP)
        CALL    runtime·badsignal(SB)
+       RET
 
        // save g
        MOVQ    g(BX), R10
index d9f979f509bb7aa8d8f46f2983253d3cc390e1c2..28ae37b8d97516462b741e68130338af77d2d996 100644 (file)
@@ -170,8 +170,11 @@ TEXT runtime·sigtramp(SB),7,$44
        // check that m exists
        MOVL    m(CX), BX
        CMPL    BX, $0
-       JNE     2(PC)
+       JNE     5(PC)
+       MOVL    sig+0(FP), BX
+       MOVL    BX, 0(SP)
        CALL    runtime·badsignal(SB)
+       RET
 
        // save g
        MOVL    g(CX), DI
index e0ca6583c65e10a4c4af757253b69c6fadec12e1..88810ff74a05aa353464ca91bda0a9d0ce18f890 100644 (file)
@@ -157,8 +157,10 @@ TEXT runtime·sigtramp(SB),7,$64
        // check that m exists
        MOVQ    m(BX), BP
        CMPQ    BP, $0
-       JNE     2(PC)
+       JNE     4(PC)
+       MOVQ    DI, 0(SP)
        CALL    runtime·badsignal(SB)
+       RET
 
        // save g
        MOVQ    g(BX), R10
index 0112cf915855a7cac6a7a0e6bab7233895be7506..38bcebfa1abe238549399969157b8ebc4ec06bf3 100644 (file)
@@ -296,7 +296,8 @@ TEXT runtime·sigaltstack(SB),7,$0
 TEXT runtime·sigtramp(SB),7,$24
        // this might be called in external code context,
        // where g and m are not set.
-       // first save R0, becuase cgo_load_gm will clobber it
+       // first save R0, because cgo_load_gm will clobber it
+       // TODO(adonovan): call runtime·badsignal if m=0, like other platforms?
        MOVW    R0, 4(R13)
        MOVW    cgo_load_gm(SB), R0
        CMP     $0, R0
index 75a38f820edae97656402d312b020646118da943..5f6738ee2d3fb662f0a3c1a071a8957ac003012a 100644 (file)
@@ -178,8 +178,11 @@ TEXT runtime·sigtramp(SB),7,$44
        // check that m exists
        MOVL    m(CX), BX
        CMPL    BX, $0
-       JNE     2(PC)
+       JNE     5(PC)
+       MOVL    signo+0(FP), BX
+       MOVL    BX, 0(SP)      
        CALL    runtime·badsignal(SB)
+       RET
 
        // save g
        MOVL    g(CX), DI
index f5feb48418f0aa4719a189d77b4dcdce0c06ef64..9fe1ebbc492e5ffac50711a6cbebde71ac8341b7 100644 (file)
@@ -196,8 +196,10 @@ TEXT runtime·sigtramp(SB),7,$64
        // check that m exists
        MOVQ    m(BX), BP
        CMPQ    BP, $0
-       JNE     2(PC)
+       JNE     4(PC)
+       MOVQ    DI, 0(SP)
        CALL    runtime·badsignal(SB)
+       RET
 
        // save g
        MOVQ    g(BX), R10
index 0774162f64bdaaa382f8b8eddd6543891918f631..d04b5e653a25b7990093ebff5386df00d20edd5c 100644 (file)
@@ -151,8 +151,11 @@ TEXT runtime·sigtramp(SB),7,$44
        // check that m exists
        MOVL    m(CX), BX
        CMPL    BX, $0
-       JNE     2(PC)
+       JNE     5(PC)
+       MOVL    signo+0(FP), BX
+       MOVL    BX, 0(SP)
        CALL    runtime·badsignal(SB)
+       RET
 
        // save g
        MOVL    g(CX), DI
index 9df903f74f93818e8cbc3dcc512e65b357fc3874..ad7de11f841700dfdda1a6f725d068d74500901d 100644 (file)
@@ -187,8 +187,10 @@ TEXT runtime·sigtramp(SB),7,$64
        // check that m exists
        MOVQ    m(BX), BP
        CMPQ    BP, $0
-       JNE     2(PC)
+       JNE     4(PC)
+       MOVQ    DI, 0(SP)
        CALL    runtime·badsignal(SB)
+       RET
 
        // save g
        MOVQ    g(BX), R10
index bfdd9873ea8bc4d9c90c53a7389d76272f3114ed..aff2b6fd371f3a2f15333cd7ee51b862ddf20e78 100644 (file)
@@ -502,7 +502,11 @@ static int8 badsignal[] = "runtime: signal received on thread not created by Go.
 // This runs on a foreign stack, without an m or a g.  No stack split.
 #pragma textflag 7
 void
-runtime·badsignal(void)
+runtime·badsignal(int32 sig)
 {
+       if (sig == SIGPROF) {
+               return;  // Ignore SIGPROFs intended for a non-Go thread.
+       }
        runtime·write(2, badsignal, sizeof badsignal - 1);
+       runtime·exit(1);
 }
index 1597b1e88b10eae186bb6c1eef6bb68f2f6de0aa..4d39f3c8042a34e0886cf4512132ee65c42557e0 100644 (file)
@@ -211,7 +211,11 @@ static int8 badsignal[] = "runtime: signal received on thread not created by Go.
 // This runs on a foreign stack, without an m or a g.  No stack split.
 #pragma textflag 7
 void
-runtime·badsignal(void)
+runtime·badsignal(int32 sig)
 {
+       if (sig == SIGPROF) {
+               return;  // Ignore SIGPROFs intended for a non-Go thread.
+       }
        runtime·write(2, badsignal, sizeof badsignal - 1);
+       runtime·exit(1);
 }
index f66d2dd4d256adab8b8358caa5372c2f1c230c23..c428ba1b392dcdddd0252e4df5c3985fc0cf2256 100644 (file)
@@ -261,7 +261,11 @@ static int8 badsignal[] = "runtime: signal received on thread not created by Go.
 // This runs on a foreign stack, without an m or a g.  No stack split.
 #pragma textflag 7
 void
-runtime·badsignal(void)
+runtime·badsignal(int32 sig)
 {
+       if (sig == SIGPROF) {
+               return;  // Ignore SIGPROFs intended for a non-Go thread.
+       }
        runtime·write(2, badsignal, sizeof badsignal - 1);
+       runtime·exit(1);
 }
index be6c205c28110d7176846b2944e732b7c2bfb85a..a703e0714ad52e4198c50dce07bb2cc90eec8e4d 100644 (file)
@@ -261,7 +261,11 @@ static int8 badsignal[] = "runtime: signal received on thread not created by Go.
 // This runs on a foreign stack, without an m or a g.  No stack split.
 #pragma textflag 7
 void
-runtime·badsignal(void)
+runtime·badsignal(int32 sig)
 {
+       if (sig == SIGPROF) {
+               return;  // Ignore SIGPROFs intended for a non-Go thread.
+       }
        runtime·write(2, badsignal, sizeof badsignal - 1);
+       runtime·exit(1);
 }
index 4e4db747457bc21da41874e88104342abe018e6e..c55f25278f03a444b7edba8e8572dd58ddababd3 100644 (file)
@@ -234,7 +234,11 @@ static int8 badsignal[] = "runtime: signal received on thread not created by Go.
 // This runs on a foreign stack, without an m or a g.  No stack split.
 #pragma textflag 7
 void
-runtime·badsignal(void)
+runtime·badsignal(int32 sig)
 {
+       if (sig == SIGPROF) {
+               return;  // Ignore SIGPROFs intended for a non-Go thread.
+       }
        runtime·write(2, badsignal, sizeof badsignal - 1);
+       runtime.exit(1)
 }
index 57d535713d2bb44a4deaad8122aff545df8459cb..9898a65b288a485b0e976303431aaca320073edb 100644 (file)
@@ -354,7 +354,7 @@ static int8 badsignal[] = "runtime: signal received on thread not created by Go.
 // This runs on a foreign stack, without an m or a g.  No stack split.
 #pragma textflag 7
 void
-runtime·badsignal(void)
+runtime·badsignal(int32 sig)
 {
        runtime·pwrite(2, badsignal, sizeof badsignal - 1, -1LL);
 }