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

diff --git a/src/pkg/runtime/386/asm.s b/src/pkg/runtime/386/asm.s
index 2505e4df6a9f42ce4f13bcb4a705b2001333fe81..a14518839acd47e3bd73848c96125880c0d49dd2 100644 (file)
--- 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 4723018a7aa9fb0c19786857d8863ee241e2bb92..3e3818c1014156bf607de2b79f15d80de6216c25 100644 (file)
--- 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 3199afe6227873b5a0fc92bd15c5f732bb0d1d9f..52e4059ae210fb36bd56784ec24b650c7c97d5a8 100644 (file)
--- 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 eee346844bf0b08d3dc4987bf1576ea6fefc8887..44511da8307eeb404c600170b0ed595baba12beb 100644 (file)
--- 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 63e6d87849f21ba64f4859303c68f210da897fee..d2ebf9b400ffc067e78d4aeac3c0fc25cb2ad438 100644 (file)
--- 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 (file)
index 0000000..bd9fe18
--- /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)
+               }
+       }
+}