From e30ce3c498b623f5a492a8e77c32077c1ecf3a1f Mon Sep 17 00:00:00 2001 From: Jorropo Date: Wed, 18 Jan 2023 18:03:30 +0100 Subject: [PATCH] sync/atomic: make intrinsics noescape except 64bits op on 32bits arch and unsafe.Pointer Fixes #16241 I made 64 bits op on 32 bits arches still leak since it was kinda promised. The promised leaks were wider than this but I don't belive it's effect can be observed in an breaking maner without using unsafe the way it's currently setup. Change-Id: I66d8df47bfe49bce3efa64ac668a2a55f70733a3 Reviewed-on: https://go-review.googlesource.com/c/go/+/462298 Reviewed-by: Mauri de Souza Meneguzzo Reviewed-by: Keith Randall LUCI-TryBot-Result: Go LUCI Reviewed-by: David Chase Reviewed-by: Dmitri Shuralyov --- src/sync/atomic/doc.go | 119 ++++++++++++-------------------- src/sync/atomic/doc_32.go | 79 +++++++++++++++++++++ src/sync/atomic/doc_64.go | 107 ++++++++++++++++++++++++++++ test/fixedbugs/issue16241.go | 59 ++++++++++++++++ test/fixedbugs/issue16241_64.go | 46 ++++++++++++ 5 files changed, 336 insertions(+), 74 deletions(-) create mode 100644 src/sync/atomic/doc_32.go create mode 100644 src/sync/atomic/doc_64.go create mode 100644 test/fixedbugs/issue16241.go create mode 100644 test/fixedbugs/issue16241_64.go diff --git a/src/sync/atomic/doc.go b/src/sync/atomic/doc.go index 7f9d64b74e..4e93404757 100644 --- a/src/sync/atomic/doc.go +++ b/src/sync/atomic/doc.go @@ -60,29 +60,26 @@ import ( // for 64-bit alignment of 64-bit words accessed atomically via the primitive // atomic functions (types [Int64] and [Uint64] are automatically aligned). // The first word in an allocated struct, array, or slice; in a global -// variable; or in a local variable (because the subject of all atomic operations -// will escape to the heap) can be relied upon to be 64-bit aligned. +// variable; or in a local variable (because on 32-bit architectures, the +// subject of 64-bit atomic operations will escape to the heap) can be +// relied upon to be 64-bit aligned. // SwapInt32 atomically stores new into *addr and returns the previous *addr value. // Consider using the more ergonomic and less error-prone [Int32.Swap] instead. +// +//go:noescape func SwapInt32(addr *int32, new int32) (old int32) -// SwapInt64 atomically stores new into *addr and returns the previous *addr value. -// Consider using the more ergonomic and less error-prone [Int64.Swap] instead -// (particularly if you target 32-bit platforms; see the bugs section). -func SwapInt64(addr *int64, new int64) (old int64) - // SwapUint32 atomically stores new into *addr and returns the previous *addr value. // Consider using the more ergonomic and less error-prone [Uint32.Swap] instead. +// +//go:noescape func SwapUint32(addr *uint32, new uint32) (old uint32) -// SwapUint64 atomically stores new into *addr and returns the previous *addr value. -// Consider using the more ergonomic and less error-prone [Uint64.Swap] instead -// (particularly if you target 32-bit platforms; see the bugs section). -func SwapUint64(addr *uint64, new uint64) (old uint64) - // SwapUintptr atomically stores new into *addr and returns the previous *addr value. // Consider using the more ergonomic and less error-prone [Uintptr.Swap] instead. +// +//go:noescape func SwapUintptr(addr *uintptr, new uintptr) (old uintptr) // SwapPointer atomically stores new into *addr and returns the previous *addr value. @@ -91,24 +88,20 @@ func SwapPointer(addr *unsafe.Pointer, new unsafe.Pointer) (old unsafe.Pointer) // CompareAndSwapInt32 executes the compare-and-swap operation for an int32 value. // Consider using the more ergonomic and less error-prone [Int32.CompareAndSwap] instead. +// +//go:noescape func CompareAndSwapInt32(addr *int32, old, new int32) (swapped bool) -// CompareAndSwapInt64 executes the compare-and-swap operation for an int64 value. -// Consider using the more ergonomic and less error-prone [Int64.CompareAndSwap] instead -// (particularly if you target 32-bit platforms; see the bugs section). -func CompareAndSwapInt64(addr *int64, old, new int64) (swapped bool) - // CompareAndSwapUint32 executes the compare-and-swap operation for a uint32 value. // Consider using the more ergonomic and less error-prone [Uint32.CompareAndSwap] instead. +// +//go:noescape func CompareAndSwapUint32(addr *uint32, old, new uint32) (swapped bool) -// CompareAndSwapUint64 executes the compare-and-swap operation for a uint64 value. -// Consider using the more ergonomic and less error-prone [Uint64.CompareAndSwap] instead -// (particularly if you target 32-bit platforms; see the bugs section). -func CompareAndSwapUint64(addr *uint64, old, new uint64) (swapped bool) - // CompareAndSwapUintptr executes the compare-and-swap operation for a uintptr value. // Consider using the more ergonomic and less error-prone [Uintptr.CompareAndSwap] instead. +// +//go:noescape func CompareAndSwapUintptr(addr *uintptr, old, new uintptr) (swapped bool) // CompareAndSwapPointer executes the compare-and-swap operation for a unsafe.Pointer value. @@ -117,100 +110,82 @@ func CompareAndSwapPointer(addr *unsafe.Pointer, old, new unsafe.Pointer) (swapp // AddInt32 atomically adds delta to *addr and returns the new value. // Consider using the more ergonomic and less error-prone [Int32.Add] instead. +// +//go:noescape func AddInt32(addr *int32, delta int32) (new int32) // AddUint32 atomically adds delta to *addr and returns the new value. // To subtract a signed positive constant value c from x, do AddUint32(&x, ^uint32(c-1)). // In particular, to decrement x, do AddUint32(&x, ^uint32(0)). // Consider using the more ergonomic and less error-prone [Uint32.Add] instead. +// +//go:noescape func AddUint32(addr *uint32, delta uint32) (new uint32) -// AddInt64 atomically adds delta to *addr and returns the new value. -// Consider using the more ergonomic and less error-prone [Int64.Add] instead -// (particularly if you target 32-bit platforms; see the bugs section). -func AddInt64(addr *int64, delta int64) (new int64) - -// AddUint64 atomically adds delta to *addr and returns the new value. -// To subtract a signed positive constant value c from x, do AddUint64(&x, ^uint64(c-1)). -// In particular, to decrement x, do AddUint64(&x, ^uint64(0)). -// Consider using the more ergonomic and less error-prone [Uint64.Add] instead -// (particularly if you target 32-bit platforms; see the bugs section). -func AddUint64(addr *uint64, delta uint64) (new uint64) - // AddUintptr atomically adds delta to *addr and returns the new value. // Consider using the more ergonomic and less error-prone [Uintptr.Add] instead. +// +//go:noescape func AddUintptr(addr *uintptr, delta uintptr) (new uintptr) // AndInt32 atomically performs a bitwise AND operation on *addr using the bitmask provided as mask // and returns the old value. // Consider using the more ergonomic and less error-prone [Int32.And] instead. +// +//go:noescape func AndInt32(addr *int32, mask int32) (old int32) // AndUint32 atomically performs a bitwise AND operation on *addr using the bitmask provided as mask // and returns the old value. // Consider using the more ergonomic and less error-prone [Uint32.And] instead. +// +//go:noescape func AndUint32(addr *uint32, mask uint32) (old uint32) -// AndInt64 atomically performs a bitwise AND operation on *addr using the bitmask provided as mask -// and returns the old value. -// Consider using the more ergonomic and less error-prone [Int64.And] instead. -func AndInt64(addr *int64, mask int64) (old int64) - -// AndUint64 atomically performs a bitwise AND operation on *addr using the bitmask provided as mask -// and returns the old. -// Consider using the more ergonomic and less error-prone [Uint64.And] instead. -func AndUint64(addr *uint64, mask uint64) (old uint64) - // AndUintptr atomically performs a bitwise AND operation on *addr using the bitmask provided as mask // and returns the old value. // Consider using the more ergonomic and less error-prone [Uintptr.And] instead. +// +//go:noescape func AndUintptr(addr *uintptr, mask uintptr) (old uintptr) // OrInt32 atomically performs a bitwise OR operation on *addr using the bitmask provided as mask // and returns the old value. // Consider using the more ergonomic and less error-prone [Int32.Or] instead. +// +//go:noescape func OrInt32(addr *int32, mask int32) (old int32) // OrUint32 atomically performs a bitwise OR operation on *addr using the bitmask provided as mask // and returns the old value. // Consider using the more ergonomic and less error-prone [Uint32.Or] instead. +// +//go:noescape func OrUint32(addr *uint32, mask uint32) (old uint32) -// OrInt64 atomically performs a bitwise OR operation on *addr using the bitmask provided as mask -// and returns the old value. -// Consider using the more ergonomic and less error-prone [Int64.Or] instead. -func OrInt64(addr *int64, mask int64) (old int64) - -// OrUint64 atomically performs a bitwise OR operation on *addr using the bitmask provided as mask -// and returns the old value. -// Consider using the more ergonomic and less error-prone [Uint64.Or] instead. -func OrUint64(addr *uint64, mask uint64) (old uint64) - // OrUintptr atomically performs a bitwise OR operation on *addr using the bitmask provided as mask // and returns the old value. // Consider using the more ergonomic and less error-prone [Uintptr.Or] instead. +// +//go:noescape func OrUintptr(addr *uintptr, mask uintptr) (old uintptr) // LoadInt32 atomically loads *addr. // Consider using the more ergonomic and less error-prone [Int32.Load] instead. +// +//go:noescape func LoadInt32(addr *int32) (val int32) -// LoadInt64 atomically loads *addr. -// Consider using the more ergonomic and less error-prone [Int64.Load] instead -// (particularly if you target 32-bit platforms; see the bugs section). -func LoadInt64(addr *int64) (val int64) - // LoadUint32 atomically loads *addr. // Consider using the more ergonomic and less error-prone [Uint32.Load] instead. +// +//go:noescape func LoadUint32(addr *uint32) (val uint32) -// LoadUint64 atomically loads *addr. -// Consider using the more ergonomic and less error-prone [Uint64.Load] instead -// (particularly if you target 32-bit platforms; see the bugs section). -func LoadUint64(addr *uint64) (val uint64) - // LoadUintptr atomically loads *addr. // Consider using the more ergonomic and less error-prone [Uintptr.Load] instead. +// +//go:noescape func LoadUintptr(addr *uintptr) (val uintptr) // LoadPointer atomically loads *addr. @@ -219,24 +194,20 @@ func LoadPointer(addr *unsafe.Pointer) (val unsafe.Pointer) // StoreInt32 atomically stores val into *addr. // Consider using the more ergonomic and less error-prone [Int32.Store] instead. +// +//go:noescape func StoreInt32(addr *int32, val int32) -// StoreInt64 atomically stores val into *addr. -// Consider using the more ergonomic and less error-prone [Int64.Store] instead -// (particularly if you target 32-bit platforms; see the bugs section). -func StoreInt64(addr *int64, val int64) - // StoreUint32 atomically stores val into *addr. // Consider using the more ergonomic and less error-prone [Uint32.Store] instead. +// +//go:noescape func StoreUint32(addr *uint32, val uint32) -// StoreUint64 atomically stores val into *addr. -// Consider using the more ergonomic and less error-prone [Uint64.Store] instead -// (particularly if you target 32-bit platforms; see the bugs section). -func StoreUint64(addr *uint64, val uint64) - // StoreUintptr atomically stores val into *addr. // Consider using the more ergonomic and less error-prone [Uintptr.Store] instead. +// +//go:noescape func StoreUintptr(addr *uintptr, val uintptr) // StorePointer atomically stores val into *addr. diff --git a/src/sync/atomic/doc_32.go b/src/sync/atomic/doc_32.go new file mode 100644 index 0000000000..9d644f25ec --- /dev/null +++ b/src/sync/atomic/doc_32.go @@ -0,0 +1,79 @@ +// Copyright 2023 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. + +//go:build 386 || arm || mips || mipsle + +package atomic + +// SwapInt64 atomically stores new into *addr and returns the previous *addr value. +// Consider using the more ergonomic and less error-prone [Int64.Swap] instead +// (particularly if you target 32-bit platforms; see the bugs section). +func SwapInt64(addr *int64, new int64) (old int64) + +// SwapUint64 atomically stores new into *addr and returns the previous *addr value. +// Consider using the more ergonomic and less error-prone [Uint64.Swap] instead +// (particularly if you target 32-bit platforms; see the bugs section). +func SwapUint64(addr *uint64, new uint64) (old uint64) + +// CompareAndSwapInt64 executes the compare-and-swap operation for an int64 value. +// Consider using the more ergonomic and less error-prone [Int64.CompareAndSwap] instead +// (particularly if you target 32-bit platforms; see the bugs section). +func CompareAndSwapInt64(addr *int64, old, new int64) (swapped bool) + +// CompareAndSwapUint64 executes the compare-and-swap operation for a uint64 value. +// Consider using the more ergonomic and less error-prone [Uint64.CompareAndSwap] instead +// (particularly if you target 32-bit platforms; see the bugs section). +func CompareAndSwapUint64(addr *uint64, old, new uint64) (swapped bool) + +// AddInt64 atomically adds delta to *addr and returns the new value. +// Consider using the more ergonomic and less error-prone [Int64.Add] instead +// (particularly if you target 32-bit platforms; see the bugs section). +func AddInt64(addr *int64, delta int64) (new int64) + +// AddUint64 atomically adds delta to *addr and returns the new value. +// To subtract a signed positive constant value c from x, do AddUint64(&x, ^uint64(c-1)). +// In particular, to decrement x, do AddUint64(&x, ^uint64(0)). +// Consider using the more ergonomic and less error-prone [Uint64.Add] instead +// (particularly if you target 32-bit platforms; see the bugs section). +func AddUint64(addr *uint64, delta uint64) (new uint64) + +// AndInt64 atomically performs a bitwise AND operation on *addr using the bitmask provided as mask +// and returns the old value. +// Consider using the more ergonomic and less error-prone [Int64.And] instead. +func AndInt64(addr *int64, mask int64) (old int64) + +// AndUint64 atomically performs a bitwise AND operation on *addr using the bitmask provided as mask +// and returns the old. +// Consider using the more ergonomic and less error-prone [Uint64.And] instead. +func AndUint64(addr *uint64, mask uint64) (old uint64) + +// OrInt64 atomically performs a bitwise OR operation on *addr using the bitmask provided as mask +// and returns the old value. +// Consider using the more ergonomic and less error-prone [Int64.Or] instead. +func OrInt64(addr *int64, mask int64) (old int64) + +// OrUint64 atomically performs a bitwise OR operation on *addr using the bitmask provided as mask +// and returns the old value. +// Consider using the more ergonomic and less error-prone [Uint64.Or] instead. +func OrUint64(addr *uint64, mask uint64) (old uint64) + +// LoadInt64 atomically loads *addr. +// Consider using the more ergonomic and less error-prone [Int64.Load] instead +// (particularly if you target 32-bit platforms; see the bugs section). +func LoadInt64(addr *int64) (val int64) + +// LoadUint64 atomically loads *addr. +// Consider using the more ergonomic and less error-prone [Uint64.Load] instead +// (particularly if you target 32-bit platforms; see the bugs section). +func LoadUint64(addr *uint64) (val uint64) + +// StoreInt64 atomically stores val into *addr. +// Consider using the more ergonomic and less error-prone [Int64.Store] instead +// (particularly if you target 32-bit platforms; see the bugs section). +func StoreInt64(addr *int64, val int64) + +// StoreUint64 atomically stores val into *addr. +// Consider using the more ergonomic and less error-prone [Uint64.Store] instead +// (particularly if you target 32-bit platforms; see the bugs section). +func StoreUint64(addr *uint64, val uint64) diff --git a/src/sync/atomic/doc_64.go b/src/sync/atomic/doc_64.go new file mode 100644 index 0000000000..5fec3f4e8f --- /dev/null +++ b/src/sync/atomic/doc_64.go @@ -0,0 +1,107 @@ +// Copyright 2023 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. + +//go:build !(386 || arm || mips || mipsle) + +package atomic + +// SwapInt64 atomically stores new into *addr and returns the previous *addr value. +// Consider using the more ergonomic and less error-prone [Int64.Swap] instead +// (particularly if you target 32-bit platforms; see the bugs section). +// +//go:noescape +func SwapInt64(addr *int64, new int64) (old int64) + +// SwapUint64 atomically stores new into *addr and returns the previous *addr value. +// Consider using the more ergonomic and less error-prone [Uint64.Swap] instead +// (particularly if you target 32-bit platforms; see the bugs section). +// +//go:noescape +func SwapUint64(addr *uint64, new uint64) (old uint64) + +// CompareAndSwapInt64 executes the compare-and-swap operation for an int64 value. +// Consider using the more ergonomic and less error-prone [Int64.CompareAndSwap] instead +// (particularly if you target 32-bit platforms; see the bugs section). +// +//go:noescape +func CompareAndSwapInt64(addr *int64, old, new int64) (swapped bool) + +// CompareAndSwapUint64 executes the compare-and-swap operation for a uint64 value. +// Consider using the more ergonomic and less error-prone [Uint64.CompareAndSwap] instead +// (particularly if you target 32-bit platforms; see the bugs section). +// +//go:noescape +func CompareAndSwapUint64(addr *uint64, old, new uint64) (swapped bool) + +// AddInt64 atomically adds delta to *addr and returns the new value. +// Consider using the more ergonomic and less error-prone [Int64.Add] instead +// (particularly if you target 32-bit platforms; see the bugs section). +// +//go:noescape +func AddInt64(addr *int64, delta int64) (new int64) + +// AddUint64 atomically adds delta to *addr and returns the new value. +// To subtract a signed positive constant value c from x, do AddUint64(&x, ^uint64(c-1)). +// In particular, to decrement x, do AddUint64(&x, ^uint64(0)). +// Consider using the more ergonomic and less error-prone [Uint64.Add] instead +// (particularly if you target 32-bit platforms; see the bugs section). +// +//go:noescape +func AddUint64(addr *uint64, delta uint64) (new uint64) + +// AndInt64 atomically performs a bitwise AND operation on *addr using the bitmask provided as mask +// and returns the old value. +// Consider using the more ergonomic and less error-prone [Int64.And] instead. +// +//go:noescape +func AndInt64(addr *int64, mask int64) (old int64) + +// AndUint64 atomically performs a bitwise AND operation on *addr using the bitmask provided as mask +// and returns the old. +// Consider using the more ergonomic and less error-prone [Uint64.And] instead. +// +//go:noescape +func AndUint64(addr *uint64, mask uint64) (old uint64) + +// OrInt64 atomically performs a bitwise OR operation on *addr using the bitmask provided as mask +// and returns the old value. +// Consider using the more ergonomic and less error-prone [Int64.Or] instead. +// +//go:noescape +func OrInt64(addr *int64, mask int64) (old int64) + +// OrUint64 atomically performs a bitwise OR operation on *addr using the bitmask provided as mask +// and returns the old value. +// Consider using the more ergonomic and less error-prone [Uint64.Or] instead. +// +//go:noescape +func OrUint64(addr *uint64, mask uint64) (old uint64) + +// LoadInt64 atomically loads *addr. +// Consider using the more ergonomic and less error-prone [Int64.Load] instead +// (particularly if you target 32-bit platforms; see the bugs section). +// +//go:noescape +func LoadInt64(addr *int64) (val int64) + +// LoadUint64 atomically loads *addr. +// Consider using the more ergonomic and less error-prone [Uint64.Load] instead +// (particularly if you target 32-bit platforms; see the bugs section). +// +//go:noescape +func LoadUint64(addr *uint64) (val uint64) + +// StoreInt64 atomically stores val into *addr. +// Consider using the more ergonomic and less error-prone [Int64.Store] instead +// (particularly if you target 32-bit platforms; see the bugs section). +// +//go:noescape +func StoreInt64(addr *int64, val int64) + +// StoreUint64 atomically stores val into *addr. +// Consider using the more ergonomic and less error-prone [Uint64.Store] instead +// (particularly if you target 32-bit platforms; see the bugs section). +// +//go:noescape +func StoreUint64(addr *uint64, val uint64) diff --git a/test/fixedbugs/issue16241.go b/test/fixedbugs/issue16241.go new file mode 100644 index 0000000000..33f1aa3dee --- /dev/null +++ b/test/fixedbugs/issue16241.go @@ -0,0 +1,59 @@ +// errorcheck -0 -m -l + +// Copyright 2023 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 foo + +import "sync/atomic" + +func AddInt32(x *int32) { // ERROR "x does not escape$" + atomic.AddInt32(x, 42) +} +func AddUint32(x *uint32) { // ERROR "x does not escape$" + atomic.AddUint32(x, 42) +} +func AddUintptr(x *uintptr) { // ERROR "x does not escape$" + atomic.AddUintptr(x, 42) +} + +func CompareAndSwapInt32(x *int32) { // ERROR "x does not escape$" + atomic.CompareAndSwapInt32(x, 42, 42) +} +func CompareAndSwapUint32(x *uint32) { // ERROR "x does not escape$" + atomic.CompareAndSwapUint32(x, 42, 42) +} +func CompareAndSwapUintptr(x *uintptr) { // ERROR "x does not escape$" + atomic.CompareAndSwapUintptr(x, 42, 42) +} + +func LoadInt32(x *int32) { // ERROR "x does not escape$" + atomic.LoadInt32(x) +} +func LoadUint32(x *uint32) { // ERROR "x does not escape$" + atomic.LoadUint32(x) +} +func LoadUintptr(x *uintptr) { // ERROR "x does not escape$" + atomic.LoadUintptr(x) +} + +func StoreInt32(x *int32) { // ERROR "x does not escape$" + atomic.StoreInt32(x, 42) +} +func StoreUint32(x *uint32) { // ERROR "x does not escape$" + atomic.StoreUint32(x, 42) +} +func StoreUintptr(x *uintptr) { // ERROR "x does not escape$" + atomic.StoreUintptr(x, 42) +} + +func SwapInt32(x *int32) { // ERROR "x does not escape$" + atomic.SwapInt32(x, 42) +} +func SwapUint32(x *uint32) { // ERROR "x does not escape$" + atomic.SwapUint32(x, 42) +} +func SwapUintptr(x *uintptr) { // ERROR "x does not escape$" + atomic.SwapUintptr(x, 42) +} diff --git a/test/fixedbugs/issue16241_64.go b/test/fixedbugs/issue16241_64.go new file mode 100644 index 0000000000..82626cb796 --- /dev/null +++ b/test/fixedbugs/issue16241_64.go @@ -0,0 +1,46 @@ +//go:build !(386 || arm || mips || mipsle) + +// errorcheck -0 -m -l + +// Copyright 2023 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 foo + +import "sync/atomic" + +func AddInt64(x *int64) { // ERROR "x does not escape$" + atomic.AddInt64(x, 42) +} +func AddUint64(x *uint64) { // ERROR "x does not escape$" + atomic.AddUint64(x, 42) +} + +func CompareAndSwapInt64(x *int64) { // ERROR "x does not escape$" + atomic.CompareAndSwapInt64(x, 42, 42) +} +func CompareAndSwapUint64(x *uint64) { // ERROR "x does not escape$" + atomic.CompareAndSwapUint64(x, 42, 42) +} + +func LoadInt64(x *int64) { // ERROR "x does not escape$" + atomic.LoadInt64(x) +} +func LoadUint64(x *uint64) { // ERROR "x does not escape$" + atomic.LoadUint64(x) +} + +func StoreInt64(x *int64) { // ERROR "x does not escape$" + atomic.StoreInt64(x, 42) +} +func StoreUint64(x *uint64) { // ERROR "x does not escape$" + atomic.StoreUint64(x, 42) +} + +func SwapInt64(x *int64) { // ERROR "x does not escape$" + atomic.SwapInt64(x, 42) +} +func SwapUint64(x *uint64) { // ERROR "x does not escape$" + atomic.SwapUint64(x, 42) +} -- 2.48.1