Skip to content

blackhole: find_catch_after_resume_live skips an unbounded prefix before the trailing -live- #1151

Description

@youknowone

Summary

BlackholeInterpreter::find_catch_after_resume_live
(majit/majit-metainterp/src/blackhole.rs:1377) skips an unbounded number
of startpoints before it looks for the trailing -live-. Its caller comment
asserts a bound that the loop does not impose, so the catch_exception it
returns is not necessarily the one belonging to the operation that raised.

The asserted invariant vs. the code

The caller (handle_exception_in_frame, blackhole.rs:1216-1224) says:

Ops before the trailing live belong to that translated operation. Once the
live is crossed, only its immediately-following catch is valid.

The second sentence is enforced — after crossed_trailing_live is set, the
very next startpoint must be a catch_exception or the scan gives up. The first
sentence is not enforced by anything in the loop:

let mut crossed_trailing_live = false;
for q in points {                       // every startpoint after resume_live_pos
    let op = code[q];
    if op == self.op_catch_exception {
        return crossed_trailing_live.then_some(q);
    }
    if crossed_trailing_live {
        return None;
    }
    if op == self.op_live {
        crossed_trailing_live = true;   // first `-live-` seen, wherever it is
    }
}

Before the first -live- the loop consumes any number of ops of any kind, with
no block boundary and no distance bound. If the raising operation's own block
has no trailing -live-/catch_exception pair, the scan walks into whatever
follows and can return a catch_exception that belongs to a different
translated operation. route_to_catch then stores the exception and jumps to
that unrelated handler.

Relationship to #174

#174 (closed) covers the producer side: the flattener maintains "one can-raise
op per block, catch_exception adjacent to its trailing -live-" by a hoist
plus a debug_assert! that is compiled out in release. This is the consumer
side of the same invariant — even granting the per-block guarantee, the scan
here is not bounded to the block, so it does not fail closed if the producer
side ever stops holding.

Status

Raised by a Codex parity review of #1126. The behaviour above is what the code
does; I could not construct a reaching case, so whether any jitcode pyre
generates today drives the scan past its intended operation is unproven.
Recording it rather than changing the scan, because narrowing it without a
reproducer risks turning a currently-caught exception into an uncaught one.

A cheap first step, if someone wants to close the gap without a reproducer, is
to promote the flattener's debug_assert! to a release check, or bound the
pre--live- skip by the raising op's own block extent.

commented by Claude

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions