From 86ff185365dbc40b64f907be9e2f35d756776e20 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Mon, 12 Feb 2018 18:50:16 +0000 Subject: re PR go/84215 (Random results in go/libgo tests) PR go/84215 runtime, sync/atomic: use write barrier for atomic pointer functions This copies atomic_pointer.go from 1.10rc2. It was omitted during the transition of the runtime from C to Go, and I forgot about it. This may help with https://gcc.gnu.org/PR84215. Reviewed-on: https://go-review.googlesource.com/93197 From-SVN: r257599 --- libgo/go/runtime/atomic_pointer.go | 69 ++++++++++++++++++++++++++++++++++++++ libgo/go/runtime/stubs.go | 16 --------- libgo/go/sync/atomic/atomic.c | 34 ------------------- 3 files changed, 69 insertions(+), 50 deletions(-) create mode 100644 libgo/go/runtime/atomic_pointer.go (limited to 'libgo/go') diff --git a/libgo/go/runtime/atomic_pointer.go b/libgo/go/runtime/atomic_pointer.go new file mode 100644 index 0000000..b66ef58 --- /dev/null +++ b/libgo/go/runtime/atomic_pointer.go @@ -0,0 +1,69 @@ +// 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 + +import ( + "runtime/internal/atomic" + "unsafe" +) + +// These functions cannot have go:noescape annotations, +// because while ptr does not escape, new does. +// If new is marked as not escaping, the compiler will make incorrect +// escape analysis decisions about the pointer value being stored. +// Instead, these are wrappers around the actual atomics (casp1 and so on) +// that use noescape to convey which arguments do not escape. + +// atomicstorep performs *ptr = new atomically and invokes a write barrier. +// +//go:nosplit +func atomicstorep(ptr unsafe.Pointer, new unsafe.Pointer) { + writebarrierptr_prewrite((*uintptr)(ptr), uintptr(new)) + atomic.StorepNoWB(noescape(ptr), new) +} + +//go:nosplit +func casp(ptr *unsafe.Pointer, old, new unsafe.Pointer) bool { + // The write barrier is only necessary if the CAS succeeds, + // but since it needs to happen before the write becomes + // public, we have to do it conservatively all the time. + writebarrierptr_prewrite((*uintptr)(unsafe.Pointer(ptr)), uintptr(new)) + return atomic.Casp1((*unsafe.Pointer)(noescape(unsafe.Pointer(ptr))), noescape(old), new) +} + +// Like above, but implement in terms of sync/atomic's uintptr operations. +// We cannot just call the runtime routines, because the race detector expects +// to be able to intercept the sync/atomic forms but not the runtime forms. + +//go:linkname sync_atomic_StoreUintptr sync_atomic.StoreUintptr +func sync_atomic_StoreUintptr(ptr *uintptr, new uintptr) + +//go:linkname sync_atomic_StorePointer sync_atomic.StorePointer +//go:nosplit +func sync_atomic_StorePointer(ptr *unsafe.Pointer, new unsafe.Pointer) { + writebarrierptr_prewrite((*uintptr)(unsafe.Pointer(ptr)), uintptr(new)) + sync_atomic_StoreUintptr((*uintptr)(unsafe.Pointer(ptr)), uintptr(new)) +} + +//go:linkname sync_atomic_SwapUintptr sync_atomic.SwapUintptr +func sync_atomic_SwapUintptr(ptr *uintptr, new uintptr) uintptr + +//go:linkname sync_atomic_SwapPointer sync_atomic.SwapPointer +//go:nosplit +func sync_atomic_SwapPointer(ptr *unsafe.Pointer, new unsafe.Pointer) unsafe.Pointer { + writebarrierptr_prewrite((*uintptr)(unsafe.Pointer(ptr)), uintptr(new)) + old := unsafe.Pointer(sync_atomic_SwapUintptr((*uintptr)(noescape(unsafe.Pointer(ptr))), uintptr(new))) + return old +} + +//go:linkname sync_atomic_CompareAndSwapUintptr sync_atomic.CompareAndSwapUintptr +func sync_atomic_CompareAndSwapUintptr(ptr *uintptr, old, new uintptr) bool + +//go:linkname sync_atomic_CompareAndSwapPointer sync_atomic.CompareAndSwapPointer +//go:nosplit +func sync_atomic_CompareAndSwapPointer(ptr *unsafe.Pointer, old, new unsafe.Pointer) bool { + writebarrierptr_prewrite((*uintptr)(unsafe.Pointer(ptr)), uintptr(new)) + return sync_atomic_CompareAndSwapUintptr((*uintptr)(noescape(unsafe.Pointer(ptr))), uintptr(old), uintptr(new)) +} diff --git a/libgo/go/runtime/stubs.go b/libgo/go/runtime/stubs.go index fa3b1ce..efb0373 100644 --- a/libgo/go/runtime/stubs.go +++ b/libgo/go/runtime/stubs.go @@ -5,7 +5,6 @@ package runtime import ( - "runtime/internal/atomic" "runtime/internal/sys" "unsafe" ) @@ -307,15 +306,6 @@ func setSupportAES(v bool) { support_aes = v } -// Here for gccgo until we port atomic_pointer.go and mgc.go. -//go:nosplit -func casp(ptr *unsafe.Pointer, old, new unsafe.Pointer) bool { - if !atomic.Casp1((*unsafe.Pointer)(noescape(unsafe.Pointer(ptr))), noescape(old), new) { - return false - } - return true -} - // Here for gccgo until we port lock_*.go. func lock(l *mutex) func unlock(l *mutex) @@ -347,12 +337,6 @@ func persistentalloc(size, align uintptr, sysStat *uint64) unsafe.Pointer // Temporary for gccgo until we port mheap.go func setprofilebucket(p unsafe.Pointer, b *bucket) -// Temporary for gccgo until we port atomic_pointer.go. -//go:nosplit -func atomicstorep(ptr unsafe.Pointer, new unsafe.Pointer) { - atomic.StorepNoWB(noescape(ptr), new) -} - // Get signal trampoline, written in C. func getSigtramp() uintptr diff --git a/libgo/go/sync/atomic/atomic.c b/libgo/go/sync/atomic/atomic.c index 32cbf03..07a4306 100644 --- a/libgo/go/sync/atomic/atomic.c +++ b/libgo/go/sync/atomic/atomic.c @@ -62,16 +62,6 @@ SwapUintptr (uintptr_t *addr, uintptr_t new) return __atomic_exchange_n (addr, new, __ATOMIC_SEQ_CST); } -void *SwapPointer (void **, void *) - __asm__ (GOSYM_PREFIX "sync_atomic.SwapPointer") - __attribute__ ((no_split_stack)); - -void * -SwapPointer (void **addr, void *new) -{ - return __atomic_exchange_n (addr, new, __ATOMIC_SEQ_CST); -} - _Bool CompareAndSwapInt32 (int32_t *, int32_t, int32_t) __asm__ (GOSYM_PREFIX "sync_atomic.CompareAndSwapInt32") __attribute__ ((no_split_stack)); @@ -126,16 +116,6 @@ CompareAndSwapUintptr (uintptr_t *val, uintptr_t old, uintptr_t new) return __sync_bool_compare_and_swap (val, old, new); } -_Bool CompareAndSwapPointer (void **, void *, void *) - __asm__ (GOSYM_PREFIX "sync_atomic.CompareAndSwapPointer") - __attribute__ ((no_split_stack)); - -_Bool -CompareAndSwapPointer (void **val, void *old, void *new) -{ - return __sync_bool_compare_and_swap (val, old, new); -} - int32_t AddInt32 (int32_t *, int32_t) __asm__ (GOSYM_PREFIX "sync_atomic.AddInt32") __attribute__ ((no_split_stack)); @@ -357,17 +337,3 @@ StoreUintptr (uintptr_t *addr, uintptr_t val) while (! __sync_bool_compare_and_swap (addr, v, val)) v = *addr; } - -void StorePointer (void **addr, void *val) - __asm__ (GOSYM_PREFIX "sync_atomic.StorePointer") - __attribute__ ((no_split_stack)); - -void -StorePointer (void **addr, void *val) -{ - void *v; - - v = *addr; - while (! __sync_bool_compare_and_swap (addr, v, val)) - v = *addr; -} -- cgit v1.1 From 52eb4ab4092228369cea5d7ca2717d32cc666c5c Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Mon, 12 Feb 2018 19:29:52 +0000 Subject: compiler: error on func declaration/definition Long long long ago Go permitted writing func F() in one file and writing func F() {} in another file. This was removed from the language, and that is now considered to be a multiple definition error. Gccgo never caught up to that, and it has been permitting this invalid code for some time. Stop permitting it, so that we give correct errors. Since we've supported it for a long time, the compiler uses it in a couple of cases: it predeclares the hash/equal methods if it decides to create them while compiling another function, and it predeclares main.main as a mechanism for getting the right warning if a program uses the wrong signature for main. For simplicity, keep those existing uses. This required a few minor changes in libgo which were relying, unnecessarily, on the current behavior. Reviewed-on: https://go-review.googlesource.com/93083 From-SVN: r257600 --- libgo/go/runtime/extern.go | 4 ---- libgo/go/runtime/stubs.go | 17 ----------------- 2 files changed, 21 deletions(-) (limited to 'libgo/go') diff --git a/libgo/go/runtime/extern.go b/libgo/go/runtime/extern.go index 36787e3..b3afd10 100644 --- a/libgo/go/runtime/extern.go +++ b/libgo/go/runtime/extern.go @@ -157,10 +157,6 @@ package runtime import "runtime/internal/sys" -// Gosched yields the processor, allowing other goroutines to run. It does not -// suspend the current goroutine, so execution resumes automatically. -func Gosched() - // Caller reports file and line number information about function invocations on // the calling goroutine's stack. The argument skip is the number of stack frames // to ascend, with 0 identifying the caller of Caller. (For historical reasons the diff --git a/libgo/go/runtime/stubs.go b/libgo/go/runtime/stubs.go index efb0373..bda2c69 100644 --- a/libgo/go/runtime/stubs.go +++ b/libgo/go/runtime/stubs.go @@ -306,10 +306,6 @@ func setSupportAES(v bool) { support_aes = v } -// Here for gccgo until we port lock_*.go. -func lock(l *mutex) -func unlock(l *mutex) - // Here for gccgo. func errno() int @@ -317,9 +313,6 @@ func errno() int func entersyscall(int32) func entersyscallblock(int32) -// Here for gccgo until we port mgc.go. -func GC() - // For gccgo to call from C code, so that the C code and the Go code // can share the memstats variable for now. //go:linkname getMstats runtime.getMstats @@ -327,16 +320,6 @@ func getMstats() *mstats { return &memstats } -// Temporary for gccgo until we port mem_GOOS.go. -func sysAlloc(n uintptr, sysStat *uint64) unsafe.Pointer -func sysFree(v unsafe.Pointer, n uintptr, sysStat *uint64) - -// Temporary for gccgo until we port malloc.go -func persistentalloc(size, align uintptr, sysStat *uint64) unsafe.Pointer - -// Temporary for gccgo until we port mheap.go -func setprofilebucket(p unsafe.Pointer, b *bucket) - // Get signal trampoline, written in C. func getSigtramp() uintptr -- cgit v1.1