From c8ef0df06cbab367ab943825ab0677e03d019c3d Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Thu, 3 Dec 2015 19:17:21 -0800 Subject: [PATCH] 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 --- misc/cgo/testsanitizers/test.bash | 105 ++++++++++++++++++++---------- misc/cgo/testsanitizers/tsan.go | 44 +++++++++++++ misc/cgo/testsanitizers/tsan2.go | 55 ++++++++++++++++ src/cmd/cgo/out.go | 60 ++++++++++++++++- 4 files changed, 228 insertions(+), 36 deletions(-) create mode 100644 misc/cgo/testsanitizers/tsan.go create mode 100644 misc/cgo/testsanitizers/tsan2.go diff --git a/misc/cgo/testsanitizers/test.bash b/misc/cgo/testsanitizers/test.bash index feacd89c27..69caa39b15 100755 --- 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 index 0000000000..9e0951c914 --- /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 index 0000000000..78234a0799 --- /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 ca0ec0aaa2..aafe6a8be5 100644 --- 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 ` +// 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 /* 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{}) ` -- 2.50.0