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 <dvyukov@google.com>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This commit is contained in:
Dmitry Vyukov 2016-09-24 17:07:35 +02:00
parent 9f1c78781b
commit 38765eba73
2 changed files with 80 additions and 21 deletions

View file

@ -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 {

View file

@ -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 <pthread.h>
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]+
==================`},
}