runtime, strings, bytes: missing publication barriers after mallocgc with needzero=false and bytealg.MakeNoZero

[thepudds]

Background

A mallocgc caller can set the needzero parameter to false to say that the caller is taking responsibility for overwriting any bytes that will be visible to the user, and hence stating that the mallocgc code does not need to zero out the allocated bytes that will be overwritten before ever being visible to normal user code. This is only permitted with noscan allocations (heap objects that do not contain pointers).

func mallocgc(size uintptr, typ *_type, needzero bool) unsafe.Pointer

The common case for mallocgc is needzero is set to true. In that case, mallocgc and the related functions are careful to issue a publication barrier after the memory is zeroed, which helps ensure user programs do not observe uninitialized memory in the presence of races including on weakly ordered machines. The publication barrier makes it so that the zeroing stores cannot be reordered with the store that publishes the allocated pointer.

Problem

However, while reviewing the runtime.freegc work in #74299, @RLH noticed a general long-standing problem that most or perhaps all current callers of mallocgc with needzero=false do not seem to be issuing publication barriers.

In #74299 (comment), @RLH wrote:

I'm not sure the GC can relinquish "do not zero" responsibility to the caller given the Go memory model outlawing out of thin air or acausal writes. Before Go got its memory model act together out of thin air values were fine in racy programs. I looked at some of the slice code and it does not mention out of thin air hazards or seem to have any publication barriers logic so I suspect the code does not deal with the issue. Furthermore they seem to not use the optimization for pointer values so they seem to working around this bug when pointers are used but not the general problem. Feel free to correct me, memory model stuff in racy programs is complicated.

and @RLH continued in #74299 (comment):

The larger issue is that anyone calling the allocator with a needszero == false gets out of thin air values back and must issue a publicationBarrier once all values have been initialized but before the library routine publishes. Perhaps my code grep abilities have gotten rusty but I do not see any calls to publicationBarrier in the go repository code after calls to mallocgc with needzero == false. We can fix it by adding publicationBarriers to the Go library code.

I believe @mknyszek concurred this is a general problem in #74299 (comment):

I think you're correct that these are missing in general. Of course, we won't notice on x86...

---

I have the start of a CL that I plan to send to add the missing publication barriers (in a few places in the runtime, and also handle elsewhere like the strings and bytes packages via internal/bytealg.MakeNoZero, and possibly elsewhere).

That said, this is a subtle area, so I'd be of course fine if the core Go team prefers to do this themselves.

[gabyhelp]

Related Issues

• runtime: mallocgc does not seem to need to call publicationBarrier when allocating noscan objects #63640
• runtime: non-preemptible zeroing in clear, append, bytes.growSlice, etc #69327

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[JunyangShao]

@golang/runtime

[mknyszek]

@thepudds thanks for filing an issue, and if you'd like to send a CL, please feel free!

To put a finer point on it, failing to include a publication barrier here I believe creates the potential for observable out-of-thin-air values, which are forbidden by Go's memory model, even in racy programs.

Here's the "bad" code in question (note that this is using runtime primitives to make a point, but the idea is that the compiler could generate code like this, or the runtime could have code like this (for example in runtime.growslice) today):

var gx *[4]int

func allocThenInitialize4Int() {
    x := (*[4]int)(mallocgc(abi.TypeOf([4]int{}), unsafe.Sizeof([4]int{}), false))
    x = [4]int{1, 2, 3, 4}
    // publicationBarrier missing
    gx = x
}

func read() {
    x := gx
    if x != nil {
        println(x[3]) // can read out-of-thin-air value on architectures with weak memory models
    }
}

func main() {
    go read()
    allocThenInitialize4Int()
}

CC @randall77 who might have more to say about this issue.