-
Notifications
You must be signed in to change notification settings - Fork 18k
cmd/compile: pointer update without write barrier in reslice operation #14855
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
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:
|
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? |
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:
and then goroutine g1 does:
and goroutine g2 does
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. |
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. |
The ssa back end does not eliminate as many write barriers as the non-ssa back end. This should be fixed. I thought there were tests of this. Were they disabled?
In particular, reslicing an addressable slice with low index 0 should not rewrite the pointer (or the cap, usually), and not rewriting the pointer will avoid the expense of a write barrier.
Test program:
Without ssa:
With ssa:
If I remember correctly, the optimization in the non-ssa back end applies to any expression of the form thing = thing[:n], where thing can be arbitrarily long, provided its the same on both sides, has no idempotency problems (like function calls), and is addressable.
/cc @randall77 @dr2chase
The text was updated successfully, but these errors were encountered: