is_active can be unsound when the activation does not dominate the reservation #160598

The remainder of this here might be complete slop. I tried to check it for accuracy but it's way beyond me. But in case it's useful for someone, I'm providing my LLM's explanation of why it thinks this bug happened. (To be clear, I've personally verified that I believe this is a bug. I have not checked this explanation and am providing it only in the hope that it may be useful. Please disregard if it's not---and let me know and I can not provide them in future issues!)

Summary: rustc_borrowck::path_utils::is_active assumes a two-phase borrow's activation post-dominates its reservation. A break inside a method argument violates that, and a later &place is misclassified as reading a "mere reservation" while an activated &mut is live.

Reproducer

#![forbid(unsafe_code)]
fn main() {
   let mut v = Some(0i32);
   let mut r: Option<&mut i32> = None;
   'b: {
       // R: two-phase `&mut v` is reserved for the receiver, then the
       // argument is evaluated — and it may `break 'b` before the call (A).
       r = Some(v.insert(match true { false => break 'b, true => 1 }));
   }
   let s = &v;          // accepted; should be E0502
   let before = *s;
   *r.unwrap() = 2;     // write through the activated &mut while `s` is live
   println!("{:?} {:?}", before, *s);   // prints: Some(1) Some(2)
}

A &Option<i32> observes its referent change. Miri (Tree Borrows) reports Undefined Behavior: write access through <tag> … is forbidden. The same shape works with loop { v.insert({ if c { break … } 5 }) }, HashMap::entry, etc. The break edge doesn't need to be taken at runtime — its presence in the CFG is enough.

What goes wrong

is_active decides whether a two-phase loan is still a mere reservation at location using only dominators:

if activation_location.dominates(location, dominators) { return true; }
let reserve_location = borrow_data.reserve_location.successor_within_block();
if reserve_location.dominates(location, dominators) { false } else { true }

The comment above it states the assumption this relies on:

• the reservation R dominates the activation A
• the activation A post-dominates the reservation R (ignoring unwinding edges).

This means that there can't be an edge that leaves A and comes back into that diamond unless it passes through R.

The second bullet is false. Two-phase borrows exist so the autoref'd receiver is reserved before the remaining arguments are evaluated, and an argument can break / break 'label out of the enclosing loop or block. That edge runs from between R and A to a join point J outside, so A does not post-dominate R.

At J (let s = &v above):

• succ(R) dominates J — every path to J passes the reservation;
• A does not dominate J — the break 'b path skips the call.

So is_active returns false, even though J is also reached from A along the fall-through path carrying the activated &mut in r. The caller in check_access_for_conflict then skips the conflict:

(Read(kind), BorrowKind::Mut { .. }) => {
   // Reading from mere reservations of mutable-borrows is OK.
   if !is_active(this.dominators(), borrow, location) {
       assert!(borrow.kind.is_two_phase_borrow());
       return ControlFlow::Continue(());
   }

(loan_invalidations.rs has the identical check.)

The Borrows dataflow is fine — the loan is in scope at J; only the "merely reserved?" refinement is wrong. Two controls confirm that:

• replace the read &v with a write v = None → E0506 (writes never consult is_active);
• hoist the break above the call so A post-dominates R again → E0502 at let s = &v.

Fix direction

"Reserved but not active at p" should mean p is not reachable from A without passing back through R. succ(R) dom p ∧ ¬(A dom p) is only equivalent to that under the post-dominance assumption. Either:

1. replace the second test with a real reachability check — active at p iff p.block is reachable from A.block without entering R.block (and p in R.block after R ⇒ inactive, which keeps v.push(v.len()) cheap); or
2. precompute the reserved-only region per two-phase borrow during BorrowSet construction (forward walk from R stopping at A) and have is_active consult it.

Both only flip answers from false to true, so nothing new is accepted. Ordinary two-phase patterns (v.push(v.len()), if/match/nested calls in arguments, arguments that return/panic!) are unaffected: either A still post-dominates R, or the early exit diverges and no join point sees both paths.