From 38765eba739461e5c5dc463860c62daee2eef4ee Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Sat, 24 Sep 2016 17:07:35 +0200 Subject: [PATCH] runtime/race: don't crash on invalid PCs Currently raceSymbolizeCode uses funcline, which is internal runtime function which crashes on incorrect PCs. Use FileLine instead, it is public and does not crash on invalid data. Note: FileLine returns "?" file on failure. That string is not NUL-terminated, so we need to additionally check what FileLine returns. Fixes #17190 Change-Id: Ic6fbd4f0e68ddd52e9b2dd25e625b50adcb69a98 Reviewed-on: https://go-review.googlesource.com/29714 Run-TryBot: Dmitry Vyukov Reviewed-by: Ian Lance Taylor --- src/runtime/race.go | 30 +++++++------- src/runtime/race/output_test.go | 71 ++++++++++++++++++++++++++++++--- 2 files changed, 80 insertions(+), 21 deletions(-) diff --git a/src/runtime/race.go b/src/runtime/race.go index 42da936ddb..6f24e09925 100644 --- a/src/runtime/race.go +++ b/src/runtime/race.go @@ -91,23 +91,23 @@ func racecallback(cmd uintptr, ctx unsafe.Pointer) { } func raceSymbolizeCode(ctx *symbolizeCodeContext) { - f := findfunc(ctx.pc) - if f == nil { - ctx.fn = &qq[0] - ctx.file = &dash[0] - ctx.line = 0 - ctx.off = ctx.pc - ctx.res = 1 - return + f := FuncForPC(ctx.pc) + if f != nil { + file, line := f.FileLine(ctx.pc) + if line != 0 { + ctx.fn = cfuncname(f.raw()) + ctx.line = uintptr(line) + ctx.file = &bytes(file)[0] // assume NUL-terminated + ctx.off = ctx.pc - f.Entry() + ctx.res = 1 + return + } } - - ctx.fn = cfuncname(f) - file, line := funcline(f, ctx.pc) - ctx.line = uintptr(line) - ctx.file = &bytes(file)[0] // assume NUL-terminated - ctx.off = ctx.pc - f.entry + ctx.fn = &qq[0] + ctx.file = &dash[0] + ctx.line = 0 + ctx.off = ctx.pc ctx.res = 1 - return } type symbolizeDataContext struct { diff --git a/src/runtime/race/output_test.go b/src/runtime/race/output_test.go index f1dc4482f1..9158f0453c 100644 --- a/src/runtime/race/output_test.go +++ b/src/runtime/race/output_test.go @@ -13,12 +13,17 @@ import ( "os/exec" "path/filepath" "regexp" + "runtime" "strings" "testing" ) func TestOutput(t *testing.T) { for _, test := range tests { + if test.goos != "" && test.goos != runtime.GOOS { + t.Logf("test %v runs only on %v, skipping: ", test.name, test.goos) + continue + } dir, err := ioutil.TempDir("", "go-build") if err != nil { t.Fatalf("failed to create temp directory: %v", err) @@ -67,11 +72,12 @@ func TestOutput(t *testing.T) { var tests = []struct { name string run string + goos string gorace string source string re string }{ - {"simple", "run", "atexit_sleep_ms=0", ` + {"simple", "run", "", "atexit_sleep_ms=0", ` package main import "time" func main() { @@ -116,7 +122,7 @@ Found 1 data race\(s\) exit status 66 `}, - {"exitcode", "run", "atexit_sleep_ms=0 exitcode=13", ` + {"exitcode", "run", "", "atexit_sleep_ms=0 exitcode=13", ` package main func main() { done := make(chan bool) @@ -130,7 +136,7 @@ func main() { } `, `exit status 13`}, - {"strip_path_prefix", "run", "atexit_sleep_ms=0 strip_path_prefix=/main.", ` + {"strip_path_prefix", "run", "", "atexit_sleep_ms=0 strip_path_prefix=/main.", ` package main func main() { done := make(chan bool) @@ -146,7 +152,7 @@ func main() { go:7 \+0x[0-9,a-f]+ `}, - {"halt_on_error", "run", "atexit_sleep_ms=0 halt_on_error=1", ` + {"halt_on_error", "run", "", "atexit_sleep_ms=0 halt_on_error=1", ` package main func main() { done := make(chan bool) @@ -163,7 +169,7 @@ func main() { exit status 66 `}, - {"test_fails_on_race", "test", "atexit_sleep_ms=0", ` + {"test_fails_on_race", "test", "", "atexit_sleep_ms=0", ` package main_test import "testing" func TestFail(t *testing.T) { @@ -182,7 +188,7 @@ PASS Found 1 data race\(s\) FAIL`}, - {"slicebytetostring_pc", "run", "atexit_sleep_ms=0", ` + {"slicebytetostring_pc", "run", "", "atexit_sleep_ms=0", ` package main func main() { done := make(chan string) @@ -198,4 +204,57 @@ func main() { .*/runtime/string\.go:.* main\.main\.func1\(\) .*/main.go:7`}, + + // Test for http://golang.org/issue/17190 + {"external_cgo_thread", "run", "linux", "atexit_sleep_ms=0", ` +package main + +/* +#include +typedef struct cb { + int foo; +} cb; +extern void goCallback(); +static inline void *threadFunc(void *p) { + goCallback(); + return 0; +} +static inline void startThread(cb* c) { + pthread_t th; + pthread_create(&th, 0, threadFunc, 0); +} +*/ +import "C" + +import "time" + +var racy int + +//export goCallback +func goCallback() { + racy++ +} + +func main() { + var c C.cb + C.startThread(&c) + time.Sleep(time.Second) + racy++ +} +`, `================== +WARNING: DATA RACE +Read at 0x[0-9,a-f]+ by main goroutine: + main\.main\(\) + .*/main\.go:34 \+0x[0-9,a-f]+ + +Previous write at 0x[0-9,a-f]+ by goroutine [0-9]: + main\.goCallback\(\) + .*/main\.go:27 \+0x[0-9,a-f]+ + main._cgoexpwrap_[0-9a-z]+_goCallback\(\) + .*/_cgo_gotypes\.go:[0-9]+ \+0x[0-9,a-f]+ + +Goroutine [0-9] \(running\) created at: + runtime\.newextram\(\) + .*/runtime/proc.go:[0-9]+ \+0x[0-9,a-f]+ +==================`}, }