runtime: check alignment of 8-byte atomic loads and stores on 386

Currently, if we do an atomic{load,store}64 of an unaligned address on
386, we'll simply get a non-atomic load/store.  This has been the
source of myriad bugs, so add alignment checks to these two
operations.  These checks parallel the equivalent checks in
sync/atomic.

The alignment check is not necessary in cas64 because it uses a locked
instruction.  The CPU will either execute this atomically or raise an
alignment fault (#AC)---depending on the alignment check flag---either
of which is fine.

This also fixes the two places in the runtime that trip the new
checks.  One is in the runtime self-test and shouldn't have caused
real problems.  The other is in tickspersecond and could, in
principle, have caused a misread of the ticks per second during
initialization.

Change-Id: If1796667012a6154f64f5e71d043c7f5fb3dd050
Reviewed-on: https://go-review.googlesource.com/3521
Reviewed-by: Russ Cox <rsc@golang.org>

diff --git a/src/runtime/asm_386.s b/src/runtime/asm_386.s
index 0a58faf19b23fe405c5661fb9e0f05c070849f74..49bba32ebe7ccede3a25af969113885f91f33393 100644 (file)
--- a/src/runtime/asm_386.s
+++ b/src/runtime/asm_386.s
@@ -555,6 +555,9 @@ TEXT runtime·atomicstore(SB), NOSPLIT, $0-8
 // uint64 atomicload64(uint64 volatile* addr);
 TEXT runtime·atomicload64(SB), NOSPLIT, $0-12
        MOVL    ptr+0(FP), AX
+       TESTL   $7, AX
+       JZ      2(PC)
+       MOVL    0, AX // crash with nil ptr deref
        LEAL    ret_lo+4(FP), BX
        // MOVQ (%EAX), %MM0
        BYTE $0x0f; BYTE $0x6f; BYTE $0x00
@@ -567,6 +570,9 @@ TEXT runtime·atomicload64(SB), NOSPLIT, $0-12
 // void runtime·atomicstore64(uint64 volatile* addr, uint64 v);
 TEXT runtime·atomicstore64(SB), NOSPLIT, $0-12
        MOVL    ptr+0(FP), AX
+       TESTL   $7, AX
+       JZ      2(PC)
+       MOVL    0, AX // crash with nil ptr deref
        // MOVQ and EMMS were introduced on the Pentium MMX.
        // MOVQ 0x8(%ESP), %MM0
        BYTE $0x0f; BYTE $0x6f; BYTE $0x44; BYTE $0x24; BYTE $0x08

diff --git a/src/runtime/runtime.go b/src/runtime/runtime.go
index 2ce4618f3fd831e2ee166647fa09ffbbb17c22e3..ba9881fd91dec8d470b7f5406686135fdb25e2f7 100644 (file)
--- a/src/runtime/runtime.go
+++ b/src/runtime/runtime.go
@@ -10,6 +10,7 @@ import _ "unsafe" // for go:linkname
 
 var ticks struct {
        lock mutex
+       pad  uint32 // ensure 8-byte alignment of val on 386
        val  uint64
 }
 

diff --git a/src/runtime/runtime1.go b/src/runtime/runtime1.go
index 6056a8dd7ee100529fbde8badbff5f9e9ef55eb5..5dcc83d2e5f42b682bfba596ec4648c0491df21c 100644 (file)
--- a/src/runtime/runtime1.go
+++ b/src/runtime/runtime1.go
@@ -95,45 +95,47 @@ func environ() []string {
        return envs
 }
 
+// TODO: These should be locals in testAtomic64, but we don't 8-byte
+// align stack variables on 386.
+var test_z64, test_x64 uint64
+
 func testAtomic64() {
-       var z64, x64 uint64
-
-       z64 = 42
-       x64 = 0
-       prefetcht0(uintptr(unsafe.Pointer(&z64)))
-       prefetcht1(uintptr(unsafe.Pointer(&z64)))
-       prefetcht2(uintptr(unsafe.Pointer(&z64)))
-       prefetchnta(uintptr(unsafe.Pointer(&z64)))
-       if cas64(&z64, x64, 1) {
+       test_z64 = 42
+       test_x64 = 0
+       prefetcht0(uintptr(unsafe.Pointer(&test_z64)))
+       prefetcht1(uintptr(unsafe.Pointer(&test_z64)))
+       prefetcht2(uintptr(unsafe.Pointer(&test_z64)))
+       prefetchnta(uintptr(unsafe.Pointer(&test_z64)))
+       if cas64(&test_z64, test_x64, 1) {
                throw("cas64 failed")
        }
-       if x64 != 0 {
+       if test_x64 != 0 {
                throw("cas64 failed")
        }
-       x64 = 42
-       if !cas64(&z64, x64, 1) {
+       test_x64 = 42
+       if !cas64(&test_z64, test_x64, 1) {
                throw("cas64 failed")
        }
-       if x64 != 42 || z64 != 1 {
+       if test_x64 != 42 || test_z64 != 1 {
                throw("cas64 failed")
        }
-       if atomicload64(&z64) != 1 {
+       if atomicload64(&test_z64) != 1 {
                throw("load64 failed")
        }
-       atomicstore64(&z64, (1<<40)+1)
-       if atomicload64(&z64) != (1<<40)+1 {
+       atomicstore64(&test_z64, (1<<40)+1)
+       if atomicload64(&test_z64) != (1<<40)+1 {
                throw("store64 failed")
        }
-       if xadd64(&z64, (1<<40)+1) != (2<<40)+2 {
+       if xadd64(&test_z64, (1<<40)+1) != (2<<40)+2 {
                throw("xadd64 failed")
        }
-       if atomicload64(&z64) != (2<<40)+2 {
+       if atomicload64(&test_z64) != (2<<40)+2 {
                throw("xadd64 failed")
        }
-       if xchg64(&z64, (3<<40)+3) != (2<<40)+2 {
+       if xchg64(&test_z64, (3<<40)+3) != (2<<40)+2 {
                throw("xchg64 failed")
        }
-       if atomicload64(&z64) != (3<<40)+3 {
+       if atomicload64(&test_z64) != (3<<40)+3 {
                throw("xchg64 failed")
        }
 }