Dismantle dying records via field takes - #10475
Merged
Merged
Conversation
Fixes two independent bugs behind issue 10461, where roc-deflate's CompressFastest panicked at --opt=speed. SpecConstr's loop-exit selection spliced the continuation into the loop body when the loop had exactly one exit site. A continuation carrying a continue or break of an enclosing loop cannot move inside this loop -- the control would rebind to the wrong loop, tripping cloneContinue's arity invariant (or silently miscompiling when the arities happen to match) -- so such a continuation now stays outside as a join body. That join body then exposed a ScalarizeJoins bug: a join parameter whose only initialization is a struct literal built directly into it and whose only use is an initialize_join_param copy qualified as a splattable wrapper temporary. Splatting deleted the literal -- that join's own edge initialization -- leaving the parameter declared but never initialized, which ARC turned into a release of an unbound local. A join parameter now never qualifies; such initializers seed by field reads, which keeps the parameter intact and lets a later fixpoint round scalarize it on its own.
Scalarizing a join parameter seeded from a non-literal initializer emitted one read-write pair per field. On a loop back edge the initializer value can borrow from a parameter's old value -- the payload of a boxed capture record carried by the loop -- and ARC releases that old value at its rebind, so the second field read dereferenced an already-released box. The interpreter higher-order closure tests caught this as a use-after-free once main's strengthened certifier checks were merged in. Seeding now reads every field before writing any parameter, the same order jump lowering uses when it evaluates all arguments before initializing any parameter.
# Conflicts: # src/postcheck/monotype_lifted/spec_constr.zig
# Conflicts: # src/eval/test/lir_inline_test.zig # src/lir/scalarize_joins.zig
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Continues the record in-place work: dying record containers are now dismantled via per-field takes (src/lir/arc_dismantle.zig, solved between borrow inference and RC emission), and take spines can cross branch diamonds so a field read placed after an if or match keeps its take.
This also fixes three bugs found while working on the branch. Issue #10461's --opt=speed panic was two of them: SpecConstr spliced a loop's continuation into the loop body even when it carried an enclosing loop's continue or break, rebinding the control to the wrong loop; and ScalarizeJoins misclassified a neighboring join's parameter as a splattable wrapper temporary and deleted its only initialization. The third was a use-after-free in scalarize's field seeding: reads were interleaved with parameter rebinds, and ARC releases a parameter's old value at its rebind, so a later read could go through an already-released box. Seeding now reads every field before writing any parameter, matching how jump lowering evaluates all arguments before initializing any parameter.