From: Dmitry Vyukov Date: Wed, 29 Jun 2016 09:09:36 +0000 (+0200) Subject: runtime: fix race atomic operations on external memory X-Git-Tag: go1.7rc1~27 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=bb337372fb6e171a6e8a7665ce91eda734f8cdd2;p=gostls13.git runtime: fix race atomic operations on external memory The assembly is broken: it does `MOVQ g(R12), R14` expecting that R12 contains tls address, but it does not do get_tls(R12) before. This magically works on linux: `MOVQ g(R12), R14` is compiled to `mov %fs:0xfffffffffffffff8,%r14` which does not use R12. But it crashes on windows. Add explicit `get_tls(R12)`. Fixes #16206 Change-Id: Ic1f21a6fef2473bcf9147de6646929781c9c1e98 Reviewed-on: https://go-review.googlesource.com/24590 Reviewed-by: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Gobot Gobot --- diff --git a/src/runtime/race/race_linux_test.go b/src/runtime/race/race_linux_test.go new file mode 100644 index 0000000000..c00ce4d3df --- /dev/null +++ b/src/runtime/race/race_linux_test.go @@ -0,0 +1,37 @@ +// Copyright 2016 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. + +// +build linux,race + +package race_test + +import ( + "sync/atomic" + "syscall" + "testing" + "unsafe" +) + +func TestAtomicMmap(t *testing.T) { + // Test that atomic operations work on "external" memory. Previously they crashed (#16206). + // Also do a sanity correctness check: under race detector atomic operations + // are implemented inside of race runtime. + mem, err := syscall.Mmap(-1, 0, 1<<20, syscall.PROT_READ|syscall.PROT_WRITE, syscall.MAP_ANON|syscall.MAP_PRIVATE) + if err != nil { + t.Fatalf("mmap failed: %v", err) + } + defer syscall.Munmap(mem) + a := (*uint64)(unsafe.Pointer(&mem[0])) + if *a != 0 { + t.Fatalf("bad atomic value: %v, want 0", *a) + } + atomic.AddUint64(a, 1) + if *a != 1 { + t.Fatalf("bad atomic value: %v, want 1", *a) + } + atomic.AddUint64(a, 1) + if *a != 2 { + t.Fatalf("bad atomic value: %v, want 2", *a) + } +} diff --git a/src/runtime/race/race_windows_test.go b/src/runtime/race/race_windows_test.go new file mode 100644 index 0000000000..307a1ea6c0 --- /dev/null +++ b/src/runtime/race/race_windows_test.go @@ -0,0 +1,46 @@ +// Copyright 2016 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. + +// +build windows,race + +package race_test + +import ( + "sync/atomic" + "syscall" + "testing" + "unsafe" +) + +func TestAtomicMmap(t *testing.T) { + // Test that atomic operations work on "external" memory. Previously they crashed (#16206). + // Also do a sanity correctness check: under race detector atomic operations + // are implemented inside of race runtime. + kernel32 := syscall.NewLazyDLL("kernel32.dll") + VirtualAlloc := kernel32.NewProc("VirtualAlloc") + VirtualFree := kernel32.NewProc("VirtualFree") + const ( + MEM_COMMIT = 0x00001000 + MEM_RESERVE = 0x00002000 + MEM_RELEASE = 0x8000 + PAGE_READWRITE = 0x04 + ) + mem, _, err := syscall.Syscall6(VirtualAlloc.Addr(), 4, 0, 1<<20, MEM_COMMIT|MEM_RESERVE, PAGE_READWRITE, 0, 0) + if err != 0 { + t.Fatalf("VirtualAlloc failed: %v", err) + } + defer syscall.Syscall(VirtualFree.Addr(), 3, mem, 1<<20, MEM_RELEASE) + a := (*uint64)(unsafe.Pointer(mem)) + if *a != 0 { + t.Fatalf("bad atomic value: %v, want 0", *a) + } + atomic.AddUint64(a, 1) + if *a != 1 { + t.Fatalf("bad atomic value: %v, want 1", *a) + } + atomic.AddUint64(a, 1) + if *a != 2 { + t.Fatalf("bad atomic value: %v, want 2", *a) + } +} diff --git a/src/runtime/race_amd64.s b/src/runtime/race_amd64.s index 94ca76da27..cc1d92f437 100644 --- a/src/runtime/race_amd64.s +++ b/src/runtime/race_amd64.s @@ -338,6 +338,7 @@ racecallatomic_ignore: // An attempt to synchronize on the address would cause crash. MOVQ AX, R15 // remember the original function MOVQ $__tsan_go_ignore_sync_begin(SB), AX + get_tls(R12) MOVQ g(R12), R14 MOVQ g_racectx(R14), RARG0 // goroutine context CALL racecall<>(SB)