From 83317d9e3cb0674f71d1118d8814aefb31ac1239 Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Thu, 15 Oct 2020 15:52:58 -0400 Subject: runtime/internal/atomic: panic nicely on unaligned 64-bit atomics On 386 and arm, unaligned 64-bit atomics aren't safe, so we check for this and panic. Currently, we panic by dereferencing nil, which may be expedient but is pretty user-hostile since it gives no hint of what the actual problem was. This CL replaces this with an actual panic. The only subtlety here is now the atomic assembly implementations are calling back into Go, so they have to play nicely with stack maps and stack scanning. On 386, this just requires declaring NO_LOCAL_POINTERS. On arm, this is somewhat more complicated: first, we have to move the alignment check into the functions that have Go signatures. Then we have to support both the tail call from these functions to the underlying implementation (which requires that they have no frame) and the call into Go to panic (which requires that they have a frame). We resolve this by forcing them to have no frame and setting up the frame manually just before the panic call. Change-Id: I19f1e860045df64088013db37a18acea47342c69 Reviewed-on: https://go-review.googlesource.com/c/go/+/262778 Trust: Austin Clements Run-TryBot: Austin Clements TryBot-Result: Go Bot Reviewed-by: Cherry Zhang Reviewed-by: Michael Knyszek --- src/sync/atomic/atomic_test.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'src/sync/atomic') diff --git a/src/sync/atomic/atomic_test.go b/src/sync/atomic/atomic_test.go index 83e7c8d763..eadc962f70 100644 --- a/src/sync/atomic/atomic_test.go +++ b/src/sync/atomic/atomic_test.go @@ -1397,8 +1397,15 @@ func TestStoreLoadRelAcq64(t *testing.T) { func shouldPanic(t *testing.T, name string, f func()) { defer func() { - if recover() == nil { + // Check that all GC maps are sane. + runtime.GC() + + err := recover() + want := "unaligned 64-bit atomic operation" + if err == nil { t.Errorf("%s did not panic", name) + } else if s, _ := err.(string); s != want { + t.Errorf("%s: wanted panic %q, got %q", name, want, err) } }() f() -- cgit v1.3 From d902791b509b641683d4ec58b282180c56918aec Mon Sep 17 00:00:00 2001 From: Martin Möhrmann Date: Sat, 21 Nov 2020 15:53:18 +0100 Subject: sync: use 386 instead of x86-32 to refer to the 32 bit x86 architecture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This aligns the naming with GOARCH using 386 as a build target for this architecture and makes it more easily found when searching for documentation related to the build target. Change-Id: I393bb89dd2f71e568124107b13e1b288fbd0c76a Reviewed-on: https://go-review.googlesource.com/c/go/+/271988 Trust: Martin Möhrmann Run-TryBot: Martin Möhrmann TryBot-Result: Go Bot Reviewed-by: Ian Lance Taylor --- src/sync/atomic/doc.go | 11 +++++------ src/sync/once.go | 2 +- 2 files changed, 6 insertions(+), 7 deletions(-) (limited to 'src/sync/atomic') diff --git a/src/sync/atomic/doc.go b/src/sync/atomic/doc.go index ff4ad80049..805ef956d5 100644 --- a/src/sync/atomic/doc.go +++ b/src/sync/atomic/doc.go @@ -43,15 +43,14 @@ import ( "unsafe" ) -// BUG(rsc): On x86-32, the 64-bit functions use instructions unavailable before the Pentium MMX. +// BUG(rsc): On 386, the 64-bit functions use instructions unavailable before the Pentium MMX. // // On non-Linux ARM, the 64-bit functions use instructions unavailable before the ARMv6k core. // -// On ARM, x86-32, and 32-bit MIPS, -// it is the caller's responsibility to arrange for 64-bit -// alignment of 64-bit words accessed atomically. The first word in a -// variable or in an allocated struct, array, or slice can be relied upon to be -// 64-bit aligned. +// On ARM, 386, and 32-bit MIPS, it is the caller's responsibility +// to arrange for 64-bit alignment of 64-bit words accessed atomically. +// The first word in a variable or in an allocated struct, array, or slice can +// be relied upon to be 64-bit aligned. // SwapInt32 atomically stores new into *addr and returns the previous *addr value. func SwapInt32(addr *int32, new int32) (old int32) diff --git a/src/sync/once.go b/src/sync/once.go index bf4b80c867..8844314e7e 100644 --- a/src/sync/once.go +++ b/src/sync/once.go @@ -15,7 +15,7 @@ type Once struct { // done indicates whether the action has been performed. // It is first in the struct because it is used in the hot path. // The hot path is inlined at every call site. - // Placing done first allows more compact instructions on some architectures (amd64/x86), + // Placing done first allows more compact instructions on some architectures (amd64/386), // and fewer instructions (to calculate offset) on other architectures. done uint32 m Mutex -- cgit v1.3