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>

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
 

misc/cgo/testsanitizers/tsan_shared.go

+// 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() {}

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;