Skip to content

Commit 0b5982f

Browse files
randall77aclements
authored andcommitted
[release-branch.go1.5] runtime: memmove/memclr pointers atomically
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 <[email protected]> Run-TryBot: Keith Randall <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-on: https://go-review.googlesource.com/16910 Reviewed-by: Russ Cox <[email protected]>
1 parent d3a4135 commit 0b5982f

10 files changed

+135
-20
lines changed

src/runtime/asm_amd64p32.s

+3
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,9 @@ TEXT runtime·memclr(SB),NOSPLIT,$0-8
636636
MOVQ BX, CX
637637
REP
638638
STOSB
639+
// Note: we zero only 4 bytes at a time so that the tail is at most
640+
// 3 bytes. That guarantees that we aren't zeroing pointers with STOSB.
641+
// See issue 13160.
639642
RET
640643

641644
TEXT runtime·getcallerpc(SB),NOSPLIT,$8-12

src/runtime/memclr_386.s

+8-3
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ tail:
2121
CMPL BX, $2
2222
JBE _1or2
2323
CMPL BX, $4
24-
JBE _3or4
24+
JB _3
25+
JE _4
2526
CMPL BX, $8
2627
JBE _5through8
2728
CMPL BX, $16
@@ -68,9 +69,13 @@ _1or2:
6869
RET
6970
_0:
7071
RET
71-
_3or4:
72+
_3:
7273
MOVW AX, (DI)
73-
MOVW AX, -2(DI)(BX*1)
74+
MOVB AX, 2(DI)
75+
RET
76+
_4:
77+
// We need a separate case for 4 to make sure we clear pointers atomically.
78+
MOVL AX, (DI)
7479
RET
7580
_5through8:
7681
MOVL AX, (DI)

src/runtime/memclr_amd64.s

+7-2
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ tail:
2323
CMPQ BX, $4
2424
JBE _3or4
2525
CMPQ BX, $8
26-
JBE _5through8
26+
JB _5through7
27+
JE _8
2728
CMPQ BX, $16
2829
JBE _9through16
2930
PXOR X0, X0
@@ -71,10 +72,14 @@ _3or4:
7172
MOVW AX, (DI)
7273
MOVW AX, -2(DI)(BX*1)
7374
RET
74-
_5through8:
75+
_5through7:
7576
MOVL AX, (DI)
7677
MOVL AX, -4(DI)(BX*1)
7778
RET
79+
_8:
80+
// We need a separate case for 8 to make sure we clear pointers atomically.
81+
MOVQ AX, (DI)
82+
RET
7883
_9through16:
7984
MOVQ AX, (DI)
8085
MOVQ AX, -8(DI)(BX*1)

src/runtime/memclr_plan9_386.s

+8-3
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ tail:
1616
CMPL BX, $2
1717
JBE _1or2
1818
CMPL BX, $4
19-
JBE _3or4
19+
JB _3
20+
JE _4
2021
CMPL BX, $8
2122
JBE _5through8
2223
CMPL BX, $16
@@ -35,9 +36,13 @@ _1or2:
3536
RET
3637
_0:
3738
RET
38-
_3or4:
39+
_3:
3940
MOVW AX, (DI)
40-
MOVW AX, -2(DI)(BX*1)
41+
MOVB AX, 2(DI)
42+
RET
43+
_4:
44+
// We need a separate case for 4 to make sure we clear pointers atomically.
45+
MOVL AX, (DI)
4146
RET
4247
_5through8:
4348
MOVL AX, (DI)

src/runtime/memmove_386.s

+10-4
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ tail:
4343
CMPL BX, $2
4444
JBE move_1or2
4545
CMPL BX, $4
46-
JBE move_3or4
46+
JB move_3
47+
JE move_4
4748
CMPL BX, $8
4849
JBE move_5through8
4950
CMPL BX, $16
@@ -118,11 +119,16 @@ move_1or2:
118119
RET
119120
move_0:
120121
RET
121-
move_3or4:
122+
move_3:
122123
MOVW (SI), AX
123-
MOVW -2(SI)(BX*1), CX
124+
MOVB 2(SI), CX
124125
MOVW AX, (DI)
125-
MOVW CX, -2(DI)(BX*1)
126+
MOVB CX, 2(DI)
127+
RET
128+
move_4:
129+
// We need a separate case for 4 to make sure we write pointers atomically.
130+
MOVL (SI), AX
131+
MOVL AX, (DI)
126132
RET
127133
move_5through8:
128134
MOVL (SI), AX

src/runtime/memmove_amd64.s

+8-2
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ tail:
5050
CMPQ BX, $4
5151
JBE move_3or4
5252
CMPQ BX, $8
53-
JBE move_5through8
53+
JB move_5through7
54+
JE move_8
5455
CMPQ BX, $16
5556
JBE move_9through16
5657
CMPQ BX, $32
@@ -131,12 +132,17 @@ move_3or4:
131132
MOVW AX, (DI)
132133
MOVW CX, -2(DI)(BX*1)
133134
RET
134-
move_5through8:
135+
move_5through7:
135136
MOVL (SI), AX
136137
MOVL -4(SI)(BX*1), CX
137138
MOVL AX, (DI)
138139
MOVL CX, -4(DI)(BX*1)
139140
RET
141+
move_8:
142+
// We need a separate case for 8 to make sure we write pointers atomically.
143+
MOVQ (SI), AX
144+
MOVQ AX, (DI)
145+
RET
140146
move_9through16:
141147
MOVQ (SI), AX
142148
MOVQ -8(SI)(BX*1), CX

src/runtime/memmove_nacl_amd64p32.s

+3
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,7 @@ back:
4646
REP; MOVSB
4747
CLD
4848

49+
// Note: we copy only 4 bytes at a time so that the tail is at most
50+
// 3 bytes. That guarantees that we aren't copying pointers with MOVSB.
51+
// See issue 13160.
4952
RET

src/runtime/memmove_plan9_386.s

+10-4
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ tail:
3939
CMPL BX, $2
4040
JBE move_1or2
4141
CMPL BX, $4
42-
JBE move_3or4
42+
JB move_3
43+
JE move_4
4344
CMPL BX, $8
4445
JBE move_5through8
4546
CMPL BX, $16
@@ -104,11 +105,16 @@ move_1or2:
104105
RET
105106
move_0:
106107
RET
107-
move_3or4:
108+
move_3:
108109
MOVW (SI), AX
109-
MOVW -2(SI)(BX*1), CX
110+
MOVB 2(SI), CX
110111
MOVW AX, (DI)
111-
MOVW CX, -2(DI)(BX*1)
112+
MOVB CX, 2(DI)
113+
RET
114+
move_4:
115+
// We need a separate case for 4 to make sure we write pointers atomically.
116+
MOVL (SI), AX
117+
MOVL AX, (DI)
112118
RET
113119
move_5through8:
114120
MOVL (SI), AX

src/runtime/memmove_plan9_amd64.s

+8-2
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ tail:
4343
CMPQ BX, $4
4444
JBE move_3or4
4545
CMPQ BX, $8
46-
JBE move_5through8
46+
JB move_5through7
47+
JE move_8
4748
CMPQ BX, $16
4849
JBE move_9through16
4950

@@ -113,12 +114,17 @@ move_3or4:
113114
MOVW AX, (DI)
114115
MOVW CX, -2(DI)(BX*1)
115116
RET
116-
move_5through8:
117+
move_5through7:
117118
MOVL (SI), AX
118119
MOVL -4(SI)(BX*1), CX
119120
MOVL AX, (DI)
120121
MOVL CX, -4(DI)(BX*1)
121122
RET
123+
move_8:
124+
// We need a separate case for 8 to make sure we write pointers atomically.
125+
MOVQ (SI), AX
126+
MOVQ AX, (DI)
127+
RET
122128
move_9through16:
123129
MOVQ (SI), AX
124130
MOVQ -8(SI)(BX*1), CX

test/fixedbugs/issue13160.go

+70
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
// run
2+
3+
// Copyright 2015 The Go Authors. All rights reserved.
4+
// Use of this source code is governed by a BSD-style
5+
// license that can be found in the LICENSE file.
6+
7+
package main
8+
9+
import (
10+
"fmt"
11+
"runtime"
12+
)
13+
14+
const N = 100000
15+
16+
func main() {
17+
// Allocate more Ps than processors. This raises
18+
// the chance that we get interrupted by the OS
19+
// in exactly the right (wrong!) place.
20+
p := runtime.NumCPU()
21+
runtime.GOMAXPROCS(2 * p)
22+
23+
// Allocate some pointers.
24+
ptrs := make([]*int, p)
25+
for i := 0; i < p; i++ {
26+
ptrs[i] = new(int)
27+
}
28+
29+
// Arena where we read and write pointers like crazy.
30+
collider := make([]*int, p)
31+
32+
done := make(chan struct{}, 2*p)
33+
34+
// Start writers. They alternately write a pointer
35+
// and nil to a slot in the collider.
36+
for i := 0; i < p; i++ {
37+
i := i
38+
go func() {
39+
for j := 0; j < N; j++ {
40+
// Write a pointer using memmove.
41+
copy(collider[i:i+1], ptrs[i:i+1])
42+
// Write nil using memclr.
43+
// (This is a magic loop that gets lowered to memclr.)
44+
r := collider[i : i+1]
45+
for k := range r {
46+
r[k] = nil
47+
}
48+
}
49+
done <- struct{}{}
50+
}()
51+
}
52+
// Start readers. They read pointers from slots
53+
// and make sure they are valid.
54+
for i := 0; i < p; i++ {
55+
i := i
56+
go func() {
57+
for j := 0; j < N; j++ {
58+
var ptr [1]*int
59+
copy(ptr[:], collider[i:i+1])
60+
if ptr[0] != nil && ptr[0] != ptrs[i] {
61+
panic(fmt.Sprintf("bad pointer read %p!", ptr[0]))
62+
}
63+
}
64+
done <- struct{}{}
65+
}()
66+
}
67+
for i := 0; i < 2*p; i++ {
68+
<-done
69+
}
70+
}

0 commit comments

Comments
 (0)