winch: drc barriers - #14081
Conversation
Step 2 of the barrier plan: all of the spec's memory traffic, none of its branches. Knowingly wrong for null until the guards land; the disas test is the oracle.
First of the two step-3 guards; the decrement guard is next.
Completes the write barrier's null guards; a global.set of ref.null touches no refcount on either side.
Completes the write barrier: the decremented count is only written back on the nonzero path, matching the cranelift DRC barrier; at zero the old value goes to the drop_gc_ref builtin in a cold block. The call gets a stack map like any other.
Proven load-bearing: with the barrier arm disabled the object is freed while the global still holds it (premature free), failing the first assertion.
Winch's barriers reference these constants unconditionally, so move them to the always-compiled gc module and make TRAP_GC_HEAP_CORRUPT public for the barrier bounds checks.
Adds the read barrier for global.get: references loaded onto the stack are enrolled in the over-approximated stack-roots list so a collection cannot free them out from under the frame that loaded them, forcing a GC when the list outgrows its threshold. The loaded reference is homed into its stack slot first so the force-gc call's stack map covers it. Both barriers now trap GC_HEAP_CORRUPT if an object's DRC header would extend past the GC heap's current length before any header access, matching Cranelift's prepare_gc_ref_access. Read-barrier test proven load-bearing under gc_zeal.
|
Happy to take this one as well @alexcrichton. |
Subscribe to Label ActionDetailsThis issue or pull request has been labeled: "wasmtime:api", "wasmtime:config", "wasmtime:docs", "winch"Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
Label Messager: wasmtime:configIt looks like you are changing Wasmtime's configuration options. Make sure to
DetailsTo modify this label's message, edit the To add new label messages or remove existing label messages, edit the |
|
cc @fitzgen on this as well as you're likely curious In talking briefly about this in-person at the CG meeting last week, one possibility as well could be that the DRC collector may not be supported by Winch and instead only the copying/null collectors are supported which don't need barriers. Unless you've got a specific need for DRC @macovedj it seems ok to me if Winch only supports a subset of the GC collectors (notably the main one though, the copying collector). Although having hooks for barriers is always a pretty nice thing since it makes adding/refactoring in the future easier since the barriers are already outlined. Mostly just wanted to throw this out there that I don't personally think DRC is required in Winch to turn GC/exceptions on for Winch in the long-run. |
Oh interesting. I don't have a strong need, and I think that not adding it would simplify the EH work ahead for winch. That said, I talked with @saulecabrera a bit offline and got the impression that this isn't that far from landing and that winch benefits from the cranelift parity. Happy to drop the PR if anybody has concerns, but also happy to keep moving it forward if the DRC support is attractive to everybody. |
|
@alexcrichton I agree that this is not required to land GC and EH. However, sometimes I find it a bit difficult to draw the line on when a compiler (Winch in my case) should support or not certain engine features. My assumption until now has been that Winch should try to provide as much parity as possible with Cranelift, unless there's a real technical reason not to (e.g., supporting function inlining). In this case parity seemed the right default to me; but also we can decide not to land it if there are any specific concerns. |
|
No yeah that all sounds reasonable to me. If it's significantly complicated for Winch to support something I personally think it's always in the cards to avoid implementing it, but that's not the case here so seems totally fine to land |
Continues the work tracked in #14057 by adding DRC barriers for GC references in Winch. This unblocks exception-handling support work in Winch.
Closes #14057.