I just fixed this with https://go-review.googlesource.com/#/c/20791/

Apologies for having an old copy.

That CL actually makes things worse. Now there is a pointer update without a write barrier. If there were a race on the slice, before the GC could not have lost track of the pointer. Now it can. This is kind of an ABA problem. Its very important never to write to a pointer in the heap without using a write barrier. The optimization must delete the pointer write entirely, not just the use of the write barrier.

At current tip:

0x000f 00015 (/tmp/x.go:4)  MOVQ    "".x+8(FP), CX
0x0014 00020 (/tmp/x.go:4)  MOVQ    16(CX), DX
0x0018 00024 (/tmp/x.go:4)  MOVQ    (CX), BX
0x001b 00027 (/tmp/x.go:4)  CMPQ    DX, $8
0x001f 00031 (/tmp/x.go:4)  JCS $0, 91
0x0021 00033 (/tmp/x.go:4)  CMPQ    DX, $0
0x0025 00037 (/tmp/x.go:4)  JEQ $0, 39
0x0027 00039 (/tmp/x.go:4)  MOVQ    BX, (CX) <<< update to heap pointer without write barrier (!!!)
0x002a 00042 (/tmp/x.go:4)  MOVQ    $8, 8(CX)
0x0032 00050 (/tmp/x.go:4)  MOVQ    DX, 16(CX) <<< unnecessary update of cap

Ok, then maybe I misunderstand what guarantees we provide in the presence of races. Are we trying to guarantee that at least the GC doesn't fail if there's a race?
With a race on writing a slice to the heap bad things can happen, like getting the pointer from one slice and the length from another. That could easily crash the gc anyway by overwriting memory.

Yes, no matter what the programmer does as far as racy writes, the GC must not lose track of and free a pointer that is still referenced. Obviously programs that corrupt memory or that hide pointers in uintptr fields may crash, but not all racy programs corrupt memory.

Much like Java, Go's memory model states that if there is a race on a single word, a future or concurrent read must see some past write. The memory model also states that multi-word writes behave like multiple single-word writes.

Suppose the initial state of the world is:

*x = make([]byte, 16) // call this x1

and then goroutine g1 does:

*x = make([]byte, 16) // call this x2

and goroutine g2 does

*x = (*x)[:8]

Although there is a race here, the guarantees about the semantics of racy programs mean that this cannot cause arbitrary memory corruption. Because the slice assignments are like three individual word assignments and because the base pointers, lengths, and caps here are valid in any combination, the result should be a valid memory state. Specifically, once both goroutines end, *x should hold one of x1[:8:16] or x1[:16:16] or x2[:8:16] or x2[:16:16]. None of those is addressing memory beyond an underlying allocation, so there's no corruption possible.

Now suppose that while these goroutines are running, the GC is also running, and consider only the slice base pointer. It could happen in the current SSA-generated code that the GC starts, then g2 reads x1 from *x on the rhs of its assignment, then g1 writes x2 to *x, then the GC scans the memory object holding *x, seeing x2. Then g2 writes x1 back to *x in its assignment, without a write barrier. Now we're in a state where the GC thinks x2 is the live pointer stored in *x, and has not seen and will never see x1. Eventually the GC will collect x1 despite it being referenced by *x, eventually causing memory corruption in a program that had none.

This is why every pointer write to the heap must use a write barrier. Optimizations to eliminate write barriers must do so by eliminating the pointer writes entirely.

Much like Java, Go's memory model states that if there is a race on a single word, a future or concurrent read must see some past write.

I'm unable to locate this guarantee in the memory model document. Can you point it out?

CL https://golang.org/cl/20951 mentions this issue.

CL https://golang.org/cl/20952 mentions this issue.

@cespare, see golang.org/ref/mem in the "Happens Before" section and search for "allowed". Each read is only allowed to see some value written by a write that is either in the past (and not clearly overwritten) or happening concurrently.

CL https://golang.org/cl/20954 mentions this issue.