]> Cypherpunks repositories - gostls13.git/commitdiff
[release-branch.go1.5] runtime: memmove/memclr pointers atomically
authorKeith Randall <khr@golang.org>
Thu, 5 Nov 2015 20:39:56 +0000 (12:39 -0800)
committerAustin Clements <austin@google.com>
Fri, 13 Nov 2015 17:26:50 +0000 (17:26 +0000)
Make sure that we're moving or zeroing pointers atomically.
Anything that is a multiple of pointer size and at least
pointer aligned might have pointers in it.  All the code looks
ok except for the 1-pointer-sized moves.

Fixes #13160
Update #12552

Change-Id: Ib97d9b918fa9f4cc5c56c67ed90255b7fdfb7b45
Reviewed-on: https://go-review.googlesource.com/16668
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/16910
Reviewed-by: Russ Cox <rsc@golang.org>
src/runtime/asm_amd64p32.s
src/runtime/memclr_386.s
src/runtime/memclr_amd64.s
src/runtime/memclr_plan9_386.s
src/runtime/memmove_386.s
src/runtime/memmove_amd64.s
src/runtime/memmove_nacl_amd64p32.s
src/runtime/memmove_plan9_386.s
src/runtime/memmove_plan9_amd64.s
test/fixedbugs/issue13160.go [new file with mode: 0644]

index a001a764e779987c0a3ac818e25a6cd9650344b8..4b12f0191d80f0802a0ce7135affb364c3245a46 100644 (file)
@@ -636,6 +636,9 @@ TEXT runtime·memclr(SB),NOSPLIT,$0-8
        MOVQ    BX, CX
        REP
        STOSB
+       // Note: we zero only 4 bytes at a time so that the tail is at most
+       // 3 bytes.  That guarantees that we aren't zeroing pointers with STOSB.
+       // See issue 13160.
        RET
 
 TEXT runtime·getcallerpc(SB),NOSPLIT,$8-12
index 3f20b69c8219b6dc139ded7316173a21102e8de1..ce962f35da15ebce3422c09df5d38853bb48b66f 100644 (file)
@@ -21,7 +21,8 @@ tail:
        CMPL    BX, $2
        JBE     _1or2
        CMPL    BX, $4
-       JBE     _3or4
+       JB      _3
+       JE      _4
        CMPL    BX, $8
        JBE     _5through8
        CMPL    BX, $16
@@ -68,9 +69,13 @@ _1or2:
        RET
 _0:
        RET
-_3or4:
+_3:
        MOVW    AX, (DI)
-       MOVW    AX, -2(DI)(BX*1)
+       MOVB    AX, 2(DI)
+       RET
+_4:
+       // We need a separate case for 4 to make sure we clear pointers atomically.
+       MOVL    AX, (DI)
        RET
 _5through8:
        MOVL    AX, (DI)
index ec24f1db239032e1a52a9e293342f9038e8e0371..3e2c4b241af1fa1869b184e13c29e74b93340986 100644 (file)
@@ -23,7 +23,8 @@ tail:
        CMPQ    BX, $4
        JBE     _3or4
        CMPQ    BX, $8
-       JBE     _5through8
+       JB      _5through7
+       JE      _8
        CMPQ    BX, $16
        JBE     _9through16
        PXOR    X0, X0
@@ -71,10 +72,14 @@ _3or4:
        MOVW    AX, (DI)
        MOVW    AX, -2(DI)(BX*1)
        RET
-_5through8:
+_5through7:
        MOVL    AX, (DI)
        MOVL    AX, -4(DI)(BX*1)
        RET
+_8:
+       // We need a separate case for 8 to make sure we clear pointers atomically.
+       MOVQ    AX, (DI)
+       RET
 _9through16:
        MOVQ    AX, (DI)
        MOVQ    AX, -8(DI)(BX*1)
index 50f327b4ed0201f3456d38666c07483b613b1a93..4707ab2e752b0535b71ff2ab11bc3c76d96414a5 100644 (file)
@@ -16,7 +16,8 @@ tail:
        CMPL    BX, $2
        JBE     _1or2
        CMPL    BX, $4
-       JBE     _3or4
+       JB      _3
+       JE      _4
        CMPL    BX, $8
        JBE     _5through8
        CMPL    BX, $16
@@ -35,9 +36,13 @@ _1or2:
        RET
 _0:
        RET
-_3or4:
+_3:
        MOVW    AX, (DI)
-       MOVW    AX, -2(DI)(BX*1)
+       MOVB    AX, 2(DI)
+       RET
+_4:
+       // We need a separate case for 4 to make sure we clear pointers atomically.
+       MOVL    AX, (DI)
        RET
 _5through8:
        MOVL    AX, (DI)
index 4c0c74c1afa28c078b3caa7bf572245010696f55..f72a73ae4f654cdb7a0bbe11de8310053a3e09af 100644 (file)
@@ -43,7 +43,8 @@ tail:
        CMPL    BX, $2
        JBE     move_1or2
        CMPL    BX, $4
-       JBE     move_3or4
+       JB      move_3
+       JE      move_4
        CMPL    BX, $8
        JBE     move_5through8
        CMPL    BX, $16
@@ -118,11 +119,16 @@ move_1or2:
        RET
 move_0:
        RET
-move_3or4:
+move_3:
        MOVW    (SI), AX
-       MOVW    -2(SI)(BX*1), CX
+       MOVB    2(SI), CX
        MOVW    AX, (DI)
-       MOVW    CX, -2(DI)(BX*1)
+       MOVB    CX, 2(DI)
+       RET
+move_4:
+       // We need a separate case for 4 to make sure we write pointers atomically.
+       MOVL    (SI), AX
+       MOVL    AX, (DI)
        RET
 move_5through8:
        MOVL    (SI), AX
index f9684353400aea2b4218d5ac8a8c4dbc168baa6e..e14614d631473108287080d85f12e2ab132ead3d 100644 (file)
@@ -50,7 +50,8 @@ tail:
        CMPQ    BX, $4
        JBE     move_3or4
        CMPQ    BX, $8
-       JBE     move_5through8
+       JB      move_5through7
+       JE      move_8
        CMPQ    BX, $16
        JBE     move_9through16
        CMPQ    BX, $32
@@ -131,12 +132,17 @@ move_3or4:
        MOVW    AX, (DI)
        MOVW    CX, -2(DI)(BX*1)
        RET
-move_5through8:
+move_5through7:
        MOVL    (SI), AX
        MOVL    -4(SI)(BX*1), CX
        MOVL    AX, (DI)
        MOVL    CX, -4(DI)(BX*1)
        RET
+move_8:
+       // We need a separate case for 8 to make sure we write pointers atomically.
+       MOVQ    (SI), AX
+       MOVQ    AX, (DI)
+       RET
 move_9through16:
        MOVQ    (SI), AX
        MOVQ    -8(SI)(BX*1), CX
index be9e1e55bec0d6cf7222c30652a0a9e4fcacd863..dd7ac764ffea46f29604c4812b1923217c0499a7 100644 (file)
@@ -46,4 +46,7 @@ back:
        REP; MOVSB
        CLD
 
+       // Note: we copy only 4 bytes at a time so that the tail is at most
+       // 3 bytes.  That guarantees that we aren't copying pointers with MOVSB.
+       // See issue 13160.
        RET
index 025d4ce1bf4e98bb6ebb4638c7e58dfdff0bffd5..3b492eb6cda61687386cdd82a8c42c7388bfa7e4 100644 (file)
@@ -39,7 +39,8 @@ tail:
        CMPL    BX, $2
        JBE     move_1or2
        CMPL    BX, $4
-       JBE     move_3or4
+       JB      move_3
+       JE      move_4
        CMPL    BX, $8
        JBE     move_5through8
        CMPL    BX, $16
@@ -104,11 +105,16 @@ move_1or2:
        RET
 move_0:
        RET
-move_3or4:
+move_3:
        MOVW    (SI), AX
-       MOVW    -2(SI)(BX*1), CX
+       MOVB    2(SI), CX
        MOVW    AX, (DI)
-       MOVW    CX, -2(DI)(BX*1)
+       MOVB    CX, 2(DI)
+       RET
+move_4:
+       // We need a separate case for 4 to make sure we write pointers atomically.
+       MOVL    (SI), AX
+       MOVL    AX, (DI)
        RET
 move_5through8:
        MOVL    (SI), AX
index 8e96b87175db2116eabcebae032890ae35ffcb0a..a1cc25567b9d706714b2a5d19929fc946fb46f17 100644 (file)
@@ -43,7 +43,8 @@ tail:
        CMPQ    BX, $4
        JBE     move_3or4
        CMPQ    BX, $8
-       JBE     move_5through8
+       JB      move_5through7
+       JE      move_8
        CMPQ    BX, $16
        JBE     move_9through16
 
@@ -113,12 +114,17 @@ move_3or4:
        MOVW    AX, (DI)
        MOVW    CX, -2(DI)(BX*1)
        RET
-move_5through8:
+move_5through7:
        MOVL    (SI), AX
        MOVL    -4(SI)(BX*1), CX
        MOVL    AX, (DI)
        MOVL    CX, -4(DI)(BX*1)
        RET
+move_8:
+       // We need a separate case for 8 to make sure we write pointers atomically.
+       MOVQ    (SI), AX
+       MOVQ    AX, (DI)
+       RET
 move_9through16:
        MOVQ    (SI), AX
        MOVQ    -8(SI)(BX*1), CX
diff --git a/test/fixedbugs/issue13160.go b/test/fixedbugs/issue13160.go
new file mode 100644 (file)
index 0000000..7eb4811
--- /dev/null
@@ -0,0 +1,70 @@
+// run
+
+// Copyright 2015 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 main
+
+import (
+       "fmt"
+       "runtime"
+)
+
+const N = 100000
+
+func main() {
+       // Allocate more Ps than processors.  This raises
+       // the chance that we get interrupted by the OS
+       // in exactly the right (wrong!) place.
+       p := runtime.NumCPU()
+       runtime.GOMAXPROCS(2 * p)
+
+       // Allocate some pointers.
+       ptrs := make([]*int, p)
+       for i := 0; i < p; i++ {
+               ptrs[i] = new(int)
+       }
+
+       // Arena where we read and write pointers like crazy.
+       collider := make([]*int, p)
+
+       done := make(chan struct{}, 2*p)
+
+       // Start writers.  They alternately write a pointer
+       // and nil to a slot in the collider.
+       for i := 0; i < p; i++ {
+               i := i
+               go func() {
+                       for j := 0; j < N; j++ {
+                               // Write a pointer using memmove.
+                               copy(collider[i:i+1], ptrs[i:i+1])
+                               // Write nil using memclr.
+                               // (This is a magic loop that gets lowered to memclr.)
+                               r := collider[i : i+1]
+                               for k := range r {
+                                       r[k] = nil
+                               }
+                       }
+                       done <- struct{}{}
+               }()
+       }
+       // Start readers.  They read pointers from slots
+       // and make sure they are valid.
+       for i := 0; i < p; i++ {
+               i := i
+               go func() {
+                       for j := 0; j < N; j++ {
+                               var ptr [1]*int
+                               copy(ptr[:], collider[i:i+1])
+                               if ptr[0] != nil && ptr[0] != ptrs[i] {
+                                       panic(fmt.Sprintf("bad pointer read %p!", ptr[0]))
+                               }
+                       }
+                       done <- struct{}{}
+               }()
+       }
+       for i := 0; i < 2*p; i++ {
+               <-done
+       }
+}