Skip to content

Commit 80ec711

Browse files
committed
runtime: use type-based write barrier for remote stack write during chansend
A send on an unbuffered channel to a blocked receiver is the only case in the runtime where one goroutine writes directly to the stack of another. The garbage collector assumes that if a goroutine is blocked, its stack contains no new pointers since the last time it ran. The send on an unbuffered channel violates this, so it needs an explicit write barrier. It has an explicit write barrier, but not one that can handle a write to another stack. Use one that can (based on type bitmap instead of heap bitmap). To make this work, raise the limit for type bitmaps so that they are used for all types up to 64 kB in size (256 bytes of bitmap). (The runtime already imposes a limit of 64 kB for a channel element size.) I have been unable to reproduce this problem in a simple test program. Could help #11035. Change-Id: I06ad994032d8cff3438c9b3eaa8d853915128af5 Reviewed-on: https://go-review.googlesource.com/10815 Reviewed-by: Austin Clements <[email protected]>
1 parent d14e9e6 commit 80ec711

File tree

5 files changed

+88
-24
lines changed

5 files changed

+88
-24
lines changed

src/cmd/compile/internal/gc/reflect.go

+9-2
Original file line numberDiff line numberDiff line change
@@ -1403,10 +1403,17 @@ func dalgsym(t *Type) *Sym {
14031403
// is 32 pointers, the bits for which fit in 4 bytes. So maxPtrmaskBytes
14041404
// must be >= 4.
14051405
//
1406-
// We use 16 because the GC programs do have some constant overhead
1406+
// We used to use 16 because the GC programs do have some constant overhead
14071407
// to get started, and processing 128 pointers seems to be enough to
14081408
// amortize that overhead well.
1409-
const maxPtrmaskBytes = 16
1409+
//
1410+
// To make sure that the runtime's chansend can call typeBitsBulkBarrier,
1411+
// we raised the limit to 2048, so that even 32-bit systems are guaranteed to
1412+
// use bitmaps for objects up to 64 kB in size.
1413+
//
1414+
// Also known to reflect/type.go.
1415+
//
1416+
const maxPtrmaskBytes = 2048
14101417

14111418
// dgcsym emits and returns a data symbol containing GC information for type t,
14121419
// along with a boolean reporting whether the UseGCProg bit should be set in

src/reflect/all_test.go

+20-20
Original file line numberDiff line numberDiff line change
@@ -4595,39 +4595,39 @@ func TestGCBits(t *testing.T) {
45954595
verifyGCBits(t, ArrayOf(1, Tptrscalar), lit(1))
45964596
verifyGCBits(t, TypeOf([2]Xscalar{}), empty)
45974597
verifyGCBits(t, ArrayOf(2, Tscalar), empty)
4598-
verifyGCBits(t, TypeOf([100]Xscalar{}), empty)
4599-
verifyGCBits(t, ArrayOf(100, Tscalar), empty)
4598+
verifyGCBits(t, TypeOf([10000]Xscalar{}), empty)
4599+
verifyGCBits(t, ArrayOf(10000, Tscalar), empty)
46004600
verifyGCBits(t, TypeOf([2]Xptr{}), lit(1, 1))
46014601
verifyGCBits(t, ArrayOf(2, Tptr), lit(1, 1))
4602-
verifyGCBits(t, TypeOf([100]Xptr{}), rep(100, lit(1)))
4603-
verifyGCBits(t, ArrayOf(100, Tptr), rep(100, lit(1)))
4602+
verifyGCBits(t, TypeOf([10000]Xptr{}), rep(10000, lit(1)))
4603+
verifyGCBits(t, ArrayOf(10000, Tptr), rep(10000, lit(1)))
46044604
verifyGCBits(t, TypeOf([2]Xscalarptr{}), lit(0, 1, 0, 1))
46054605
verifyGCBits(t, ArrayOf(2, Tscalarptr), lit(0, 1, 0, 1))
4606-
verifyGCBits(t, TypeOf([100]Xscalarptr{}), rep(100, lit(0, 1)))
4607-
verifyGCBits(t, ArrayOf(100, Tscalarptr), rep(100, lit(0, 1)))
4606+
verifyGCBits(t, TypeOf([10000]Xscalarptr{}), rep(10000, lit(0, 1)))
4607+
verifyGCBits(t, ArrayOf(10000, Tscalarptr), rep(10000, lit(0, 1)))
46084608
verifyGCBits(t, TypeOf([2]Xptrscalar{}), lit(1, 0, 1))
46094609
verifyGCBits(t, ArrayOf(2, Tptrscalar), lit(1, 0, 1))
4610-
verifyGCBits(t, TypeOf([100]Xptrscalar{}), rep(100, lit(1, 0)))
4611-
verifyGCBits(t, ArrayOf(100, Tptrscalar), rep(100, lit(1, 0)))
4612-
verifyGCBits(t, TypeOf([1][100]Xptrscalar{}), rep(100, lit(1, 0)))
4613-
verifyGCBits(t, ArrayOf(1, ArrayOf(100, Tptrscalar)), rep(100, lit(1, 0)))
4614-
verifyGCBits(t, TypeOf([2][100]Xptrscalar{}), rep(200, lit(1, 0)))
4615-
verifyGCBits(t, ArrayOf(2, ArrayOf(100, Tptrscalar)), rep(200, lit(1, 0)))
4610+
verifyGCBits(t, TypeOf([10000]Xptrscalar{}), rep(10000, lit(1, 0)))
4611+
verifyGCBits(t, ArrayOf(10000, Tptrscalar), rep(10000, lit(1, 0)))
4612+
verifyGCBits(t, TypeOf([1][10000]Xptrscalar{}), rep(10000, lit(1, 0)))
4613+
verifyGCBits(t, ArrayOf(1, ArrayOf(10000, Tptrscalar)), rep(10000, lit(1, 0)))
4614+
verifyGCBits(t, TypeOf([2][10000]Xptrscalar{}), rep(2*10000, lit(1, 0)))
4615+
verifyGCBits(t, ArrayOf(2, ArrayOf(10000, Tptrscalar)), rep(2*10000, lit(1, 0)))
46164616

46174617
verifyGCBits(t, TypeOf((chan [100]Xscalar)(nil)), lit(1))
46184618
verifyGCBits(t, ChanOf(BothDir, ArrayOf(100, Tscalar)), lit(1))
46194619

4620-
verifyGCBits(t, TypeOf((func([100]Xscalarptr))(nil)), lit(1))
4621-
verifyGCBits(t, FuncOf([]Type{ArrayOf(100, Tscalarptr)}, nil, false), lit(1))
4620+
verifyGCBits(t, TypeOf((func([10000]Xscalarptr))(nil)), lit(1))
4621+
verifyGCBits(t, FuncOf([]Type{ArrayOf(10000, Tscalarptr)}, nil, false), lit(1))
46224622

4623-
verifyGCBits(t, TypeOf((map[[100]Xscalarptr]Xscalar)(nil)), lit(1))
4624-
verifyGCBits(t, MapOf(ArrayOf(100, Tscalarptr), Tscalar), lit(1))
4623+
verifyGCBits(t, TypeOf((map[[10000]Xscalarptr]Xscalar)(nil)), lit(1))
4624+
verifyGCBits(t, MapOf(ArrayOf(10000, Tscalarptr), Tscalar), lit(1))
46254625

4626-
verifyGCBits(t, TypeOf((*[100]Xscalar)(nil)), lit(1))
4627-
verifyGCBits(t, PtrTo(ArrayOf(100, Tscalar)), lit(1))
4626+
verifyGCBits(t, TypeOf((*[10000]Xscalar)(nil)), lit(1))
4627+
verifyGCBits(t, PtrTo(ArrayOf(10000, Tscalar)), lit(1))
46284628

4629-
verifyGCBits(t, TypeOf(([][100]Xscalar)(nil)), lit(1))
4630-
verifyGCBits(t, SliceOf(ArrayOf(100, Tscalar)), lit(1))
4629+
verifyGCBits(t, TypeOf(([][10000]Xscalar)(nil)), lit(1))
4630+
verifyGCBits(t, SliceOf(ArrayOf(10000, Tscalar)), lit(1))
46314631

46324632
hdr := make([]byte, 8/PtrSize)
46334633
verifyGCBits(t, MapBucketOf(Tscalar, Tptr), join(hdr, rep(8, lit(0)), rep(8, lit(1)), lit(1)))

src/reflect/type.go

+4-1
Original file line numberDiff line numberDiff line change
@@ -1797,6 +1797,9 @@ func SliceOf(t Type) Type {
17971797
return cachePut(ckey, &slice.rtype)
17981798
}
17991799

1800+
// See cmd/compile/internal/gc/reflect.go for derivation of constant.
1801+
const maxPtrmaskBytes = 2048
1802+
18001803
// ArrayOf returns the array type with the given count and element type.
18011804
// For example, if t represents int, ArrayOf(5, t) represents [5]int.
18021805
//
@@ -1865,7 +1868,7 @@ func ArrayOf(count int, elem Type) Type {
18651868
array.gcdata = typ.gcdata
18661869
array.ptrdata = typ.ptrdata
18671870

1868-
case typ.kind&kindGCProg == 0 && array.size <= 16*8*ptrSize:
1871+
case typ.kind&kindGCProg == 0 && array.size <= maxPtrmaskBytes*8*ptrSize:
18691872
// Element is small with pointer mask; array is still small.
18701873
// Create direct pointer mask by turning each 1 bit in elem
18711874
// into count 1 bits in larger mask.

src/runtime/chan.go

+10-1
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,16 @@ func chansend(t *chantype, c *hchan, ep unsafe.Pointer, block bool, callerpc uin
165165

166166
recvg := sg.g
167167
if sg.elem != nil {
168-
typedmemmove(c.elemtype, unsafe.Pointer(sg.elem), ep)
168+
// This is the only place in the entire runtime where one goroutine
169+
// writes to the stack of another goroutine. The GC assumes that
170+
// stack writes only happen when the goroutine is running and are
171+
// only done by that goroutine. Using a write barrier is sufficient to
172+
// make up for violating that assumption, but the write barrier has to work.
173+
// typedmemmove will call heapBitsBulkBarrier, but the target bytes
174+
// are not in the heap, so that will not help. We arrange to call
175+
// memmove and typeBitsBulkBarrier instead.
176+
memmove(sg.elem, ep, c.elemtype.size)
177+
typeBitsBulkBarrier(c.elemtype, uintptr(sg.elem), c.elemtype.size)
169178
sg.elem = nil
170179
}
171180
recvg.param = unsafe.Pointer(sg)

src/runtime/mbitmap.go

+45
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,51 @@ func heapBitsBulkBarrier(p, size uintptr) {
413413
}
414414
}
415415

416+
// typeBitsBulkBarrier executes writebarrierptr_nostore
417+
// for every pointer slot in the memory range [p, p+size),
418+
// using the type bitmap to locate those pointer slots.
419+
// The type typ must correspond exactly to [p, p+size).
420+
// This executes the write barriers necessary after a copy.
421+
// Both p and size must be pointer-aligned.
422+
// The type typ must have a plain bitmap, not a GC program.
423+
// The only use of this function is in channel sends, and the
424+
// 64 kB channel element limit takes care of this for us.
425+
//
426+
// Must not be preempted because it typically runs right after memmove,
427+
// and the GC must not complete between those two.
428+
//
429+
//go:nosplit
430+
func typeBitsBulkBarrier(typ *_type, p, size uintptr) {
431+
if typ == nil {
432+
throw("runtime: typeBitsBulkBarrier without type")
433+
}
434+
if typ.size != size {
435+
println("runtime: typeBitsBulkBarrier with type ", *typ._string, " of size ", typ.size, " but memory size", size)
436+
throw("runtime: invalid typeBitsBulkBarrier")
437+
}
438+
if typ.kind&kindGCProg != 0 {
439+
println("runtime: typeBitsBulkBarrier with type ", *typ._string, " with GC prog")
440+
throw("runtime: invalid typeBitsBulkBarrier")
441+
}
442+
if !writeBarrierEnabled {
443+
return
444+
}
445+
ptrmask := typ.gcdata
446+
var bits uint32
447+
for i := uintptr(0); i < typ.ptrdata; i += ptrSize {
448+
if i&(ptrSize*8-1) == 0 {
449+
bits = uint32(*ptrmask)
450+
ptrmask = addb(ptrmask, 1)
451+
} else {
452+
bits = bits >> 1
453+
}
454+
if bits&1 != 0 {
455+
x := (*uintptr)(unsafe.Pointer(p + i))
456+
writebarrierptr_nostore(x, *x)
457+
}
458+
}
459+
}
460+
416461
// The methods operating on spans all require that h has been returned
417462
// by heapBitsForSpan and that size, n, total are the span layout description
418463
// returned by the mspan's layout method.

0 commit comments

Comments
 (0)