cmd/cgo: add hooks for thread sanitizer

When Go code is used with C code compiled with -fsanitize=thread, adds
thread sanitizer calls so that correctly synchronized Go code does not
cause spurious failure reports from the thread sanitizer.  This may
cause some false negatives, but for the thread sanitizer what is most
important is avoiding false positives.

Change-Id: If670e4a6f2874c7a2be2ff7db8728c6036340a52
Reviewed-on: https://go-review.googlesource.com/17421
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>

diff --git a/misc/cgo/testsanitizers/test.bash b/misc/cgo/testsanitizers/test.bash
index feacd89c279b6e4af80321a436f3f594d076f3b2..69caa39b15157ad27bc88c4d25aa5f6f5988779e 100755 (executable)
--- a/misc/cgo/testsanitizers/test.bash
+++ b/misc/cgo/testsanitizers/test.bash
@@ -15,35 +15,36 @@ if test -x "$(type -p clang)"; then
 fi
 export CC
 
+msan=yes
+
 TMPDIR=${TMPDIR:-/tmp}
 echo > ${TMPDIR}/testsanitizers$$.c
 if $CC -fsanitize=memory -c ${TMPDIR}/testsanitizers$$.c -o ${TMPDIR}/testsanitizers$$.o 2>&1 | grep "unrecognized" >& /dev/null; then
-  echo "skipping msan test: -fsanitize=memory not supported"
-  rm -f ${TMPDIR}/testsanitizers$$.*
-  exit 0
+  echo "skipping msan tests: -fsanitize=memory not supported"
+  msan=no
 fi
 rm -f ${TMPDIR}/testsanitizers$$.*
 
 # The memory sanitizer in versions of clang before 3.6 don't work with Go.
-if $CC --version | grep clang >& /dev/null; then
+if test "$msan" = "yes" && $CC --version | grep clang >& /dev/null; then
   ver=$($CC --version | sed -e 's/.* version \([0-9.-]*\).*/\1/')
   major=$(echo $ver | sed -e 's/\([0-9]*\).*/\1/')
   minor=$(echo $ver | sed -e 's/[0-9]*\.\([0-9]*\).*/\1/')
   if test "$major" -lt 3 || test "$major" -eq 3 -a "$minor" -lt 6; then
-    echo "skipping msan test; clang version $major.$minor (older than 3.6)"
-    exit 0
+    echo "skipping msan tests: clang version $major.$minor (older than 3.6)"
+    msan=no
   fi
 
   # Clang before 3.8 does not work with Linux at or after 4.1.
   # golang.org/issue/12898.
-  if test "$major" -lt 3 || test "$major" -eq 3 -a "$minor" -lt 8; then
+  if test "$msan" = "yes" -a "$major" -lt 3 || test "$major" -eq 3 -a "$minor" -lt 8; then
     if test "$(uname)" = Linux; then
       linuxver=$(uname -r)
       linuxmajor=$(echo $linuxver | sed -e 's/\([0-9]*\).*/\1/')
       linuxminor=$(echo $linuxver | sed -e 's/[0-9]*\.\([0-9]*\).*/\1/')
       if test "$linuxmajor" -gt 4 || test "$linuxmajor" -eq 4 -a "$linuxminor" -ge 1; then
-        echo "skipping msan test; clang version $major.$minor (older than 3.8) incompatible with linux version $linuxmajor.$linuxminor (4.1 or newer)"
-        exit 0
+        echo "skipping msan tests: clang version $major.$minor (older than 3.8) incompatible with linux version $linuxmajor.$linuxminor (4.1 or newer)"
+       msan=no
       fi
     fi
   fi
@@ -51,39 +52,75 @@ fi
 
 status=0
 
-if ! go build -msan std; then
-  echo "FAIL: build -msan std"
-  status=1
-fi
+if test "$msan" = "yes"; then
+    if ! go build -msan std; then
+       echo "FAIL: build -msan std"
+       status=1
+    fi
 
-if ! go run -msan msan.go; then
-  echo "FAIL: msan"
-  status=1
-fi
+    if ! go run -msan msan.go; then
+       echo "FAIL: msan"
+       status=1
+    fi
 
-if ! CGO_LDFLAGS="-fsanitize=memory" CGO_CPPFLAGS="-fsanitize=memory" go run -msan -a msan2.go; then
-  echo "FAIL: msan2 with -fsanitize=memory"
-  status=1
-fi
+    if ! CGO_LDFLAGS="-fsanitize=memory" CGO_CPPFLAGS="-fsanitize=memory" go run -msan -a msan2.go; then
+       echo "FAIL: msan2 with -fsanitize=memory"
+       status=1
+    fi
 
-if ! go run -msan -a msan2.go; then
-  echo "FAIL: msan2"
-  status=1
-fi
+    if ! go run -msan -a msan2.go; then
+       echo "FAIL: msan2"
+       status=1
+    fi
 
-if ! go run -msan msan3.go; then
-  echo "FAIL: msan3"
-  status=1
+    if ! go run -msan msan3.go; then
+       echo "FAIL: msan3"
+       status=1
+    fi
+
+    if ! go run -msan msan4.go; then
+       echo "FAIL: msan4"
+       status=1
+    fi
+
+    if go run -msan msan_fail.go 2>/dev/null; then
+       echo "FAIL: msan_fail"
+       status=1
+    fi
 fi
 
-if ! go run -msan msan4.go; then
-  echo "FAIL: msan4"
-  status=1
+tsan=yes
+
+TMPDIR=${TMPDIR:-/tmp}
+echo > ${TMPDIR}/testsanitizers$$.c
+if $CC -fsanitize=thread -c ${TMPDIR}/testsanitizers$$.c -o ${TMPDIR}/testsanitizers$$.o 2>&1 | grep "unrecognized" >& /dev/null; then
+  echo "skipping tsan tests: -fsanitize=thread not supported"
+  tsan=no
 fi
+rm -f ${TMPDIR}/testsanitizers$$.*
+
+if test "$tsan" = "yes"; then
+    err=${TMPDIR}/tsanerr$$.out
+
+    if ! go run tsan.go 2>$err; then
+       echo "FAIL: tsan"
+       status=1
+    elif grep -i warning $err >/dev/null 2>&1; then
+       cat $err
+       echo "FAIL: tsan"
+       status=1
+    fi
+
+    if ! go run tsan2.go 2>$err; then
+       echo "FAIL: tsan2"
+       status=1
+    elif grep -i warning $err >/dev/null 2>&1; then
+       cat $err
+       echo "FAIL: tsan2"
+       status=1
+    fi
 
-if go run -msan msan_fail.go 2>/dev/null; then
-  echo "FAIL: msan_fail"
-  status=1
+    rm -f $err
 fi
 
 exit $status

diff --git a/misc/cgo/testsanitizers/tsan.go b/misc/cgo/testsanitizers/tsan.go
new file mode 100644 (file)
index 0000000..9e0951c
--- /dev/null
+++ b/misc/cgo/testsanitizers/tsan.go
@@ -0,0 +1,44 @@
+// Copyright 2015 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 produced false race reports when run under the C/C++
+// ThreadSanitizer, as it did not understand the synchronization in
+// the Go code.
+
+/*
+#cgo CFLAGS: -fsanitize=thread
+#cgo LDFLAGS: -fsanitize=thread
+
+int val;
+
+int getVal() {
+       return val;
+}
+
+void setVal(int i) {
+       val = i;
+}
+*/
+import "C"
+
+import (
+       "runtime"
+)
+
+func main() {
+       runtime.LockOSThread()
+       C.setVal(1)
+       c := make(chan bool)
+       go func() {
+               runtime.LockOSThread()
+               C.setVal(2)
+               c <- true
+       }()
+       <-c
+       if v := C.getVal(); v != 2 {
+               panic(v)
+       }
+}

diff --git a/misc/cgo/testsanitizers/tsan2.go b/misc/cgo/testsanitizers/tsan2.go
new file mode 100644 (file)
index 0000000..78234a0
--- /dev/null
+++ b/misc/cgo/testsanitizers/tsan2.go
@@ -0,0 +1,55 @@
+// Copyright 2015 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 produced false race reports when run under the C/C++
+// ThreadSanitizer, as it did not understand the synchronization in
+// the Go code.
+
+/*
+#cgo CFLAGS: -fsanitize=thread
+#cgo LDFLAGS: -fsanitize=thread
+
+extern void GoRun(void);
+
+// Yes, you can have definitions if you use //export, as long as they are weak.
+
+int val __attribute__ ((weak));
+
+int run(void) __attribute__ ((weak));
+
+int run() {
+       val = 1;
+       GoRun();
+       return val;
+}
+
+void setVal(int) __attribute__ ((weak));
+
+void setVal(int i) {
+       val = i;
+}
+*/
+import "C"
+
+import "runtime"
+
+//export GoRun
+func GoRun() {
+       runtime.LockOSThread()
+       c := make(chan bool)
+       go func() {
+               runtime.LockOSThread()
+               C.setVal(2)
+               c <- true
+       }()
+       <-c
+}
+
+func main() {
+       if v := C.run(); v != 2 {
+               panic(v)
+       }
+}

diff --git a/src/cmd/cgo/out.go b/src/cmd/cgo/out.go
index ca0ec0aaa28e99023bedcbbdbd931aa423b4d616..aafe6a8be574f9719314926b30854c63d29873e3 100644 (file)
--- a/src/cmd/cgo/out.go
+++ b/src/cmd/cgo/out.go
@@ -507,6 +507,7 @@ func (p *Package) writeOutput(f *File, srcfile string) {
        // Gcc output starts with the preamble.
        fmt.Fprintf(fgcc, "%s\n", f.Preamble)
        fmt.Fprintf(fgcc, "%s\n", gccProlog)
+       fmt.Fprintf(fgcc, "%s\n", tsanProlog)
 
        for _, key := range nameKeys(f.Name) {
                n := f.Name[key]
@@ -573,6 +574,7 @@ func (p *Package) writeOutputFunc(fgcc *os.File, n *Name) {
                // Save the stack top for use below.
                fmt.Fprintf(fgcc, "\tchar *stktop = _cgo_topofstack();\n")
        }
+       fmt.Fprintf(fgcc, "\t_cgo_tsan_acquire();\n")
        fmt.Fprintf(fgcc, "\t")
        if t := n.FuncType.Result; t != nil {
                fmt.Fprintf(fgcc, "__typeof__(a->r) r = ")
@@ -598,6 +600,7 @@ func (p *Package) writeOutputFunc(fgcc *os.File, n *Name) {
                fmt.Fprintf(fgcc, "a->p%d", i)
        }
        fmt.Fprintf(fgcc, ");\n")
+       fmt.Fprintf(fgcc, "\t_cgo_tsan_release();\n")
        if n.FuncType.Result != nil {
                // The cgo call may have caused a stack copy (via a callback).
                // Adjust the return value pointer appropriately.
@@ -636,9 +639,13 @@ func (p *Package) writeGccgoOutputFunc(fgcc *os.File, n *Name) {
        }
        fmt.Fprintf(fgcc, ")\n")
        fmt.Fprintf(fgcc, "{\n")
+       if t := n.FuncType.Result; t != nil {
+               fmt.Fprintf(fgcc, "\t%s r;\n", t.C.String())
+       }
+       fmt.Fprintf(fgcc, "\t_cgo_tsan_acquire();\n")
        fmt.Fprintf(fgcc, "\t")
        if t := n.FuncType.Result; t != nil {
-               fmt.Fprintf(fgcc, "return ")
+               fmt.Fprintf(fgcc, "r = ")
                // Cast to void* to avoid warnings due to omitted qualifiers.
                if c := t.C.String(); c[len(c)-1] == '*' {
                        fmt.Fprintf(fgcc, "(void*)")
@@ -656,6 +663,16 @@ func (p *Package) writeGccgoOutputFunc(fgcc *os.File, n *Name) {
                fmt.Fprintf(fgcc, "p%d", i)
        }
        fmt.Fprintf(fgcc, ");\n")
+       fmt.Fprintf(fgcc, "\t_cgo_tsan_release();\n")
+       if t := n.FuncType.Result; t != nil {
+               fmt.Fprintf(fgcc, "\treturn ")
+               // Cast to void* to avoid warnings due to omitted qualifiers
+               // and explicit incompatible struct types.
+               if c := t.C.String(); c[len(c)-1] == '*' {
+                       fmt.Fprintf(fgcc, "(void*)")
+               }
+               fmt.Fprintf(fgcc, "r;\n")
+       }
        fmt.Fprintf(fgcc, "}\n")
        fmt.Fprintf(fgcc, "\n")
 }
@@ -683,6 +700,7 @@ func (p *Package) writeExports(fgo2, fm, fgcc, fgcch io.Writer) {
 
        fmt.Fprintf(fgcc, "extern void crosscall2(void (*fn)(void *, int), void *, int);\n")
        fmt.Fprintf(fgcc, "extern void _cgo_wait_runtime_init_done();\n\n")
+       fmt.Fprintf(fgcc, "%s\n", tsanProlog)
 
        for _, exp := range p.ExpFunc {
                fn := exp.Func
@@ -798,7 +816,9 @@ func (p *Package) writeExports(fgo2, fm, fgcc, fgcch io.Writer) {
                        func(i int, aname string, atype ast.Expr) {
                                fmt.Fprintf(fgcc, "\ta.p%d = p%d;\n", i, i)
                        })
+               fmt.Fprintf(fgcc, "\t_cgo_tsan_release();\n")
                fmt.Fprintf(fgcc, "\tcrosscall2(_cgoexp%s_%s, &a, %d);\n", cPrefix, exp.ExpName, off)
+               fmt.Fprintf(fgcc, "\t_cgo_tsan_acquire();\n")
                if gccResult != "void" {
                        if len(fntype.Results.List) == 1 && len(fntype.Results.List[0].Names) <= 1 {
                                fmt.Fprintf(fgcc, "\treturn a.r0;\n")
@@ -915,6 +935,7 @@ func (p *Package) writeGccgoExports(fgo2, fm, fgcc, fgcch io.Writer) {
        fmt.Fprintf(fgcc, "#include \"_cgo_export.h\"\n")
 
        fmt.Fprintf(fgcc, "%s\n", gccgoExportFileProlog)
+       fmt.Fprintf(fgcc, "%s\n", tsanProlog)
 
        for _, exp := range p.ExpFunc {
                fn := exp.Func
@@ -985,11 +1006,15 @@ func (p *Package) writeGccgoExports(fgo2, fm, fgcc, fgcch io.Writer) {
 
                fmt.Fprint(fgcc, "\n")
                fmt.Fprintf(fgcc, "%s %s %s {\n", cRet, exp.ExpName, cParams)
+               if resultCount > 0 {
+                       fmt.Fprintf(fgcc, "\t%s r;\n", cRet)
+               }
                fmt.Fprintf(fgcc, "\tif(_cgo_wait_runtime_init_done)\n")
                fmt.Fprintf(fgcc, "\t\t_cgo_wait_runtime_init_done();\n")
+               fmt.Fprintf(fgcc, "\t_cgo_tsan_release();\n")
                fmt.Fprint(fgcc, "\t")
                if resultCount > 0 {
-                       fmt.Fprint(fgcc, "return ")
+                       fmt.Fprint(fgcc, "r = ")
                }
                fmt.Fprintf(fgcc, "%s(", goName)
                if fn.Recv != nil {
@@ -1003,6 +1028,10 @@ func (p *Package) writeGccgoExports(fgo2, fm, fgcc, fgcch io.Writer) {
                                fmt.Fprintf(fgcc, "p%d", i)
                        })
                fmt.Fprint(fgcc, ");\n")
+               fmt.Fprintf(fgcc, "\t_cgo_tsan_acquire();\n")
+               if resultCount > 0 {
+                       fmt.Fprint(fgcc, "\treturn r;\n")
+               }
                fmt.Fprint(fgcc, "}\n")
 
                // Dummy declaration for _cgo_main.c
@@ -1257,6 +1286,31 @@ extern char* _cgo_topofstack(void);
 #include <string.h>
 `
 
+// Prologue defining TSAN functions in C.
+const tsanProlog = `
+#define _cgo_tsan_acquire()
+#define _cgo_tsan_release()
+#if defined(__has_feature)
+#if __has_feature(thread_sanitizer)
+#undef _cgo_tsan_acquire
+#undef _cgo_tsan_release
+
+long long _cgo_sync __attribute__ ((common));
+
+extern void __tsan_acquire(void*);
+extern void __tsan_release(void*);
+
+static void _cgo_tsan_acquire() {
+       __tsan_acquire(&_cgo_sync);
+}
+
+static void _cgo_tsan_release() {
+       __tsan_release(&_cgo_sync);
+}
+#endif
+#endif
+`
+
 const builtinProlog = `
 #include <stddef.h> /* for ptrdiff_t and size_t below */
 
@@ -1290,8 +1344,10 @@ func _cgoCheckResult(interface{})
 `
 
 const gccgoGoProlog = `
+//extern runtime.cgoCheckPointer
 func _cgoCheckPointer(interface{}, ...interface{}) interface{}
 
+//extern runtime.cgoCheckResult
 func _cgoCheckResult(interface{})
 `