From 91f0f18100564478f77c6fc8e16ea56f9528951c Mon Sep 17 00:00:00 2001 From: Dmitriy Vyukov Date: Fri, 29 Jul 2011 13:47:24 -0400 Subject: [PATCH] runtime: fix data race in findfunc() The data race can lead to reads of partially initialized concurrently mutated symbol data. The change also adds a simple sanity test for Caller() and FuncForPC(). R=rsc CC=golang-dev https://golang.org/cl/4817058 --- src/pkg/runtime/386/asm.s | 6 +++++ src/pkg/runtime/amd64/asm.s | 6 +++++ src/pkg/runtime/arm/atomic.c | 13 ++++++++++ src/pkg/runtime/runtime.h | 1 + src/pkg/runtime/symtab.c | 7 +++-- src/pkg/runtime/symtab_test.go | 47 ++++++++++++++++++++++++++++++++++ 6 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 src/pkg/runtime/symtab_test.go diff --git a/src/pkg/runtime/386/asm.s b/src/pkg/runtime/386/asm.s index 2505e4df6a9..a14518839ac 100644 --- a/src/pkg/runtime/386/asm.s +++ b/src/pkg/runtime/386/asm.s @@ -354,6 +354,12 @@ TEXT runtime·atomicstorep(SB), 7, $0 XCHGL AX, 0(BX) RET +TEXT runtime·atomicstore(SB), 7, $0 + MOVL 4(SP), BX + MOVL 8(SP), AX + XCHGL AX, 0(BX) + RET + // void jmpdefer(fn, sp); // called from deferreturn. // 1. pop the caller diff --git a/src/pkg/runtime/amd64/asm.s b/src/pkg/runtime/amd64/asm.s index 4723018a7aa..3e3818c1014 100644 --- a/src/pkg/runtime/amd64/asm.s +++ b/src/pkg/runtime/amd64/asm.s @@ -398,6 +398,12 @@ TEXT runtime·atomicstorep(SB), 7, $0 XCHGQ AX, 0(BX) RET +TEXT runtime·atomicstore(SB), 7, $0 + MOVQ 8(SP), BX + MOVL 16(SP), AX + XCHGL AX, 0(BX) + RET + // void jmpdefer(fn, sp); // called from deferreturn. // 1. pop the caller diff --git a/src/pkg/runtime/arm/atomic.c b/src/pkg/runtime/arm/atomic.c index 3199afe6227..52e4059ae21 100644 --- a/src/pkg/runtime/arm/atomic.c +++ b/src/pkg/runtime/arm/atomic.c @@ -68,3 +68,16 @@ runtime·atomicstorep(void* volatile* addr, void* v) return; } } + +#pragma textflag 7 +void +runtime·atomicstore(uint32 volatile* addr, uint32 v) +{ + uint32 old; + + for(;;) { + old = *addr; + if(runtime·cas(addr, old, v)) + return; + } +} \ No newline at end of file diff --git a/src/pkg/runtime/runtime.h b/src/pkg/runtime/runtime.h index eee346844bf..44511da8307 100644 --- a/src/pkg/runtime/runtime.h +++ b/src/pkg/runtime/runtime.h @@ -433,6 +433,7 @@ bool runtime·casp(void**, void*, void*); uint32 runtime·xadd(uint32 volatile*, int32); uint32 runtime·xchg(uint32 volatile*, uint32); uint32 runtime·atomicload(uint32 volatile*); +void runtime·atomicstore(uint32 volatile*, uint32); void* runtime·atomicloadp(void* volatile*); void runtime·atomicstorep(void* volatile*, void*); void runtime·jmpdefer(byte*, void*); diff --git a/src/pkg/runtime/symtab.c b/src/pkg/runtime/symtab.c index 63e6d87849f..d2ebf9b400f 100644 --- a/src/pkg/runtime/symtab.c +++ b/src/pkg/runtime/symtab.c @@ -78,6 +78,7 @@ static int32 nfunc; static byte **fname; static int32 nfname; +static uint32 funcinit; static Lock funclock; static void @@ -427,10 +428,12 @@ runtime·findfunc(uintptr addr) // (Before enabling the signal handler, // SetCPUProfileRate calls findfunc to trigger // the initialization outside the handler.) - if(runtime·atomicloadp(&func) == nil) { + if(runtime·atomicload(&funcinit) == 0) { runtime·lock(&funclock); - if(func == nil) + if(funcinit == 0) { buildfuncs(); + runtime·atomicstore(&funcinit, 1); + } runtime·unlock(&funclock); } diff --git a/src/pkg/runtime/symtab_test.go b/src/pkg/runtime/symtab_test.go new file mode 100644 index 00000000000..bd9fe18c478 --- /dev/null +++ b/src/pkg/runtime/symtab_test.go @@ -0,0 +1,47 @@ +// Copyright 2009 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 runtime_test + +import ( + "runtime" + "strings" + "testing" +) + +func TestCaller(t *testing.T) { + procs := runtime.GOMAXPROCS(-1) + c := make(chan bool, procs) + for p := 0; p < procs; p++ { + go func() { + for i := 0; i < 1000; i++ { + testCallerFoo(t) + } + c <- true + }() + defer func() { + <-c + }() + } +} + +func testCallerFoo(t *testing.T) { + testCallerBar(t) +} + +func testCallerBar(t *testing.T) { + for i := 0; i < 2; i++ { + pc, file, line, ok := runtime.Caller(i) + f := runtime.FuncForPC(pc) + if !ok || + !strings.HasSuffix(file, "symtab_test.go") || + (i == 0 && !strings.HasSuffix(f.Name(), "testCallerBar")) || + (i == 1 && !strings.HasSuffix(f.Name(), "testCallerFoo")) || + line < 5 || line > 1000 || + f.Entry() >= pc { + t.Errorf("incorrect symbol info %d: %t %d %d %s %s %d", + i, ok, f.Entry(), pc, f.Name(), file, line) + } + } +}