runtime: poll libc to deliver signals under TSAN

fixes #18717

Change-Id: I7244463d2e7489e0b0fe3b74c4b782e71210beb2
Reviewed-on: https://go-review.googlesource.com/35494
Run-TryBot: Bryan Mills <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>

Author: Bryan C. Mills

misc/cgo/testsanitizers/test.bash

@@ -198,6 +198,9 @@ if test "$tsan" = "yes"; then
 
 	# This test requires rebuilding runtime/cgo with -fsanitize=thread.
 	testtsan tsan7.go "CGO_CFLAGS=-fsanitize=thread CGO_LDFLAGS=-fsanitize=thread" "-installsuffix=tsan"
+
+	# This test requires rebuilding runtime/cgo with -fsanitize=thread.
+	testtsan tsan10.go "CGO_CFLAGS=-fsanitize=thread CGO_LDFLAGS=-fsanitize=thread" "-installsuffix=tsan"
     fi
 fi
 

misc/cgo/testsanitizers/tsan10.go

@@ -0,0 +1,31 @@
+// 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 hung when run under the C/C++ ThreadSanitizer.
+// TSAN defers asynchronous signals until the signaled thread calls into libc.
+// Since the Go runtime makes direct futex syscalls, Go runtime threads could
+// run for an arbitrarily long time without triggering the libc interceptors.
+// See https://golang.org/issue/18717.
+
+import (
+	"os"
+	"os/signal"
+	"syscall"
+)
+
+/*
+#cgo CFLAGS: -g -fsanitize=thread
+#cgo LDFLAGS: -g -fsanitize=thread
+*/
+import "C"
+
+func main() {
+	c := make(chan os.Signal, 1)
+	signal.Notify(c, syscall.SIGUSR1)
+	defer signal.Stop(c)
+	syscall.Kill(syscall.Getpid(), syscall.SIGUSR1)
+	<-c
+}

src/runtime/cgo.go

@@ -16,6 +16,7 @@ import "unsafe"
 //go:linkname _cgo_notify_runtime_init_done _cgo_notify_runtime_init_done
 //go:linkname _cgo_callers _cgo_callers
 //go:linkname _cgo_set_context_function _cgo_set_context_function
+//go:linkname _cgo_yield _cgo_yield
 
 var (
 	_cgo_init                     unsafe.Pointer
@@ -24,6 +25,7 @@ var (
 	_cgo_notify_runtime_init_done unsafe.Pointer
 	_cgo_callers                  unsafe.Pointer
 	_cgo_set_context_function     unsafe.Pointer
+	_cgo_yield                    unsafe.Pointer
 )
 
 // iscgo is set to true by the runtime/cgo package

src/runtime/cgo/callbacks.go

@@ -92,5 +92,15 @@ var _cgo_notify_runtime_init_done = &x_cgo_notify_runtime_init_done
 var x_cgo_set_context_function byte
 var _cgo_set_context_function = &x_cgo_set_context_function
 
+// Calls a libc function to execute background work injected via libc
+// interceptors, such as processing pending signals under the thread
+// sanitizer.
+//
+// Left as a nil pointer if no libc interceptors are expected.
+
+//go:cgo_import_static _cgo_yield
+//go:linkname _cgo_yield _cgo_yield
+var _cgo_yield unsafe.Pointer
+
 //go:cgo_export_static _cgo_topofstack
 //go:cgo_export_dynamic _cgo_topofstack

src/runtime/cgo/gcc_util.c

@@ -22,3 +22,39 @@ x_cgo_thread_start(ThreadStart *arg)
 
 	_cgo_sys_thread_start(ts);	/* OS-dependent half */
 }
+
+#ifndef CGO_TSAN
+void(* const _cgo_yield)() = NULL;
+#else
+
+#include <string.h>
+
+/*
+Stub for allowing libc interceptors to execute.
+
+_cgo_yield is set to NULL if we do not expect libc interceptors to exist.
+*/
+static void
+x_cgo_yield()
+{
+	/*
+	The libc function(s) we call here must form a no-op and include at least one
+	call that triggers TSAN to process pending asynchronous signals.
+
+	sleep(0) would be fine, but it's not portable C (so it would need more header
+	guards).
+	free(NULL) has a fast-path special case in TSAN, so it doesn't
+	trigger signal delivery.
+	free(malloc(0)) would work (triggering the interceptors in malloc), but
+	it also runs a bunch of user-supplied malloc hooks.
+
+	So we choose strncpy(_, _, 0): it requires an extra header,
+	but it's standard and should be very efficient.
+	*/
+	char nothing = 0;
+	strncpy(&nothing, &nothing, 0);
+}
+
+void(* const _cgo_yield)() = &x_cgo_yield;
+
+#endif  /* GO_TSAN */

src/runtime/lock_futex.go

@@ -140,9 +140,17 @@ func notesleep(n *note) {
 	if gp != gp.m.g0 {
 		throw("notesleep not on g0")
 	}
+	ns := int64(-1)
+	if _cgo_yield != nil {
+		// Sleep for an arbitrary-but-moderate interval to poll libc interceptors.
+		ns = 10e6
+	}
 	for atomic.Load(key32(&n.key)) == 0 {
 		gp.m.blocked = true
-		futexsleep(key32(&n.key), 0, -1)
+		futexsleep(key32(&n.key), 0, ns)
+		if _cgo_yield != nil {
+			asmcgocall(_cgo_yield, nil)
+		}
 		gp.m.blocked = false
 	}
 }
@@ -156,9 +164,16 @@ func notetsleep_internal(n *note, ns int64) bool {
 	gp := getg()
 
 	if ns < 0 {
+		if _cgo_yield != nil {
+			// Sleep for an arbitrary-but-moderate interval to poll libc interceptors.
+			ns = 10e6
+		}
 		for atomic.Load(key32(&n.key)) == 0 {
 			gp.m.blocked = true
-			futexsleep(key32(&n.key), 0, -1)
+			futexsleep(key32(&n.key), 0, ns)
+			if _cgo_yield != nil {
+				asmcgocall(_cgo_yield, nil)
+			}
 			gp.m.blocked = false
 		}
 		return true

src/runtime/lock_sema.go

@@ -163,7 +163,16 @@ func notesleep(n *note) {
 	}
 	// Queued. Sleep.
 	gp.m.blocked = true
-	semasleep(-1)
+	if _cgo_yield == nil {
+		semasleep(-1)
+	} else {
+		// Sleep for an arbitrary-but-moderate interval to poll libc interceptors.
+		const ns = 10e6
+		for atomic.Loaduintptr(&n.key) == 0 {
+			semasleep(ns)
+			asmcgocall(_cgo_yield, nil)
+		}
+	}
 	gp.m.blocked = false
 }
 
@@ -186,7 +195,16 @@ func notetsleep_internal(n *note, ns int64, gp *g, deadline int64) bool {
 	if ns < 0 {
 		// Queued. Sleep.
 		gp.m.blocked = true
-		semasleep(-1)
+		if _cgo_yield == nil {
+			semasleep(-1)
+		} else {
+			// Sleep for an arbitrary-but-moderate interval to poll libc interceptors.
+			const ns = 10e6
+			for atomic.Loaduintptr(&n.key) == 0 {
+				semasleep(ns)
+				asmcgocall(_cgo_yield, nil)
+			}
+		}
 		gp.m.blocked = false
 		return true
 	}

src/runtime/proc.go

@@ -1899,6 +1899,9 @@ top:
 			ready(gp, 0, true)
 		}
 	}
+	if _cgo_yield != nil {
+		asmcgocall(_cgo_yield, nil)
+	}
 
 	// local runq
 	if gp, inheritTime := runqget(_p_); gp != nil {