runtime/cgo: return correct sa_flags

A typo in the previous revision ("act" instead of "oldact") caused us
to return the sa_flags from the new (or zeroed) sigaction rather than
the old one.

In the presence of a signal handler registered before
runtime.libpreinit, this caused setsigstack to erroneously zero out
important sa_flags (such as SA_SIGINFO) in its attempt to re-register
the existing handler with SA_ONSTACK.

Change-Id: I3cd5152a38ec0d44ae611f183bc1651d65b8a115
Reviewed-on: https://go-review.googlesource.com/37852
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

diff --git a/misc/cgo/testsanitizers/test.bash b/misc/cgo/testsanitizers/test.bash
index 556ef108205dc1854b02646d73e1728fc076d458..80de7388f632106c75e6e5ed3f65945aa08dfff6 100755 (executable)
--- a/misc/cgo/testsanitizers/test.bash
+++ b/misc/cgo/testsanitizers/test.bash
@@ -77,7 +77,7 @@ testmsanshared() {
   fi
   go build -msan -buildmode=c-shared $suffix -o ${TMPDIR}/libmsanshared.$libext msan_shared.go
 
-       echo 'int main() { return 0; }' > ${TMPDIR}/testmsanshared.c
+  echo 'int main() { return 0; }' > ${TMPDIR}/testmsanshared.c
   $CC $(go env GOGCCFLAGS) -fsanitize=memory -o ${TMPDIR}/testmsanshared ${TMPDIR}/testmsanshared.c ${TMPDIR}/libmsanshared.$libext
 
   if ! LD_LIBRARY_PATH=. ${TMPDIR}/testmsanshared; then
@@ -131,6 +131,25 @@ if test "$msan" = "yes"; then
     testmsanshared
 fi
 
+testtsanshared() {
+  goos=$(go env GOOS)
+  suffix="-installsuffix tsan"
+  libext="so"
+  if [ "$goos" == "darwin" ]; then
+         libext="dylib"
+  fi
+  go build -buildmode=c-shared $suffix -o ${TMPDIR}/libtsanshared.$libext tsan_shared.go
+
+  echo 'int main() { return 0; }' > ${TMPDIR}/testtsanshared.c
+  $CC $(go env GOGCCFLAGS) -fsanitize=thread -o ${TMPDIR}/testtsanshared ${TMPDIR}/testtsanshared.c ${TMPDIR}/libtsanshared.$libext
+
+  if ! LD_LIBRARY_PATH=. ${TMPDIR}/testtsanshared; then
+    echo "FAIL: tsan_shared"
+    status=1
+  fi
+  rm -f ${TMPDIR}/{testtsanshared,testtsanshared.c,libtsanshared.$libext}
+}
+
 if test "$tsan" = "yes"; then
     echo 'int main() { return 0; }' > ${TMPDIR}/testsanitizers$$.c
     ok=yes
@@ -196,6 +215,8 @@ if test "$tsan" = "yes"; then
        testtsan tsan7.go "CGO_CFLAGS=-fsanitize=thread CGO_LDFLAGS=-fsanitize=thread" "-installsuffix=tsan"
        testtsan tsan10.go "CGO_CFLAGS=-fsanitize=thread CGO_LDFLAGS=-fsanitize=thread" "-installsuffix=tsan"
        testtsan tsan11.go "CGO_CFLAGS=-fsanitize=thread CGO_LDFLAGS=-fsanitize=thread" "-installsuffix=tsan"
+
+       testtsanshared
     fi
 fi
 

diff --git a/misc/cgo/testsanitizers/tsan_shared.go b/misc/cgo/testsanitizers/tsan_shared.go
new file mode 100644 (file)
index 0000000..55ff67e
--- /dev/null
+++ b/misc/cgo/testsanitizers/tsan_shared.go
@@ -0,0 +1,63 @@
+// Copyright 2017 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+// This program failed with SIGSEGV when run under the C/C++ ThreadSanitizer.
+// The Go runtime had re-registered the C handler with the wrong flags due to a
+// typo, resulting in null pointers being passed for the info and context
+// parameters to the handler.
+
+/*
+#cgo CFLAGS: -fsanitize=thread
+#cgo LDFLAGS: -fsanitize=thread
+
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <ucontext.h>
+
+void check_params(int signo, siginfo_t *info, void *context) {
+       ucontext_t* uc = (ucontext_t*)(context);
+
+       if (info->si_signo != signo) {
+               fprintf(stderr, "info->si_signo does not match signo.\n");
+               abort();
+       }
+
+       if (uc->uc_stack.ss_size == 0) {
+               fprintf(stderr, "uc_stack has size 0.\n");
+               abort();
+       }
+}
+
+
+// Set up the signal handler in a high priority constructor, so
+// that it is installed before the Go code starts.
+
+static void register_handler(void) __attribute__ ((constructor (200)));
+
+static void register_handler() {
+       struct sigaction sa;
+       memset(&sa, 0, sizeof(sa));
+       sigemptyset(&sa.sa_mask);
+       sa.sa_flags = SA_SIGINFO;
+       sa.sa_sigaction = check_params;
+
+       if (sigaction(SIGUSR1, &sa, NULL) != 0) {
+               perror("failed to register SIGUSR1 handler");
+               exit(EXIT_FAILURE);
+       }
+}
+*/
+import "C"
+
+import "syscall"
+
+func init() {
+       C.raise(C.int(syscall.SIGUSR1))
+}
+
+func main() {}

diff --git a/src/runtime/cgo/gcc_sigaction.c b/src/runtime/cgo/gcc_sigaction.c
index 5aca2710bd4883f05e16601ed9b3267ed91974fd..566097fa6d7684d77e0d2892474f017ed699dc2c 100644 (file)
--- a/src/runtime/cgo/gcc_sigaction.c
+++ b/src/runtime/cgo/gcc_sigaction.c
@@ -65,11 +65,11 @@ x_cgo_sigaction(intptr_t signum, const go_sigaction_t *goact, go_sigaction_t *ol
                }
                oldgoact->mask = 0;
                for (i = 0; i < 8 * sizeof(oldgoact->mask); i++) {
-                       if (sigismember(&act.sa_mask, i+1) == 1) {
+                       if (sigismember(&oldact.sa_mask, i+1) == 1) {
                                oldgoact->mask |= (uint64_t)(1)<<i;
                        }
                }
-               oldgoact->flags = act.sa_flags;
+               oldgoact->flags = oldact.sa_flags;
        }
 
        return ret;