fix(mir): compensate sibling-field drops along multi-hop alias chains#2382
Merged
Conversation
A match that binds a string-typed field of an owned record or tuple and consumes the scrutinee root left the field's original handle stranded. Codegen retains string field loads (hew_string_clone), so the binder owns a clone while the original still sits in the root slot; the root's composite drop is suppressed (the binder seeds release_owner_bases) and the consume mark retracts the root, so nothing released the original and it leaked one buffer per destructure. Discharge the original in place right after the load: FieldDropInPlace raw-loads the root's field handle, releases it, and null-stores the slot, leaving the binder as sole owner. The discharge is gated on all three facts that keep it double-free free — the scrutinee is consumed, the root is not an interior alias, and the field is string-typed (the retaining-load class). Non-string binders take the one handle and leave a dead root slot, so they are untouched; an alias root's original belongs to the outer composite, which frees it. The interior-alias classifier is hoisted above the bind loop so the bound-string and skipped-field discharges read one verdict. Fixes #2373
Owned-local drop obligations were pushed as bare (binding, name, ty) tuples at a dozen lowering seams, leaving the ownership fact to be re-derived by every downstream drop pass from the instruction stream. Introduce OwnedLocalEntry as the ledger's carried unit and Builder::register_owned_local as its single constructor: every seam now routes through it, classifying the value's ownership once at its defining write and recording it on the entry alongside a scope-exit disposition. The twelve drop provers, build_lifo_drops, and the unwired-Vec diagnostic read an unchanged (binding, name, ty) view via owned_locals_snapshot, so behaviour is byte-identical — the checked-mir corpus dumps identically across all 39 fixtures at both stages. Ownership is now a fact written down once at the defining write and carried on the value for the drop passes to read, rather than a shape each pass re-matches from the instructions. Provenance is carried on the entry too, recorded where an interior alias is trivially provable.
The six seams that took a binding out of the owned-locals ledger mid- lowering did it by physically removing the entry (`owned_locals.retain(|e| e.binding != b)`): the two inner-scope generator/vec-iter releases, the three per-frame yield/recv/vec-string body-end releases, and the consume in `mark_binding_moved`. A removed entry is gone from the ledger by the time any end-of-pass scan runs, so a binding whose release was handled mid-lowering is invisible to a whole-ledger walk — the retracted-invisible class behind the tranche double-free and the two-level extraction leak. Replace each removal with a `set_owned_local_disposition` write that dispositions the entry off the scope-exit-live set (`BodyEndReleased`, `ConsumedAt`, `ScopeReleased`) while leaving it in the ledger. The drop- elaboration view (`owned_locals_snapshot`) already filtered to `ScopeExit`, so it sees exactly the set the physical removals used to leave behind; the three direct membership probes (the overwrite-release gate, the registration de-dup guard, and the overwrite-guard-flag alloc) gain the same `ScopeExit` filter, so a retracted entry reads as absent to them. The result is byte-identical: checked-mir verify and golden show zero churn across all 39 fixtures at both stages. A new `owned_locals_ledger` accessor exposes the whole ledger regardless of disposition — the option an end-of-pass scan takes to observe a binding whose release was already handled, which physical removal made impossible. No production scan reads it yet.
A `let mid = o.mid` / `let inner = t.0` projection whose field is an inline aggregate (record / tuple / inline-enum) byte-copies the member with no retain, so the binder is an interior alias of the still-live owner, not an independent owner. But it registered as an owned local by type alone, so a non-consumed projection binder seeded the record and tuple composite provers' `release_owner_bases` set. When such a binder was also a field-binder of its own root, the provers' Defect-1 blanket tripped and excluded EVERY root from its composite drop: a two-level extraction chain (`let mid = o.mid; let leaf = mid.leaf; match leaf ...`) leaked the whole tree, 4 nodes per frame. Classify the three field-load cases the same way codegen emits them: `string` retains (the binder owns a fresh clone), an inline aggregate is a byte-copy alias, and a single-pointer heap leaf (Vec / bytes / map / Generator / indirect-enum node) transfers its one handle. Only the byte-copy alias changes behaviour: it records the owner's provenance and is dispositioned off the scope-exit-live set, so it emits no composite drop of its own and its base local never seeds `release_owner_bases`. The blanket no longer trips and the owner's composite frees the whole tree exactly once. The other two classes keep their existing scope-exit ownership, so string binders and transferred handles are unchanged. The composite provers now read the recorded alias provenance directly. A one-hop alias (`mid` reads the root) is already reachable from the whole-value alias map, but a deeper alias (`leaf` reads `mid`) is not, so its ESCAPE into an owning sink -- returned, stored into an owning record, sent -- would leave the owner admitted to free a subtree the escapee handed to the caller (a double-free, and a use-after-free the caller walks). Fold every recorded alias, resolved through its provenance chain to the owner it aliases and closed forward through hand-off moves, into the provers' field-binder set attributed to that owner. A deep alias that escapes now excludes exactly its owner (fail-closed: the owner's non-escaped siblings leak, never a re-freed subtree); an alias only read interiorly (the consumed-match path) leaves the owner admitted. The alias provenance keys this on the recorded fact, not the live disposition, so a recorded alias later consumed into the return still excludes its owner -- consuming an alias moves no ownership. Recording is fail-closed: a projection whose owner root cannot be named at the defining write keeps today's blanket exclusion -- leak, never double-free. A frozen three-way verdict table pins the classification, and a compiled-binary oracle covers the record and tuple chains, the double-skip destructure, a bystander root, the transferred-handle and consumed-owner controls, and the escaped-alias shapes (returned, stored into an owning record, tuple twin) under the poisoned allocator. Fixes: #2375
DropKind::CowHeap carried its release protocol as a &'static str literal the producer chose and codegen re-derived from the value type to check. That dual derivation needed three codegen congruence validators and a codegen-local release-symbol table that could drift from the MIR pick. Replace the literal with a typed CowHeapRelease — the heap-leaf identity folded with the Vec<E> element refinement — so codegen resolves the C-ABI symbol from the carried fact via CowHeapRelease::release_symbol, built on HeapLeaf::release_symbol. An unknown or type-incongruent release symbol is now unrepresentable, so the three congruence validators and codegen's cow_heap_release_symbol picker retire with the literal they guarded. The sole surviving release picker is the address-based aggregate-field walk (resolved_ty_cow_heap_release), for nested fields that carry no MIR fact of their own, and it reads the same one authority. MIR dumps stay byte-identical across the checked-mir corpus and the full leak-oracle family stays green, including the Guard-Malloc exactly-once legs.
Two comments described a drop-elaboration model that no longer holds now that owned locals are a populated per-function ownership ledger read on every function body. The `elaborate` call-site comment claimed the spine never lowers heap bindings so `owned_locals` is empty and the pass is only exercised by hand-constructed unit inputs. It now describes the real ledger: every drop-obliged binding registers once at its defining write through `register_owned_local`, carrying its classified `ValueOwnership`, any interior-alias `ValueProvenance`, and a `Disposition`; the elaborator reads the scope-exit-live view (a retracted binding carries a non-`ScopeExit` disposition rather than being removed); and the pass is proven end-to-end by the compiled leak-oracle fixtures under the poisoned allocator, not just unit inputs. The `ownership` module doc claimed the typed carrier was additive and switched no production call site. That is no longer true: the ledger records a `ValueOwnership` on every entry and the record/tuple composite drop provers consume the carried `InteriorAlias`-shaped `ValueProvenance` so a byte-copy field alias names its owner by location instead of each prover re-deriving it. The doc now states the carrier is read, with the coarser per-value facts still travelling on the ledger to be read as their consuming seams land. Comment-only; no production behaviour change.
The composite-drop prover suppresses an owned record's whole in-place drop when a deep byte-copy alias escapes into an owning sink — a `let mid = o.mid; let leaf = mid.leaf; return leaf` chain hands the `leaf` subtree to the caller, so the owner must not free it (the #2375 double-free). The prover reaches such a >=2-hop alias through `close_alias_binders_forward`, but the #2212 sibling-discharge emitter saw only a one-hop binder loaded directly off a whole-value alias member. So along a multi-hop chain the widened exclusion removed the composite drop while nothing discharged the non-escaped siblings ALONG the chain — the outer `c` and the intermediate `mid.x` leaked every frame (2 strings/call on the escaped-return shape). Add a chain-walk companion to the sibling emitter: from the escapee up its immediate-parent chain to the owning root, discharge every owned field that does not lead to the next (escaping) hop, addressed through the still-live byte-copy alias local at each level. Exactly-once holds: the escaped field at each level is never discharged (the escapee owns it), every other owned field drops once through its level's slot. The walk uses a new `alias_projection_chain` accessor that keeps the immediate-parent structure the resolved `alias_owner_field_binders` discards. Fail-closed and coupled to the prover: exactly one alias may escape, at a single instruction whose escape trigger is a strict subset of the prover's exclusion triggers; the chain must resolve through >=2 byte-copy hops to a single candidate root; the escape block must not loop; and no chain node may be read after the escape. Any use the walk cannot model bails the pass (leak-as-before, never a double-free). Add the missing leak-slope pins for the escape shapes: the escaped-return and escaped-into record shapes now hold a flat slope (0 leaks/call under the poisoned allocator, verified under Guard-Malloc with no double-free), where before this fix they leaked 2 strings/call.
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.
Summary
This completes the MIR ownership drop-elaboration work: bound-string originals on consumed match roots are now discharged (fixes #2373), byte-copy field projections are classified as interior aliases (fixes #2375), owned locals are routed through a single registration authority, owned locals are retracted by disposition rather than removed outright, cow-heap drop release is carried as a typed fact instead of a bare symbol string, and sibling-field drops are compensated along multi-hop alias chains so every owned field drops exactly once.
The last piece is the multi-hop compensation: when a byte-copy field is projected through two or more
lethops and the innermost projection escapes the function (byreturnor into another record), the owner's whole-tree composite drop is correctly excluded to avoid a double-free — but the sibling fields along that chain (e.g.mid.x,outer.c) need their own compensating drop, since nothing else frees them.Builder::alias_projection_chainnow keeps the intermediate structure of each alias hop, andcompute_escaped_record_chain_sibling_dropswalks it to emit exactly oneFieldDropInPlaceper owned field that isn't on the escaping path.Verification
returnand into a holder record, dual-escape, field aliased-and-read both before and after the escape point).nested_projection_chain_leak_oracle.rs(escaped_return_record_leak_slope_below_tolerance,escaped_into_record_leak_slope_below_tolerance) — red on the pre-fix commit, green after.Out of scope
escaped_return_tuple, e.g.let pair = o.pair; let leaf = pair.0;) — the tuple composite prover is a separate code path this branch doesn't touch. Leak count is unchanged before and after this branch.matchpattern instead of a plain field projection (let leaf = match mid { Mid { leaf, x: _ } => leaf }; return leaf;). Reproduces identically onorigin/mainand on both the pre- and post-fix commits of this branch — the composite-drop prover's alias provenance tracking doesn't recognize a match-destructured binder the way it recognizes a plain field load, so the owner's composite drop is never excluded and the returned value is freed twice.Both are real, pre-existing bugs distinct from the two this PR fixes; tracking them as separate follow-ups.
Fixes #2373
Fixes #2375