diff --git a/.github/workflows/pyre-ci.yml b/.github/workflows/pyre-ci.yml index d421b33d0ad..2f1db580cc9 100644 --- a/.github/workflows/pyre-ci.yml +++ b/.github/workflows/pyre-ci.yml @@ -726,8 +726,12 @@ jobs: # the wasm32 target above; WASM_MODULE_PATH is the .wasm-host.wasm the test # loads), so run the ignored tests against them here. The plain # `cargo test --all` job cannot: it never builds those artifacts. + # + # Serialized: each test spawns a full pyre process on both backends, and + # the default one-thread-per-core fan-out ran them concurrently. Two runs + # then lost a runner with an empty stderr, in a different test each time. if: runner.os == 'Linux' - run: cargo test -p majit-backend-wasm --test codegen_test -- --ignored + run: cargo test -p majit-backend-wasm --test codegen_test -- --ignored --test-threads=1 - name: Run release-only dispatcher-graph acceptance test (Linux only) # `slow_generated_jitcodes_preserve_complete_dispatcher_graph` self-ignores # under `debug_assertions`, so the `cargo test --all` job never runs it: it diff --git a/majit/majit-backend/src/resume_guard_descr.rs b/majit/majit-backend/src/resume_guard_descr.rs index c058c04e976..fb1ad6d0896 100644 --- a/majit/majit-backend/src/resume_guard_descr.rs +++ b/majit/majit-backend/src/resume_guard_descr.rs @@ -264,6 +264,11 @@ pub struct ResumeGuardDescr { /// longer depends on the guard's per-trace fail index. `0` = not a /// range guard. pub range_foriter_key: AtomicU64, + /// Pyre-only: FOR_ITER green key for guards emitted while inlining a user + /// instance's `__next__`. A bridge from one of these guards must retain + /// the generic `jit_next` conversion path when it re-enters FOR_ITER. + /// `0` means this descr did not originate in that inline route. + pub instance_next_foriter_key: AtomicU64, } // Safety: single-threaded JIT (RPython GIL parity). @@ -289,6 +294,12 @@ impl Descr for ResumeGuardDescr { key => Some(key), } } + fn instance_next_foriter_green_key(&self) -> Option { + match self.instance_next_foriter_key.load(Ordering::Relaxed) { + 0 => None, + key => Some(key), + } + } /// compile.py:844-846: ResumeGuardDescr.clone() fn clone_descr(&self) -> Option { Some(Arc::new(ResumeGuardDescr { @@ -316,9 +327,12 @@ impl Descr for ResumeGuardDescr { bridge_body_ptr_cache: Box::new(AtomicUsize::new(0)), bridge_dispatch_cell: AtomicPtr::new(std::ptr::null_mut()), bridge_dispatch_drop_fn: OnceLock::new(), - // The clone guards the same range site; preserve the tag so a - // cloned range class guard still demotes on failure. + // The clone guards the same FOR_ITER site; preserve either tag so + // guard-failure routing survives guard copying. range_foriter_key: AtomicU64::new(self.range_foriter_key.load(Ordering::Relaxed)), + instance_next_foriter_key: AtomicU64::new( + self.instance_next_foriter_key.load(Ordering::Relaxed), + ), })) } } @@ -600,6 +614,7 @@ pub fn make_resume_guard_descr_typed(types: Vec) -> DescrRef { bridge_dispatch_cell: AtomicPtr::new(std::ptr::null_mut()), bridge_dispatch_drop_fn: OnceLock::new(), range_foriter_key: AtomicU64::new(0), + instance_next_foriter_key: AtomicU64::new(0), }) } diff --git a/majit/majit-gc/src/collector.rs b/majit/majit-gc/src/collector.rs index 3f588381981..eb1d0281932 100644 --- a/majit/majit-gc/src/collector.rs +++ b/majit/majit-gc/src/collector.rs @@ -3717,10 +3717,20 @@ impl MiniMarkGC { None => { let type_info = self.types.get(type_id); let length = unsafe { *((obj_addr + type_info.length_offset) as *const usize) }; + // `set_forwarding_address` stores the new address in the word + // right after the header — `obj_addr + 0`. Every type whose + // `length_offset` is 0 therefore has its length word overwritten + // the moment it is forwarded, and `ItemsBlock` is one + // (`ITEMS_BLOCK_LEN_OFFSET` is `capacity`, its first field). A + // length that is a plausible heap address is that corpse, read + // by a path that skipped the `is_forwarded` check, not an + // uninitialized allocation — name which one this is rather than + // leaving both readings open. + let forwarded = unsafe { (*header_of(obj_addr)).is_forwarded() }; panic!( "GC BUG: varsize length describes no allocation: length={} (read at \ obj_addr={:#x} + length_offset={}) item_size={} fixed_size={} \ - type_id={} header_addr={:#x} nursery_start={:#x} site={}", + type_id={} header_addr={:#x} nursery_start={:#x} forwarded={} site={}", length, obj_addr, type_info.length_offset, @@ -3729,6 +3739,7 @@ impl MiniMarkGC { type_id, obj_addr - GcHeader::SIZE, self.nursery.start_ptr() as usize, + forwarded, site, ); } diff --git a/majit/majit-gc/src/header.rs b/majit/majit-gc/src/header.rs index 54e9508c0b0..ce4a756e28d 100644 --- a/majit/majit-gc/src/header.rs +++ b/majit/majit-gc/src/header.rs @@ -96,6 +96,12 @@ impl GcHeader { /// `hdr + SIZE` lies outside the single-field extent a `&mut GcHeader` /// reference is allowed to touch under Rust's aliasing model. /// + /// `hdr + SIZE` is the object's own first payload word, so a varsize type + /// registered with `length_offset == 0` — `ItemsBlock`, whose `capacity` + /// is its first field — has its length destroyed here. Any size read of a + /// forwarded object of such a type returns the forwarding address in place + /// of the length; check `is_forwarded` and follow it first. + /// /// # Safety /// `hdr` must point to a valid `GcHeader` followed by at least /// `size_of::()` bytes of writable memory, and no other reference diff --git a/majit/majit-ir/src/descr.rs b/majit/majit-ir/src/descr.rs index 64c52004955..43ad4efe40b 100644 --- a/majit/majit-ir/src/descr.rs +++ b/majit/majit-ir/src/descr.rs @@ -3599,6 +3599,14 @@ pub trait Descr: Send + Sync + std::fmt::Debug { .and_then(|prev| prev.range_foriter_green_key()) } + /// Pyre-only: the FOR_ITER green key whose user-instance `__next__` + /// inline emitted this guard, or `None`. Copied guards chase their donor + /// descr exactly like [`Descr::range_foriter_green_key`]. + fn instance_next_foriter_green_key(&self) -> Option { + self.prev_descr() + .and_then(|prev| prev.instance_next_foriter_green_key()) + } + /// intbounds.py: descr.is_integer_bounded() / get_integer_min/max. /// Returns (field_size_bytes, is_signed) if this is a field descriptor. /// Used by intbounds to narrow GETFIELD result bounds. diff --git a/majit/majit-ir/src/effectinfo.rs b/majit/majit-ir/src/effectinfo.rs index e8443073d64..37c0cdc2112 100644 --- a/majit/majit-ir/src/effectinfo.rs +++ b/majit/majit-ir/src/effectinfo.rs @@ -935,7 +935,10 @@ pub enum PyreHelperKind { /// raise instead of declining to the trait. RaiseVarargs, /// `get_current_exception()` — the PUSH_EXC_INFO `prev = ec.sys_exc_value` - /// save residual (`() → Ref`, TLS read via `cpu.get_current_exception_fn`). + /// save residual, and the read a catch-covered bare `raise` uses to obtain + /// the exception it re-raises (`() → Ref`, TLS read via + /// `cpu.get_current_exception_fn`). Only the save is followed by a store + /// and a matching POP_EXCEPT restore. /// The full-body walker recognises this tag to /// lower it to `GETFIELD_GC_R(ec, sys_exc_value)` so the exc-info save /// participates in the balanced save/restore the heap optimizer @@ -958,6 +961,9 @@ pub enum PyreHelperKind { /// the in-flight iteration to the live frame instead of dropping it (the /// iterator advance is an irreversible side effect with no journal undo). ForIterNext, + /// `jit_exception_match(exc, match_class)` — the infallible Python-level + /// exception MRO test used by FOR_ITER's materialized catch arm. + ForIterExceptionMatch, /// `get_iter(obj)` — the GET_ITER residual (`iter(obj)`). The full-body /// walker recognises exact machine-word `range` objects and emits the /// virtual `W_IntRangeIterator` allocation shape directly. @@ -1054,9 +1060,11 @@ pub enum PyreHelperKind { MakeFunction, /// `bh_clear_in_flight_exception()` — the `[] -> void` residual emitted by /// PUSH_EXC_INFO to complete the caught-exception ownership transfer. The - /// full-body walker recognises this tag to keep the executed-effect - /// odometer off it: the written slot is a GC-liveness root with no value - /// reader, so a non-committing walk has nothing to undo. + /// full-body walker applies the concrete clear during its authoritative + /// walk and emits no runtime IR: compiled traceback construction never + /// publishes this interpreter-only GC-liveness carrier. Generic fallback + /// also keeps the executed-effect odometer off it because the slot has no + /// value reader and a non-committing walk has nothing to undo. ClearInFlightException, } diff --git a/majit/majit-metainterp/src/compile.rs b/majit/majit-metainterp/src/compile.rs index 1ae23f264a5..9da737b8fef 100644 --- a/majit/majit-metainterp/src/compile.rs +++ b/majit/majit-metainterp/src/compile.rs @@ -3242,6 +3242,7 @@ pub fn make_fail_descr_with_index(fail_index: u32, num_live: usize) -> DescrRef bridge_dispatch_cell: AtomicPtr::new(std::ptr::null_mut()), bridge_dispatch_drop_fn: OnceLock::new(), range_foriter_key: AtomicU64::new(0), + instance_next_foriter_key: AtomicU64::new(0), }) } @@ -3335,6 +3336,7 @@ pub fn make_resume_guard_descr_typed(types: Vec) -> DescrRef { bridge_dispatch_cell: AtomicPtr::new(std::ptr::null_mut()), bridge_dispatch_drop_fn: OnceLock::new(), range_foriter_key: AtomicU64::new(0), + instance_next_foriter_key: AtomicU64::new(0), }) } @@ -3357,6 +3359,52 @@ pub fn make_resume_guard_descr_range_foriter(green_key: u64) -> DescrRef { descr } +/// Tag a guard emitted while inlining a user instance's `__next__` with the +/// caller FOR_ITER key. A guard-failure bridge uses the tag to keep the +/// exception-to-exhaustion conversion on the generic residual path. +/// +/// `opcode` selects the subtype `compile.py:924-942 invent_fail_descr_for_op` +/// would have minted. Stamping this descr fills `op.getdescr()`, and +/// `store_final_boxes_in_guard` only invents on the empty arm — so a marker +/// minted as a plain `ResumeGuardDescr` would cost a `GUARD_NOT_FORCED` its +/// `is_guard_forced()` (which vetoes bridge compilation) or a +/// `GUARD_NO_EXCEPTION` its `is_guard_exc()` (which routes the pending +/// exception). The whole inlined `__next__` body is tagged, residual guards +/// included, so both opcodes reach here. +pub fn make_resume_guard_descr_instance_next_foriter( + opcode: Option, + green_key: u64, +) -> DescrRef { + let descr = match opcode { + Some(OpCode::GuardNotForced | OpCode::GuardNotForced2) => { + make_resume_guard_forced_descr_typed(Vec::new()) + } + Some(OpCode::GuardException | OpCode::GuardNoException) => { + make_resume_guard_exc_descr_typed(Vec::new()) + } + _ => make_resume_guard_descr_typed(Vec::new()), + }; + resume_guard_inner(&descr) + .expect("every arm above constructs a ResumeGuardDescr or a newtype over one") + .instance_next_foriter_key + .store(green_key, Ordering::Relaxed); + descr +} + +/// The `ResumeGuardDescr` inside a descr that either is one or is one of its +/// tag-only newtypes. +fn resume_guard_inner(descr: &DescrRef) -> Option<&ResumeGuardDescr> { + let any = descr.as_any()?; + if let Some(plain) = any.downcast_ref::() { + return Some(plain); + } + if let Some(forced) = any.downcast_ref::() { + return Some(&forced.inner); + } + any.downcast_ref::() + .map(|exc| &exc.inner) +} + /// compile.py:892: ResumeAtPositionDescr(ResumeGuardDescr) — subclass /// with no additional fields or method overrides. Type tag only. /// @@ -3616,6 +3664,7 @@ pub fn make_resume_at_position_descr_typed(types: Vec) -> DescrRef { bridge_dispatch_cell: AtomicPtr::new(std::ptr::null_mut()), bridge_dispatch_drop_fn: OnceLock::new(), range_foriter_key: AtomicU64::new(0), + instance_next_foriter_key: AtomicU64::new(0), }, }) } @@ -3665,6 +3714,18 @@ impl majit_ir::Descr for ResumeGuardForcedDescr { fn is_resume_guard(&self) -> bool { true } + /// Subclassing in RPython keeps the base attributes readable; a Rust + /// newtype only exposes what it forwards, and the default accessor walks + /// `prev_descr`, which a wrapper does not have. Forward both walker + /// marker keys explicitly: the FOR_ITER routes key failure handling on + /// them, and `store_final_boxes_in_guard` re-mints a marked descr on + /// unroll's second emission only while it can still read the key. + fn range_foriter_green_key(&self) -> Option { + self.inner.range_foriter_green_key() + } + fn instance_next_foriter_green_key(&self) -> Option { + self.inner.instance_next_foriter_green_key() + } /// compile.py:873-876 ResumeGuardDescr.clone() — `ResumeGuardForcedDescr` /// inherits the base implementation (no override at compile.py:939+), /// so cloning produces a plain `ResumeGuardDescr` with resume attributes @@ -3884,6 +3945,7 @@ pub fn make_resume_guard_forced_descr_typed(types: Vec) -> DescrRef { bridge_dispatch_cell: AtomicPtr::new(std::ptr::null_mut()), bridge_dispatch_drop_fn: OnceLock::new(), range_foriter_key: AtomicU64::new(0), + instance_next_foriter_key: AtomicU64::new(0), }, }) } @@ -3917,6 +3979,15 @@ impl majit_ir::Descr for ResumeGuardExcDescr { fn is_resume_guard(&self) -> bool { true } + /// The `ResumeGuardForcedDescr` reasoning applies here unchanged: a + /// newtype exposes only what it forwards, and both walker marker keys + /// have to stay readable through it. + fn range_foriter_green_key(&self) -> Option { + self.inner.range_foriter_green_key() + } + fn instance_next_foriter_green_key(&self) -> Option { + self.inner.instance_next_foriter_green_key() + } /// compile.py:881-882 `class ResumeGuardExcDescr(ResumeGuardDescr): pass` /// — no clone() override, so inheriting compile.py:873-876 /// `ResumeGuardDescr.clone()` produces a plain `ResumeGuardDescr` with @@ -4136,6 +4207,7 @@ pub fn make_resume_guard_exc_descr_typed(types: Vec) -> DescrRef { bridge_dispatch_cell: AtomicPtr::new(std::ptr::null_mut()), bridge_dispatch_drop_fn: OnceLock::new(), range_foriter_key: AtomicU64::new(0), + instance_next_foriter_key: AtomicU64::new(0), }, }) } @@ -5132,6 +5204,7 @@ impl majit_ir::Descr for CompileLoopVersionDescr { bridge_dispatch_cell: AtomicPtr::new(std::ptr::null_mut()), bridge_dispatch_drop_fn: OnceLock::new(), range_foriter_key: AtomicU64::new(0), + instance_next_foriter_key: AtomicU64::new(0), }, })) } @@ -5353,6 +5426,7 @@ fn make_compile_loop_version_descr_with_payload(types: Vec, payload: RdPay bridge_dispatch_cell: AtomicPtr::new(std::ptr::null_mut()), bridge_dispatch_drop_fn: OnceLock::new(), range_foriter_key: AtomicU64::new(0), + instance_next_foriter_key: AtomicU64::new(0), }, }) } @@ -5874,6 +5948,7 @@ mod fail_descr_tests { bridge_dispatch_cell: AtomicPtr::new(std::ptr::null_mut()), bridge_dispatch_drop_fn: OnceLock::new(), range_foriter_key: AtomicU64::new(0), + instance_next_foriter_key: AtomicU64::new(0), }, }) as DescrRef; let lv_fi = lv.index(); @@ -5972,6 +6047,34 @@ mod fail_descr_tests { } /// compile.py:832-851 ResumeGuardCopiedDescr(prev) parity: + /// The instance-next FOR_ITER marker key must stay readable no matter + /// which subtype the guard's opcode selects. Two consumers depend on it: + /// guard-failure routing keys the FOR_ITER handling on it, and + /// `store_final_boxes_in_guard` re-mints a marked descr for unroll's + /// second emission only while it can still read the key — without that + /// re-mint the second emission finalizes an already-finalized descr and + /// trips the once-per-descr `finish()` assert. + #[test] + fn test_instance_next_marker_survives_every_guard_subtype() { + const KEY: u64 = 0xF0_1D_ED; + for opcode in [ + None, + Some(OpCode::GuardNotForced), + Some(OpCode::GuardNotForced2), + Some(OpCode::GuardException), + Some(OpCode::GuardNoException), + Some(OpCode::GuardClass), + ] { + let descr = make_resume_guard_descr_instance_next_foriter(opcode, KEY); + assert_eq!( + descr.instance_next_foriter_green_key(), + Some(KEY), + "opcode {opcode:?} minted a descr whose marker key is unreadable" + ); + assert!(descr.is_resume_guard(), "opcode {opcode:?}"); + } + } + /// `get_resumestorage()` chases to `prev`, `fail_arg_types` /// shares the donor's vector, `is_resume_guard_copied()` flags /// the subtype, and the exc variant additionally reports diff --git a/majit/majit-metainterp/src/lib.rs b/majit/majit-metainterp/src/lib.rs index c3108c6db8d..0280aa89064 100644 --- a/majit/majit-metainterp/src/lib.rs +++ b/majit/majit-metainterp/src/lib.rs @@ -149,7 +149,7 @@ pub use call_descr::{ }; pub use compile::{ make_fail_descr, make_fail_descr_typed, make_finish_fail_descr_typed, - make_resume_guard_descr_range_foriter, + make_resume_guard_descr_instance_next_foriter, make_resume_guard_descr_range_foriter, }; pub use io_buffer::{ emit_commit_io, encode_decimal_i64, io_buffer_commit, io_buffer_discard, io_buffer_write, diff --git a/majit/majit-metainterp/src/optimizeopt/mod.rs b/majit/majit-metainterp/src/optimizeopt/mod.rs index 194019e8aeb..a5bbc7c5785 100644 --- a/majit/majit-metainterp/src/optimizeopt/mod.rs +++ b/majit/majit-metainterp/src/optimizeopt/mod.rs @@ -6300,20 +6300,28 @@ impl OptContext { op.getdescr().is_some_and(|d| d.is_resume_guard_copied()) ); - // A walker-native range FOR_ITER class guard carries a pre-minted - // marker descr (`range_foriter_green_key`) so its failure can demote - // the specialization by descr identity. `Op::clone` shares the descr - // Arc, so unroll's phase-1/phase-2 emissions of the guard reach this - // function on the same, already-finalized descr. Mint a fresh marked - // descr for this emission — mirroring the `op.descr.is_none()` arm's - // fresh-per-emission descr — so the once-per-descr `finish()` - // invariant below still holds for it (and for every other guard). + // Walker-native FOR_ITER guards can carry a pre-minted range or + // user-instance-next marker descr so failure routing is keyed by descr + // identity. `Op::clone` shares the descr Arc, so unroll's + // phase-1/phase-2 emissions of the guard reach this function on the + // same, already-finalized descr. Mint a fresh marked descr for this + // emission — mirroring the `op.descr.is_none()` arm's fresh-per-emission + // descr — so the once-per-descr `finish()` invariant below still holds + // for it (and for every other guard). let refinalize_marked_key = op .getdescr() .and_then(|d| d.range_foriter_green_key()) .filter(|_| op.resolved_rd_numb().is_some()); + let refinalize_instance_next_key = op + .getdescr() + .and_then(|d| d.instance_next_foriter_green_key()) + .filter(|_| op.resolved_rd_numb().is_some()); if let Some(key) = refinalize_marked_key { op.setdescr(crate::compile::make_resume_guard_descr_range_foriter(key)); + } else if let Some(key) = refinalize_instance_next_key { + op.setdescr( + crate::compile::make_resume_guard_descr_instance_next_foriter(Some(op.opcode), key), + ); } // resume.py:397 `assert not storage.rd_numb` — finish() runs at diff --git a/majit/majit-metainterp/src/recorder.rs b/majit/majit-metainterp/src/recorder.rs index 437f7eb156a..59ddc93ecae 100644 --- a/majit/majit-metainterp/src/recorder.rs +++ b/majit/majit-metainterp/src/recorder.rs @@ -451,6 +451,46 @@ impl Trace { } } + /// Replace the descriptor on the last recorded operation. + pub fn set_last_op_descr(&mut self, descr: DescrRef) { + if let Some(op) = self.ops.last() { + op.setdescr(descr); + } + } + + /// Opcode of the op [`set_last_op_descr`](Self::set_last_op_descr) would + /// stamp, for a caller that has to mint the descr subtype the opcode + /// requires. + pub fn last_op_opcode(&self) -> Option { + self.ops.last().map(|op| op.opcode) + } + + /// Opcode of the op + /// [`set_guard_op_descr_from_end`](Self::set_guard_op_descr_from_end) + /// would stamp, selected by the same walk. + pub fn guard_op_opcode_from_end(&self, from_end: usize) -> Option { + self.ops + .iter() + .rev() + .filter(|op| op.opcode.is_guard()) + .nth(from_end) + .map(|op| op.opcode) + } + + /// Replace the descriptor on the guard `from_end` guards back from the + /// most recently recorded one. + pub fn set_guard_op_descr_from_end(&mut self, from_end: usize, descr: DescrRef) { + if let Some(op) = self + .ops + .iter() + .rev() + .filter(|op| op.opcode.is_guard()) + .nth(from_end) + { + op.setdescr(descr); + } + } + /// Opcode of the most recently recorded guard, if any. Snapshot /// capture keys `after_residual_call` on the guard opcode itself /// (`pyjitpl.py:2599-2603 generate_guard`). diff --git a/majit/majit-metainterp/src/trace_ctx.rs b/majit/majit-metainterp/src/trace_ctx.rs index 9d8c5c2e859..993852a8b1f 100644 --- a/majit/majit-metainterp/src/trace_ctx.rs +++ b/majit/majit-metainterp/src/trace_ctx.rs @@ -2219,6 +2219,22 @@ impl TraceCtx { self.recorder.last_guard_opcode() } + pub fn set_last_op_descr(&mut self, descr: DescrRef) { + self.recorder.set_last_op_descr(descr); + } + + pub fn set_guard_op_descr_from_end(&mut self, from_end: usize, descr: DescrRef) { + self.recorder.set_guard_op_descr_from_end(from_end, descr); + } + + pub fn last_op_opcode(&self) -> Option { + self.recorder.last_op_opcode() + } + + pub fn guard_op_opcode_from_end(&self, from_end: usize) -> Option { + self.recorder.guard_op_opcode_from_end(from_end) + } + /// The structured green key values, if provided. pub fn green_key_values(&self) -> Option<&GreenKey> { self.green_key_values.as_ref() diff --git a/majit/majit-translate/src/front/result_exc.rs b/majit/majit-translate/src/front/result_exc.rs index 25b33022d52..0c8000f136c 100644 --- a/majit/majit-translate/src/front/result_exc.rs +++ b/majit/majit-translate/src/front/result_exc.rs @@ -1029,10 +1029,10 @@ pub(crate) struct RewireOutcome { /// consuming. pub rewrapped: usize, /// Drain-loop `match next()` sites fused by [`try_fuse_drain_match`] - /// into an exception-edge handler with a guest-KIND test, eliminating - /// the `Result` shell's `Ok`/`StopIteration` ctors + `PyErrorKind::eq` - /// residuals. Counted separately from `rewrapped` and independent of - /// `tail_forwards` (which feeds `lower_result_exc_returns`). + /// into an exception-edge handler with an object-level StopIteration + /// subclass test, eliminating the `Result` shell's guard residuals. + /// Counted separately from `rewrapped` and independent of `tail_forwards` + /// (which feeds `lower_result_exc_returns`). pub fused: usize, } @@ -1119,12 +1119,12 @@ fn rewire_one_call_site( let Some(branch_op_idx) = branch_op_idx else { // No `Result::branch` op → a hand-written `match` consumer. The // drain-loop `match next()` fusion recognises its exact shape and - // rewrites it into an exception-edge handler with a guest-KIND test; - // every other custom-match shape (and the drain shape when any - // hazard guard trips) falls through to `catch_and_rewrap`. The - // fusion is fail-safe: an `Err` from `try_fuse_drain_match` MUST NOT - // propagate (that would decline the whole graph); it converts here - // into the existing rewrap path. + // rewrites it into an exception-edge handler with an object-level + // StopIteration test; every other custom-match shape (and the drain + // shape when any hazard guard trips) falls through to + // `catch_and_rewrap`. The fusion is fail-safe: an `Err` from + // `try_fuse_drain_match` MUST NOT propagate (that would decline the + // whole graph); it converts here into the existing rewrap path. if try_fuse_drain_match(graph, a, r).is_ok() { return Ok(SiteOutcome::Fused); } @@ -1561,20 +1561,16 @@ fn verify_drain_reraise_returns_err_payload( /// ```text /// match next(w_iterator) { /// Ok(w_item) => append(items, w_item), -/// Err(e) if e.kind == PyErrorKind::StopIteration => break, +/// Err(e) if e.matches_stop_iteration() => break, /// Err(e) => return Err(e), /// } /// ``` /// which lowers to a materialised `Result<*mut PyObject, PyError>` shell: -/// a `__discriminant` switch whose arms build a `StopIteration` -/// `SyntheticTransparentCtor`, read `__pos_0[Result::Err]`, and call -/// `PyErrorKind::eq` — niladic ctors with no host symbol the jd1 walker -/// SIGBUSes on. This rewrites the `next()` block into `LastException` -/// exits (normal → the `Ok` arm; exception → a handler `H`) whose handler -/// runs the **guest-KIND** test `w_exception_get_kind(evalue) == 10` -/// (`ExcKind::StopIteration`) — the exact source semantics -/// `e.kind == PyErrorKind::StopIteration` through the -/// `ExcKind ↔ PyErrorKind` bijection, NOT an MRO/subclass match. +/// a `__discriminant` switch whose Err arm reads `__pos_0[Result::Err]` +/// and calls `PyError::matches_stop_iteration`. This rewrites the `next()` +/// block into `LastException` exits (normal → the `Ok` arm; exception → a +/// handler `H`) whose handler calls the equivalent object-level predicate on +/// the live exception value, preserving the MRO/subclass match. /// /// Fail-safe: returns `Err` on ANY structural mismatch or hazard, and the /// caller ([`rewire_one_call_site`]) converts that into `catch_and_rewrap` @@ -1680,47 +1676,12 @@ fn try_fuse_drain_match(graph: &mut FunctionGraph, a: usize, r: &Variable) -> Re // payload directly). Record its payload position on the Ok link. assert_single_pred(graph, ok_target, &name)?; - // --- (5) Err arm: single predecessor, EXACTLY the four guard ops - // (StopIteration ctor, Err payload read, kind read, PyErrorKind::eq). + // --- (5) Err arm: single predecessor, EXACTLY the two guard ops + // (Err payload read, PyError::matches_stop_iteration call). assert_single_pred(graph, err_target, &name)?; let r_err = forward_alias(graph, &r_b, &err_link) .ok_or_else(|| format!("{name}: drain fuse: Err link drops the Result value"))?; let err_ops = &graph.blocks[err_target].operations; - // `PyErrorKind::StopIteration` reaches the `eq` below in either of two - // lowered forms: as a niladic `SyntheticTransparentCtor` while the - // fieldless variant is still carried as an ADT constructor, or as a plain - // `ConstInt` once the fieldless enum lowers to its discriminant. Accept - // both. The constant form is value-checked, so a comparison against a - // different kind (`e.kind == PyErrorKind::ValueError`) can never be fused - // into a StopIteration test; the operand is additionally pinned by the - // `PyErrorKind::eq` argument check below. - // - // 9 == `PyErrorKind::StopIteration` (pyre-interpreter error.rs); like the - // `ExcKind::StopIteration` 10 used by the synthesised handler, the two are - // coupled — renumbering that variant makes this recognizer decline, which - // `unpackiterable_drain_match_fuses_to_kind_test` reports as a failure. - const PYERRORKIND_STOP_ITERATION: i64 = 9; - let (ctor_idx, sc) = err_ops - .iter() - .enumerate() - .find_map(|(i, op)| { - let is_stop_iteration = match &op.kind { - OpKind::Call { - target: - CallTarget::SyntheticTransparentCtor { - name: n, - owner_path, - }, - .. - } => n == "StopIteration" && owner_path.last().is_some_and(|s| s == "PyErrorKind"), - OpKind::ConstInt(v) => *v == PYERRORKIND_STOP_ITERATION, - _ => false, - }; - is_stop_iteration.then(|| op.result.clone().map(|s| (i, s)))? - }) - .ok_or_else(|| { - format!("{name}: drain fuse: Err arm lacks the StopIteration ctor or discriminant") - })?; let (errpay_idx, err_payload) = err_ops .iter() .enumerate() @@ -1738,73 +1699,46 @@ fn try_fuse_drain_match(graph: &mut FunctionGraph, a: usize, r: &Variable) -> Re _ => None, }) .ok_or_else(|| format!("{name}: drain fuse: Err arm lacks the Err __pos_0 read"))?; - let (kind_idx, kind_var) = err_ops - .iter() - .enumerate() - .find_map(|(i, op)| match &op.kind { - OpKind::FieldRead { base, field, .. } - if *base == err_payload - && field.name == "kind" - && field.owner_root.as_deref() == Some("PyError") => - { - op.result.clone().map(|k| (i, k)) - } - _ => None, - }) - .ok_or_else(|| format!("{name}: drain fuse: Err arm lacks the PyError.kind read"))?; - let (eq_idx, eq_result) = err_ops + let (predicate_idx, predicate_result) = err_ops .iter() .enumerate() .find_map(|(i, op)| match &op.kind { OpKind::Call { target: CallTarget::Method { - name: m, + name: method, receiver_root, .. }, args, .. - } if m == "eq" - && receiver_root.as_deref() == Some("PyErrorKind") - && args.len() == 2 - && args.contains(&kind_var) - && args.contains(&sc) => + } if method == "matches_stop_iteration" + && receiver_root.as_deref() == Some("PyError") + && args.as_slice() == std::slice::from_ref(&err_payload) => { - op.result.clone().map(|m| (i, m)) - } - // The derived `PyErrorKind::eq` between two fieldless enums lowers - // to a discriminant `BinOp { op: "eq" }` (both operands sit in the - // int bank), the twin of the `sc` ConstInt form handled above. Pin - // the operands to exactly `{kind_var, sc}` in either order — as - // strict as the `Method` `args.contains` pair — so a comparison - // against any other kind can never fuse into a StopIteration test. - OpKind::BinOp { - op: binop, - lhs, - rhs, - .. - } if binop == "eq" - && ((*lhs == kind_var && *rhs == sc) || (*lhs == sc && *rhs == kind_var)) => - { - op.result.clone().map(|m| (i, m)) + op.result.clone().map(|matched| (i, matched)) } _ => None, }) - .ok_or_else(|| format!("{name}: drain fuse: Err arm lacks the PyErrorKind::eq call"))?; - // The ctor and eq are Calls (not `is_pure_op`), so they must be among - // the recognized indices, not merely tolerated. + .ok_or_else(|| { + format!("{name}: drain fuse: Err arm lacks PyError::matches_stop_iteration") + })?; + if err_ops.len() != 2 || errpay_idx >= predicate_idx { + return Err(format!( + "{name}: drain fuse: Err arm is not exactly payload-read then StopIteration predicate" + )); + } assert_block_pure_besides( graph, err_target, - &[ctor_idx, errpay_idx, kind_idx, eq_idx], + &[errpay_idx, predicate_idx], "Err arm", &name, )?; - // --- (6) Err arm → bool-switch block: `m2 = bool(eq)`, `exitswitch == + // --- (6) Err arm → bool-switch block: `m2 = bool(predicate)`, `exitswitch == // Value(m2)`, pure besides, single predecessor. - let (bswitch, eq_bs) = follow_single_exit(graph, err_target, &eq_result) + let (bswitch, predicate_bs) = follow_single_exit(graph, err_target, &predicate_result) .map_err(|e| format!("{name}: drain fuse: Err arm exit: {e}"))?; assert_single_pred(graph, bswitch, &name)?; let (bool_idx, bool_temp) = graph.blocks[bswitch] @@ -1812,17 +1746,19 @@ fn try_fuse_drain_match(graph: &mut FunctionGraph, a: usize, r: &Variable) -> Re .iter() .enumerate() .find_map(|(i, op)| match &op.kind { - OpKind::UnaryOp { op: o, operand, .. } if o == "bool" && *operand == eq_bs => { + OpKind::UnaryOp { op: o, operand, .. } if o == "bool" && *operand == predicate_bs => { op.result.clone().map(|m| (i, m)) } _ => None, }) - .ok_or_else(|| format!("{name}: drain fuse: bool-switch block {bswitch} lacks bool(eq)"))?; + .ok_or_else(|| { + format!("{name}: drain fuse: bool-switch block {bswitch} lacks bool(predicate)") + })?; match &graph.blocks[bswitch].exitswitch { Some(ExitSwitch::Value(v)) if *v == bool_temp => {} other => { return Err(format!( - "{name}: drain fuse: block {bswitch} exitswitch {other:?} is not the eq bool switch" + "{name}: drain fuse: block {bswitch} exitswitch {other:?} is not the predicate bool switch" )); } } @@ -1955,9 +1891,9 @@ fn try_fuse_drain_match(graph: &mut FunctionGraph, a: usize, r: &Variable) -> Re // --- Build the break edge (H → break-target) args, resolving each // original break-link value back toward A scope. Values defined in the // detached B / Err-arm / bool-switch blocks decline unless they are a - // const-justifiable temp: the eq bool (true on the matched arm) and the - // Err-arm discriminant (1). The Result value `r` and any other detached - // temp decline — they are not available on the exception edge. + // const-justifiable temp: the predicate bool (true on the matched arm) and + // the Err-arm discriminant (1). The Result value `r` and any other + // detached temp decline — they are not available on the exception edge. // `forwarded` collects the DISTINCT A-scope loop-carried vars the break // edge needs; each becomes a forwarded inputarg of H. // A break-arg resolution: either a constant, or an A-scope var to forward. @@ -1973,7 +1909,7 @@ fn try_fuse_drain_match(graph: &mut FunctionGraph, a: usize, r: &Variable) -> Re }; // Hop bswitch → Err-arm. The exitswitch bool temp is the only // bswitch-defined value; the break arm never carries it, but guard - // it just in case (matched arm ⟹ eq true). + // it just in case (matched arm ⟹ predicate true). if *x == bool_temp { return Ok(BreakArg::Const(LinkArg::Const(Constant::new( ConstValue::Bool(true), @@ -1998,14 +1934,14 @@ fn try_fuse_drain_match(graph: &mut FunctionGraph, a: usize, r: &Variable) -> Re return Ok(BreakArg::Const(LinkArg::Const(c.clone()))); }; let y = y.clone(); - // Hop Err-arm → B. Err-arm-defined values (the four guard ops) - // decline, except the eq bool (matched arm ⟹ true). - if y == eq_result { + // Hop Err-arm → B. Err-arm-defined values (the two guard ops) + // decline, except the predicate bool (matched arm ⟹ true). + if y == predicate_result { return Ok(BreakArg::Const(LinkArg::Const(Constant::new( ConstValue::Bool(true), )))); } - if y == sc || y == err_payload || y == kind_var { + if y == err_payload { return Err(format!( "{name}: drain fuse: break edge carries a detached Err-arm temp (dead `Err(e)` re-bind)" )); @@ -2135,41 +2071,24 @@ fn try_fuse_drain_match(graph: &mut FunctionGraph, a: usize, r: &Variable) -> Re let (r_id, r_inputs) = graph.create_block_with_arg_vars(1); let r_vb = r_inputs[0].clone(); - // H: `k = exc_kind_discriminant(vb)`; `cmp = (k == 10)`. `set_branch` - // below wraps `cmp` in the `bool` hop the switch condition expects. - let k = graph + // H: run the object-level StopIteration predicate on `vb`. `set_branch` + // below wraps the result in the `bool` hop the switch condition expects. + let matched = graph .push_op_var( h_id, OpKind::Call { target: CallTarget::function_path([ - "pyre_object", - "interp_exceptions", - "exc_kind_discriminant", + "pyre_interpreter", + "error", + "exception_object_matches_stop_iteration", ]), args: vec![h_vb.clone()], result_ty: ValueType::Int, }, true, ) - .expect("exc_kind_discriminant produces a value"); - // 10 == `ExcKind::StopIteration` (pyre-object interp_exceptions.rs); the two - // are coupled — renumbering that discriminant silently breaks this test. - let c10 = graph - .push_op_var(h_id, OpKind::ConstInt(10), true) - .expect("ConstInt produces a value"); - let cmp = graph - .push_op_var( - h_id, - OpKind::BinOp { - op: "eq".to_string(), - lhs: k, - rhs: c10, - result_ty: ValueType::Int, - }, - true, - ) - .expect("BinOp eq produces a value"); - // GAP#4: the kind test reads only `vb`; the `etype` slot must stay unused + .expect("exception_object_matches_stop_iteration produces a value"); + // GAP#4: the predicate reads only `vb`; the `etype` slot must stay unused // so the exception edge may thread the caught type in without a live // consumer (H is freshly built here, so this is a construction invariant). debug_assert!( @@ -2210,12 +2129,12 @@ fn try_fuse_drain_match(graph: &mut FunctionGraph, a: usize, r: &Variable) -> Re } } - // H: branch on the kind test — `cmp` true (kind == StopIteration) → break - // target; false → R (reraise `raise vb`). `set_branch` wraps `cmp` in - // `bool` and installs arity-checked links (MUST-ADD#2). + // H: branch on the object-level predicate — true → break target; false → + // R (reraise `raise vb`). `set_branch` wraps `matched` in `bool` and + // installs arity-checked links (MUST-ADD#2). graph.set_branch( h_id, - cmp, + matched, BlockId(break_target), break_vars, r_id, diff --git a/majit/majit-translate/tests/test_result_exc_lowering.rs b/majit/majit-translate/tests/test_result_exc_lowering.rs index a8c2f47dff2..b64b3fbfca4 100644 --- a/majit/majit-translate/tests/test_result_exc_lowering.rs +++ b/majit/majit-translate/tests/test_result_exc_lowering.rs @@ -179,24 +179,21 @@ fn execute_wrapper_family_lowers_to_raise_links() { /// Facet A firing guard — the jd1 drain-loop `match next()` fusion. /// /// `_unpackiterable_unknown_length`'s StopIteration drain loop is a -/// hand-written `match next() { Ok(w) => append, Err(e) if e.kind == -/// StopIteration => break, Err(e) => return Err(e) }`. Lowered naively it -/// materialises a `Result` shell whose Err arm holds a `StopIteration` -/// `SyntheticTransparentCtor` + a `PyErrorKind::eq` — residuals with no host -/// funcptr that SIGBUS the jd1 walk. `try_fuse_drain_match` -/// (`front::result_exc`) replaces that shell with a `LastException` -/// exception-edge whose handler is the exact-kind test -/// `exc_kind_discriminant(evalue) == 10` (`ExcKind::StopIteration`). +/// hand-written `match next() { Ok(w) => append, Err(e) if +/// e.matches_stop_iteration() => break, Err(e) => return Err(e) }`. Lowered +/// naively it materialises a `Result` shell and leaves the PyError predicate +/// on its Err arm. `try_fuse_drain_match` (`front::result_exc`) replaces that +/// shell with a `LastException` exception-edge whose handler runs the +/// equivalent object-level MRO predicate on the live exception value. /// /// The fusion is FAIL-SAFE: on any shape it does not recognise it silently -/// falls back to `catch_and_rewrap`, which leaves the `StopIteration` ctor in -/// place. That silent decline is invisible to the correctness suite — the -/// default (non-jd1) run never executes the fusion, and the -/// `unpack_drain_exact_kind` parity test only guards the default path — yet it -/// reintroduces the unwalkable ctors and reopens the jd1 SIGBUS. A drain -/// rework that perturbs the recognised shape (or a recognizer regression) is -/// exactly such a silent decline. This lowers the REAL drain and asserts the -/// fused signature is present and the ctor is gone, so a decline fails loud. +/// falls back to `catch_and_rewrap`, leaving the source predicate in place. +/// That silent decline is invisible to the default (non-jd1) drain path yet +/// reintroduces the Result shell the jd1 walk cannot consume. A drain rework +/// that perturbs the recognised shape (or a recognizer regression) is exactly +/// such a silent decline. This lowers the real drain and asserts the fused +/// helper signature is present and the source-method residual is gone, so a +/// decline fails loud. #[test] fn unpackiterable_drain_match_fuses_to_kind_test() { let llbc = interp(); @@ -206,10 +203,10 @@ fn unpackiterable_drain_match_fuses_to_kind_test() { ) .expect("lower _unpackiterable_unknown_length"); - // Positive firing signal: the fusion synthesises the exc_kind_discriminant - // kind-test call — the only site in the tree that emits this FunctionPath, - // so its presence proves `try_fuse_drain_match` fired (not declined). - let exc_kind_calls = graph + // Positive firing signal: only the fusion emits this object-level helper + // FunctionPath, so its presence proves `try_fuse_drain_match` fired rather + // than declining to catch_and_rewrap. + let object_predicate_calls = graph .blocks .iter() .flat_map(|b| b.operations.iter()) @@ -217,20 +214,21 @@ fn unpackiterable_drain_match_fuses_to_kind_test() { matches!( &op.kind, OpKind::Call { target: CallTarget::FunctionPath { segments }, .. } - if segments.last().map(String::as_str) == Some("exc_kind_discriminant") + if segments.last().map(String::as_str) + == Some("exception_object_matches_stop_iteration") ) }) .count(); assert!( - exc_kind_calls >= 1, - "drain fusion must synthesise the exc_kind_discriminant kind-test \ + object_predicate_calls >= 1, + "drain fusion must synthesise the object-level StopIteration predicate \ (0 = recognizer silently declined to catch_and_rewrap → the \ - StopIteration ctor/eq residuals remain and SIGBUS the jd1 walk)" + Result shell and source predicate remain on the jd1 walk)" ); - // Elimination signal: the StopIteration guard ctor survives ONLY on the - // decline (catch_and_rewrap) path, so a fired fusion leaves none. - let stopiteration_ctors = graph + // Elimination signal: the source PyError method survives only on the + // decline path, so a fired fusion leaves none. + let source_predicate_calls = graph .blocks .iter() .flat_map(|b| b.operations.iter()) @@ -238,15 +236,32 @@ fn unpackiterable_drain_match_fuses_to_kind_test() { matches!( &op.kind, OpKind::Call { - target: CallTarget::SyntheticTransparentCtor { name, .. }, + target: CallTarget::Method { name, .. }, .. - } if name == "StopIteration" + } if name == "matches_stop_iteration" + ) + }) + .count(); + assert_eq!( + source_predicate_calls, 0, + "the source PyError predicate must be gone after the drain fusion" + ); + + let exc_kind_calls = graph + .blocks + .iter() + .flat_map(|b| b.operations.iter()) + .filter(|op| { + matches!( + &op.kind, + OpKind::Call { target: CallTarget::FunctionPath { segments }, .. } + if segments.last().map(String::as_str) == Some("exc_kind_discriminant") ) }) .count(); assert_eq!( - stopiteration_ctors, 0, - "the StopIteration guard ctor must be gone after the drain fusion" + exc_kind_calls, 0, + "the fused handler must not reintroduce the flat exception-kind test" ); // The fused next() call site carries a LastException exit. @@ -260,8 +275,9 @@ fn unpackiterable_drain_match_fuses_to_kind_test() { "the drain next() site must become a LastException exception-edge" ); eprintln!( - "drain fusion: exc_kind_discriminant={exc_kind_calls} \ - stopiteration_ctors={stopiteration_ctors} lastexc_blocks={lastexc_blocks}" + "drain fusion: object_predicate={object_predicate_calls} \ + source_predicate={source_predicate_calls} exc_kind_discriminant={exc_kind_calls} \ + lastexc_blocks={lastexc_blocks}" ); } diff --git a/pyre/bench/synth/exception_with_exit_self_null_slot.cranelift.jitstats b/pyre/bench/synth/exception_with_exit_self_null_slot.cranelift.jitstats index 32c163496e6..afca5607736 100644 --- a/pyre/bench/synth/exception_with_exit_self_null_slot.cranelift.jitstats +++ b/pyre/bench/synth/exception_with_exit_self_null_slot.cranelift.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_blackhole_adopted_multi_frame=0 -fbw_blackhole_adopted_single_frame=1 +fbw_blackhole_adopted_single_frame=9 fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 diff --git a/pyre/bench/synth/exception_with_exit_self_null_slot.dynasm.jitstats b/pyre/bench/synth/exception_with_exit_self_null_slot.dynasm.jitstats index 32c163496e6..afca5607736 100644 --- a/pyre/bench/synth/exception_with_exit_self_null_slot.dynasm.jitstats +++ b/pyre/bench/synth/exception_with_exit_self_null_slot.dynasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_blackhole_adopted_multi_frame=0 -fbw_blackhole_adopted_single_frame=1 +fbw_blackhole_adopted_single_frame=9 fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 diff --git a/pyre/bench/synth/exception_with_exit_self_null_slot.wasm.jitstats b/pyre/bench/synth/exception_with_exit_self_null_slot.wasm.jitstats index 32c163496e6..afca5607736 100644 --- a/pyre/bench/synth/exception_with_exit_self_null_slot.wasm.jitstats +++ b/pyre/bench/synth/exception_with_exit_self_null_slot.wasm.jitstats @@ -3,7 +3,7 @@ descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 fbw_blackhole_adopted_multi_frame=0 -fbw_blackhole_adopted_single_frame=1 +fbw_blackhole_adopted_single_frame=9 fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 diff --git a/pyre/check.py b/pyre/check.py index 77f21307423..5e5b5719c5a 100644 --- a/pyre/check.py +++ b/pyre/check.py @@ -206,6 +206,24 @@ def _detect_pyre_stdlib(): # as the run it is subtracted from. STARTUP_SAMPLES = 5 EXEC_TIME_FLOOR_S = WIN_TIMER_QUANTUM_S if sys.platform == "win32" else 0.005 +# `exec` is `bench - startup`, and startup is a measured median rather than a +# constant. Between runs of the same job on the same platform, pypy readings +# moved 0.013s -> 0.031s and 0.019s -> 0.029s, dynasm moved 0.088s -> 0.158s, +# and cranelift moved 0.084s -> 0.144s (while cpython moved 0.022s -> 0.023s). +# As a fraction of the larger reading in each pair that is 34%, 58%, 44% and +# 42%. Whatever the startup estimate is off by lands whole in `exec`, so the +# residual error of a startup-subtracted time scales with the startup, not +# with the timer quantum -- for pypy it is 2-6x EXEC_TIME_FLOOR_S and for the +# backends more than 10x. Half the measured startup sits in the middle of the +# observed range. +# Consequently the effective ceiling becomes +# `limit * (1 + STARTUP_DRIFT_FRACTION * startup / exec_baseline)`: a fixture +# whose baseline is real work is barely affected, while one whose baseline is +# the same size as its own startup gets several times the slack -- which is +# what its measurement can actually support. +# The allowance applies only to the recorded-ratio gates; the wasm/dynasm gate +# opts out at its call site. +STARTUP_DRIFT_FRACTION = 0.5 # A floor failure is only trustworthy when the baseline clears the execution # floor enough for small relative error: execution time is the difference # between two independently measured values. @@ -2540,6 +2558,12 @@ def _retry_performance_gate( pyre_times.append(elapsed) return statistics.median(pyre_times), statistics.median(baseline_times) + def _startup_drift(self, key): + """Run-to-run error of the startup `_exec_time` subtracted for *key*.""" + if self.args.no_startup_subtract: + return 0.0 + return STARTUP_DRIFT_FRACTION * self.startup.get(key, 0.0) + def _baseline_exec_time_clamped(self, baseline, baseline_time): """Whether startup subtraction pinned a baseline to its floor.""" exec_b = self._exec_time(baseline, baseline_time) @@ -2567,6 +2591,7 @@ def _baseline_exec_time_thin(self, baseline, baseline_time): def _performance_gate_passed( self, backend, script, timeout, elapsed, limit, baseline_time, baseline_cmd, expected_output, baseline_key, minimum=None, + *, allow_startup_drift=True, ): """Check one performance ratio, retrying a failure by median. @@ -2594,12 +2619,21 @@ def _performance_gate_passed( # for a baseline that is real work. `exception_traceback_loop_forms` # read 35.6x on one runner and 37.1x on another against an unchanged # ~2.3s exec, a 0.06s baseline wobbling +-0.004s across the 36x line. + # + # That term stays: it is the granularity of the clock, and it is what a + # baseline sitting at the floor is worth. It is not the whole error. + # An exec time also carries whatever its startup estimate was off by, + # which is a separate quantity of a different size -- see + # STARTUP_DRIFT_FRACTION, applied per side below rather than folded in + # here, because the two bounds are distorted by opposite sides. compare_buffer = BENCH_COMPARE_BUFFER_S if sys.platform == "win32": compare_buffer += 2 * WIN_TIMER_QUANTUM_S * (1 + limit) else: compare_buffer += limit * EXEC_TIME_FLOOR_S + drift = self._startup_drift if allow_startup_drift else (lambda _key: 0.0) + def failed_bound(measured, baseline_value): exec_measured = self._exec_time(backend, measured) exec_baseline = self._exec_time(baseline_key, baseline_value) @@ -2634,12 +2668,33 @@ def failed_bound(measured, baseline_value): # wants a ratio gate has to give pypy enough work to measure. if self._baseline_exec_time_clamped(baseline_key, baseline_value): return None - if exec_measured > exec_baseline * limit + compare_buffer: + # Each bound is distorted by one side only, so each gets the + # allowance on that side. A startup estimate that came out high + # under-states every exec derived from it: for the ceiling that + # shrinks the DENOMINATOR and inflates the ratio, so the allowance + # goes to the baseline; for the floor it shrinks the NUMERATOR and + # the fixture reads as having reached parity, so the allowance goes + # to the measured side. The opposite side of each bound is left + # alone -- an under-stated numerator only makes the ceiling pass, + # and a gate is not made honest by relaxing it where it cannot + # falsely fire. + # + # One run demonstrated both at once: pypy startup read 0.029s + # against 0.019s on the same base, and dynasm 0.158s against + # 0.088s, which failed four ceilings and three floors while every + # fixture's own measured time had gone down. + if exec_measured > ( + exec_baseline + drift(baseline_key) + ) * limit + compare_buffer: return "ceiling" + # The measured side enters amplified by `limit / minimum`, so its + # error is amplified with it: the allowance belongs inside that + # factor, not in the flat buffer. if ( minimum is not None and exec_baseline >= FLOOR_GATE_MIN_BASELINE_S - and exec_measured * (limit / minimum) + compare_buffer + and (exec_measured + drift(backend)) + * (limit / minimum) + compare_buffer < exec_baseline * limit ): return "floor" @@ -2867,11 +2922,27 @@ def _ratio(elapsed_val, pypy_val): ): self.wasm_ratio_ungated.append(name) else: + # The startup-drift allowance is calibrated on the recorded + # per-bench ratios, whose baseline is re-measured on every host + # while the ceiling stays fixed at what the fitting run saw. + # This gate's failures have not been shown to move that way, + # and widening it costs a red that names a real backend gap: + # `short_circuit_value_kept_stack` reads 5.3x here against a 4x + # ceiling and clears it only with the allowance. The denominator + # concern this gate does have is handled above by declining the + # gate outright and naming the fixture in the summary, not by + # quietly widening the bound. + # + # This ceiling is also meant to come back down as the backend + # closes the gap, so it has to mean the number it states. A + # standing allowance underneath it would be re-fitted along with + # it, and the tightening would buy less than it says it does. passed, bound, checked_elapsed, checked_baseline, retry_note = ( self._performance_gate_passed( backend, script, timeout, elapsed, ceiling, dynasm_elapsed, [self._pyre("dynasm"), script], pypy_output, "dynasm", + allow_startup_drift=False, ) ) if not passed: diff --git a/pyre/extra_tests/parity_tests/for_iter_inside_except_reraise.py b/pyre/extra_tests/parity_tests/for_iter_inside_except_reraise.py new file mode 100644 index 00000000000..22e5a35bea0 --- /dev/null +++ b/pyre/extra_tests/parity_tests/for_iter_inside_except_reraise.py @@ -0,0 +1,98 @@ +# CPython-suite gap: the suite does not run a hot user-defined iterator to +# exhaustion inside an `except` body and then re-raise the handled exception +# with a bare `raise`. +# parity-tests reason: FOR_ITER consumes a StopIteration, and the frame state +# on the consuming edge must keep naming the exception the enclosing handler +# caught -- otherwise the bare `raise` re-raises the loop's own exhaustion +# signal instead of the handled exception. + +try: + import pypyjit + + pypyjit.set_param("threshold=1,function_threshold=1") +except ImportError: + pass + + +class It: + """A user-defined iterator, so FOR_ITER takes the instance `__next__` route.""" + + def __init__(self, n): + self.n = n + + def __iter__(self): + return self + + def __next__(self): + if self.n <= 0: + raise StopIteration + self.n -= 1 + return self.n + + +def bare_reraise(n): + """The bare `raise` sits in the handler body, uncovered by any other try.""" + try: + raise ValueError("outer") + except ValueError: + for _ in It(n): + pass + raise + + +def bare_reraise_nested(n): + """Two handlers deep: the innermost handled exception wins.""" + try: + raise KeyError("outer") + except KeyError: + try: + raise IndexError("inner") + except IndexError: + for _ in It(n): + pass + raise + + +def reraise_by_name(n): + """The named-exception control: `raise e` never reads the frame's pair.""" + try: + raise ValueError("outer") + except ValueError as e: + for _ in It(n): + pass + raise e + + +def propagate_from_handler(n): + """A fresh raise out of the handler chains the handled exception as context.""" + try: + raise ValueError("outer") + except ValueError: + for _ in It(n): + pass + raise TypeError("fresh") + + +def caught(fn, n): + try: + fn(n) + except BaseException as e: # noqa: BLE001 - the defect swaps the exception type + context = type(e.__context__).__name__ if e.__context__ is not None else None + return (type(e).__name__, str(e), context) + return ("no exception", "", None) + + +for _ in range(3000): + # An empty loop body and a non-empty one reach the exhaustion edge with + # different stack depths, so exercise both. + for count in (0, 3): + assert caught(bare_reraise, count) == ("ValueError", "outer", None) + assert caught(bare_reraise_nested, count) == ("IndexError", "inner", "KeyError") + assert caught(reraise_by_name, count) == ("ValueError", "outer", None) + assert caught(propagate_from_handler, count) == ( + "TypeError", + "fresh", + "ValueError", + ) + +print("OK") diff --git a/pyre/extra_tests/parity_tests/for_iter_instance_next_deep_stop.py b/pyre/extra_tests/parity_tests/for_iter_instance_next_deep_stop.py new file mode 100644 index 00000000000..65a4626bc5b --- /dev/null +++ b/pyre/extra_tests/parity_tests/for_iter_instance_next_deep_stop.py @@ -0,0 +1,35 @@ +# CPython-suite gap: the suite does not exhaust a hot user iterator through a +# nested Python frame below __next__. +# parity-tests reason: a deep StopIteration must reach the caller FOR_ITER +# handler instead of leaking from an inlined callee chain. + + +class DelegatingIterator: + def __init__(self, limit): + self.index = 0 + self.limit = limit + + def __iter__(self): + return self + + def _advance(self): + if self.index >= self.limit: + raise StopIteration + self.index += 1 + return self.index + + def __next__(self): + return self._advance() + + +def consume(limit): + count = 0 + for value in DelegatingIterator(limit): + assert value == count + 1 + count += 1 + return count + + +for _ in range(12): + assert consume(1600) == 1600 +print("OK") diff --git a/pyre/extra_tests/parity_tests/for_iter_instance_next_delegating.py b/pyre/extra_tests/parity_tests/for_iter_instance_next_delegating.py new file mode 100644 index 00000000000..34eb07dc657 --- /dev/null +++ b/pyre/extra_tests/parity_tests/for_iter_instance_next_delegating.py @@ -0,0 +1,78 @@ +# CPython-suite gap: the suite does not run a hot delegating __next__ whose +# exhaustion arrives from a residual call rather than an explicit raise. +# parity-tests reason: FOR_ITER owns the StopIteration-to-exhaustion mapping, +# so a callee that merely propagates it must still end the loop. + + +class Wrap: + def __init__(self, inner): + self._it = inner + + def __iter__(self): + return self + + def __next__(self): + return next(self._it) + + +def consume(n): + total = 0 + for value in Wrap(iter(range(n))): + total += value + return total + + +expected = 20000 * 19999 // 2 +for _ in range(12): + assert consume(20000) == expected + + +# The same shape one level deeper: the inner iterator is itself a wrapper, so +# the exhaustion propagates through two Python __next__ frames. +class Doubled: + def __init__(self, inner): + self._it = inner + + def __iter__(self): + return self + + def __next__(self): + return next(self._it) + + +def consume_nested(n): + count = 0 + for _ in Doubled(Wrap(iter(range(n)))): + count += 1 + return count + + +for _ in range(12): + assert consume_nested(4000) == 4000 + + +# A delegating __next__ that also raises its own StopIteration on a sentinel +# must end the loop at the sentinel, not at the inner iterator's exhaustion. +class StopEarly: + def __init__(self, inner, stop_at): + self._it = inner + self._stop_at = stop_at + self._seen = 0 + + def __iter__(self): + return self + + def __next__(self): + value = next(self._it) + self._seen += 1 + if value == self._stop_at: + raise StopIteration + return value + + +for _ in range(12): + early = StopEarly(iter(range(20000)), 1500) + assert sum(1 for _ in early) == 1500 + assert early._seen == 1501 + +print("OK") diff --git a/pyre/extra_tests/parity_tests/for_iter_instance_next_effect_before_guard.py b/pyre/extra_tests/parity_tests/for_iter_instance_next_effect_before_guard.py new file mode 100644 index 00000000000..a6374c16695 --- /dev/null +++ b/pyre/extra_tests/parity_tests/for_iter_instance_next_effect_before_guard.py @@ -0,0 +1,42 @@ +# CPython-suite gap: the suite does not exercise a JIT guard inside an inlined +# user-defined __next__ after an observable effect. +# parity-tests reason: caller-boundary FOR_ITER resume must not apply the +# current iterator step twice when the hot branch changes direction. + +effects = [] + + +class SwitchingIterator: + def __init__(self, limit, switch_at): + self.index = 0 + self.limit = limit + self.switch_at = switch_at + + def __iter__(self): + return self + + def __next__(self): + if self.index >= self.limit: + raise StopIteration + value = self.index + self.index += 1 + effects.append(value) + if value < self.switch_at: + return value + 1 + return value - 1 + + +def consume(limit, switch_at): + total = 0 + for value in SwitchingIterator(limit, switch_at): + total += value + return total + + +rounds = 12 +limit = 1600 +for _ in range(rounds): + consume(limit, 1200) + +assert len(effects) == rounds * limit +print("OK") diff --git a/pyre/extra_tests/parity_tests/for_iter_instance_next_non_function.py b/pyre/extra_tests/parity_tests/for_iter_instance_next_non_function.py new file mode 100644 index 00000000000..50468f0fe7f --- /dev/null +++ b/pyre/extra_tests/parity_tests/for_iter_instance_next_non_function.py @@ -0,0 +1,70 @@ +# CPython-suite gap: the suite does not exercise non-function __next__ +# descriptors at a hot FOR_ITER site. +# parity-tests reason: the user-function inline route must decline classmethod, +# builtin, and callable-instance forms without changing iterator semantics. + + +class ClassMethodIterator: + remaining = 0 + + def __init__(self, count): + type(self).remaining = count + + def __iter__(self): + return self + + @classmethod + def __next__(cls): + if cls.remaining == 0: + raise StopIteration + cls.remaining -= 1 + return cls.remaining + + +def builtin_iterator(count): + source = iter(range(count)) + + class BuiltinIterator: + def __iter__(self): + return self + + __next__ = source.__next__ + + return BuiltinIterator() + + +class NextCallable: + def __init__(self): + self.remaining = 0 + + def __call__(self): + if self.remaining == 0: + raise StopIteration + self.remaining -= 1 + return self.remaining + + +class CallableIterator: + __next__ = NextCallable() + + def __init__(self, count): + type(self).__next__.remaining = count + + def __iter__(self): + return self + + +def consume(iterator): + count = 0 + for _ in iterator: + count += 1 + if count == 1600: + break + return count + + +for _ in range(12): + assert consume(ClassMethodIterator(1600)) == 1600 + assert consume(builtin_iterator(1600)) == 1600 + assert consume(CallableIterator(1600)) == 1600 +print("OK") diff --git a/pyre/extra_tests/parity_tests/for_iter_instance_next_pending_exception.py b/pyre/extra_tests/parity_tests/for_iter_instance_next_pending_exception.py new file mode 100644 index 00000000000..0c23b5b7ab9 --- /dev/null +++ b/pyre/extra_tests/parity_tests/for_iter_instance_next_pending_exception.py @@ -0,0 +1,48 @@ +# CPython-suite gap: the suite does not deopt a hot user-defined __next__ at a +# raising residual after the iterator has advanced. +# parity-tests reason: a pending non-StopIteration exception must propagate +# once without replaying the current state transition. + +advances = [] + + +class RaisingIterator: + def __init__(self, limit, raise_at): + self.index = 0 + self.limit = limit + self.raise_at = raise_at + + def __iter__(self): + return self + + def __next__(self): + if self.index >= self.limit: + raise StopIteration + self.index += 1 + advances.append(self.index) + if self.index == self.raise_at: + int("not-an-integer") + return self.index + + +def consume(limit, raise_at): + count = 0 + for _ in RaisingIterator(limit, raise_at): + count += 1 + return count + + +for _ in range(8): + assert consume(1600, 2000) == 1600 + +advances.clear() +errors = 0 +k = 1300 +try: + consume(1600, k) +except ValueError: + errors += 1 + +assert errors == 1 +assert len(advances) == k +print("OK") diff --git a/pyre/extra_tests/parity_tests/for_iter_instance_next_polymorphic_class.py b/pyre/extra_tests/parity_tests/for_iter_instance_next_polymorphic_class.py new file mode 100644 index 00000000000..246a150ce52 --- /dev/null +++ b/pyre/extra_tests/parity_tests/for_iter_instance_next_polymorphic_class.py @@ -0,0 +1,78 @@ +# CPython-suite gap: the suite does not alternate two user iterator classes at +# one hot FOR_ITER site. +# parity-tests reason: user instances share one physical layout, so pinning the +# layout alone would let a second class run the first class's __next__. + + +class Ascending: + def __init__(self, limit): + self.index = 0 + self.limit = limit + + def __iter__(self): + return self + + def __next__(self): + if self.index >= self.limit: + raise StopIteration + self.index += 1 + return self.index + + +class Descending: + def __init__(self, limit): + self.index = limit + self.limit = limit + + def __iter__(self): + return self + + def __next__(self): + if self.index <= 0: + raise StopIteration + self.index -= 1 + return self.index + + +def collect(iterator): + seen = [] + for value in iterator: + seen.append(value) + return seen + + +limit = 1600 +ascending = list(range(1, limit + 1)) +descending = list(range(limit - 1, -1, -1)) + +# Warm the site on one class alone so the loop compiles against it, then keep +# feeding both through the same FOR_ITER. +for _ in range(8): + assert collect(Ascending(limit)) == ascending + +for _ in range(8): + assert collect(Descending(limit)) == descending + assert collect(Ascending(limit)) == ascending + +# A third class whose __next__ returns a different type must not inherit either +# body: a stale method would return ints here. +class Tagging: + def __init__(self, limit): + self.index = 0 + self.limit = limit + + def __iter__(self): + return self + + def __next__(self): + if self.index >= self.limit: + raise StopIteration + self.index += 1 + return "t%d" % self.index + + +for _ in range(8): + tagged = collect(Tagging(4)) + assert tagged == ["t1", "t2", "t3", "t4"], tagged + +print("OK") diff --git a/pyre/extra_tests/parity_tests/for_iter_instance_next_stop_subclass.py b/pyre/extra_tests/parity_tests/for_iter_instance_next_stop_subclass.py new file mode 100644 index 00000000000..a8c14b21bd3 --- /dev/null +++ b/pyre/extra_tests/parity_tests/for_iter_instance_next_stop_subclass.py @@ -0,0 +1,35 @@ +# CPython-suite gap: the suite does not exhaust a hot user-defined __next__ +# with a StopIteration subclass. +# parity-tests reason: FOR_ITER exhaustion matching is by subclass, including +# after the iterator method has entered the trace. + + +class MyStop(StopIteration): + pass + + +class SubclassStoppingIterator: + def __init__(self, limit): + self.index = 0 + self.limit = limit + + def __iter__(self): + return self + + def __next__(self): + if self.index >= self.limit: + raise MyStop + self.index += 1 + return self.index + + +def consume(limit): + count = 0 + for _ in SubclassStoppingIterator(limit): + count += 1 + return count + + +for _ in range(12): + assert consume(1600) == 1600 +print("OK") diff --git a/pyre/extra_tests/parity_tests/for_iter_instance_next_try_enclosed.py b/pyre/extra_tests/parity_tests/for_iter_instance_next_try_enclosed.py new file mode 100644 index 00000000000..2e51ec862a7 --- /dev/null +++ b/pyre/extra_tests/parity_tests/for_iter_instance_next_try_enclosed.py @@ -0,0 +1,106 @@ +# CPython-suite gap: the suite does not run a hot user-defined __next__ whose +# FOR_ITER sits inside a try range in the same frame. +# parity-tests reason: FOR_ITER's materialized catch routes a non-StopIteration +# exception to the Python handler covering its PC, so the handler edge, the +# multi-clause match order and the finally path must all survive that rewiring. + +advances = [] + + +class Raising: + def __init__(self, limit, raise_at, exc): + self.index = 0 + self.limit = limit + self.raise_at = raise_at + self.exc = exc + + def __iter__(self): + return self + + def __next__(self): + if self.index >= self.limit: + raise StopIteration + self.index += 1 + advances.append(self.index) + if self.index == self.raise_at: + raise self.exc("boom") + return self.index + + +# The loop is inside the try, so the raise takes FOR_ITER's handler edge +# rather than leaving the frame. +def consume_caught(limit, raise_at, exc): + total = 0 + caught = None + try: + for value in Raising(limit, raise_at, exc): + total += value + except ValueError as e: + caught = ("ValueError", str(e)) + except TypeError as e: + caught = ("TypeError", str(e)) + return total, caught + + +# Hot with no exception at all: the try range must not disturb exhaustion. +for _ in range(12): + total, caught = consume_caught(2000, 0, ValueError) + assert total == 2000 * 2001 // 2, total + assert caught is None, caught + +# The second except clause must win when the first does not match, which +# exercises the handler's own match chain downstream of FOR_ITER's edge. +advances.clear() +total, caught = consume_caught(2000, 1500, TypeError) +assert caught == ("TypeError", "boom"), caught +assert total == 1499 * 1500 // 2, total +assert len(advances) == 1500, len(advances) + +advances.clear() +total, caught = consume_caught(2000, 1500, ValueError) +assert caught == ("ValueError", "boom"), caught +assert len(advances) == 1500, len(advances) + + +# A finally between the loop and the handler must still run exactly once. +def consume_finally(limit, raise_at): + marks = [] + try: + for _ in Raising(limit, raise_at, ValueError): + pass + except ValueError: + marks.append("caught") + finally: + marks.append("finally") + return marks + + +for _ in range(12): + assert consume_finally(1200, 0) == ["finally"] + +assert consume_finally(1200, 900) == ["caught", "finally"] + + +# An unhandled kind inside the try must leave the frame, not be swallowed by +# the handler edge. +def consume_escapes(limit, raise_at): + try: + for _ in Raising(limit, raise_at, KeyError): + pass + except ValueError: + return "wrong" + return "no-raise" + + +for _ in range(12): + assert consume_escapes(1200, 0) == "no-raise" + +escaped = 0 +try: + consume_escapes(1200, 700) +except KeyError: + escaped = 1 + +assert escaped == 1 + +print("OK") diff --git a/pyre/extra_tests/parity_tests/for_iter_raising_next_traceback.py b/pyre/extra_tests/parity_tests/for_iter_raising_next_traceback.py new file mode 100644 index 00000000000..f156b64d627 --- /dev/null +++ b/pyre/extra_tests/parity_tests/for_iter_raising_next_traceback.py @@ -0,0 +1,89 @@ +# CPython-suite gap: the suite never inspects the traceback of a +# non-StopIteration exception raised out of __next__ by a hot FOR_ITER. +# parity-tests reason: FOR_ITER's mismatch arm re-raises a value that already +# carries this frame's traceback node, so the re-raise must not record a second +# one. + +try: + import pypyjit + + pypyjit.set_param("threshold=1,function_threshold=1") +except ImportError: + pass + +import dis +import sys +import traceback + + +class Boom: + def __iter__(self): + return self + + def __next__(self): + return self.missing + + +def enclosed(): + for value in Boom(): + return value + + +def bare(): + for value in Boom(): + return value + + +def for_iter_offset(function): + return next( + instruction.offset + for instruction in dis.get_instructions(function) + if instruction.opname == "FOR_ITER" + ) + + +# The looping frame's node must name the FOR_ITER that was in flight, not +# whatever instruction a coarser coordinate lookup happens to land on. Derive +# the offset from the function's own bytecode so the check states the rule +# instead of pinning a layout. +expected_lasti = { + "enclosed": for_iter_offset(enclosed), + "bare": for_iter_offset(bare), +} + +shapes = set() +loop_frame_lasti = set() + + +def observe(tb): + shapes.add(tuple(f.name for f in traceback.extract_tb(tb))) + while tb is not None: + name = tb.tb_frame.f_code.co_name + if name in expected_lasti: + loop_frame_lasti.add((name, tb.tb_lasti)) + tb = tb.tb_next + + +for _ in range(8): + try: + try: + enclosed() + except AttributeError: + raise + except AttributeError: + observe(sys.exc_info()[2]) + + try: + bare() + except AttributeError: + observe(sys.exc_info()[2]) + +assert shapes == { + ("", "enclosed", "__next__"), + ("", "bare", "__next__"), +}, sorted(shapes) +assert loop_frame_lasti == { + ("enclosed", expected_lasti["enclosed"]), + ("bare", expected_lasti["bare"]), +}, sorted(loop_frame_lasti) +print("OK") diff --git a/pyre/extra_tests/parity_tests/for_iter_raising_next_walk_abort.py b/pyre/extra_tests/parity_tests/for_iter_raising_next_walk_abort.py new file mode 100644 index 00000000000..4cb7d0bc0be --- /dev/null +++ b/pyre/extra_tests/parity_tests/for_iter_raising_next_walk_abort.py @@ -0,0 +1,40 @@ +# CPython-suite gap: the suite does not repeatedly trace a FOR_ITER whose +# iterator raises a non-StopIteration exception from __next__. +# parity-tests reason: FOR_ITER's internal catch must forward the materialized +# exception through its match split without aborting the full-body walk and +# dropping an outer-loop iteration. + +try: + import pypyjit + + pypyjit.set_param("threshold=1,function_threshold=1") +except ImportError: + pass + +advances = [] + + +class Boom: + def __iter__(self): + return self + + def __next__(self): + advances.append(1) + return self.missing + + +def show(x): + return x + + +rounds_seen = [] +for round_no in range(6): + advances.clear() + try: + show([x for x in Boom()]) + except AttributeError: + pass + rounds_seen.append((round_no, len(advances))) + +assert rounds_seen == [(i, 1) for i in range(6)], rounds_seen +print("OK") diff --git a/pyre/extra_tests/parity_tests/for_iter_try_enclosed_builtin_iter.py b/pyre/extra_tests/parity_tests/for_iter_try_enclosed_builtin_iter.py new file mode 100644 index 00000000000..0313dab547f --- /dev/null +++ b/pyre/extra_tests/parity_tests/for_iter_try_enclosed_builtin_iter.py @@ -0,0 +1,79 @@ +# CPython-suite gap: the suite does not JIT-compile a hot builtin-iterator +# loop whose FOR_ITER sits inside a try range. +# parity-tests reason: FOR_ITER materializes its own catch, so the enclosing +# try's handler edge must not disturb the loop's exit shape — this holds for +# every iterator, not only user-defined ones. + + +def sum_range(n): + total = 0 + try: + for value in range(n): + total += value + except ValueError: + return -1 + return total + + +for _ in range(12): + assert sum_range(2000) == 1999 * 2000 // 2 + + +def sum_list(values): + total = 0 + try: + for value in values: + total += value + except ValueError: + return -1 + return total + + +data = list(range(2000)) +for _ in range(12): + assert sum_list(data) == 1999 * 2000 // 2 + + +# Nested try ranges around the same loop, and a loop whose body itself raises +# into the enclosing handler. +def sum_nested(n, raise_at): + total = 0 + try: + try: + for value in range(n): + if value == raise_at: + raise ValueError("inner") + total += value + except TypeError: + return -2 + except ValueError: + return total + return total + + +for _ in range(12): + assert sum_nested(2000, -1) == 1999 * 2000 // 2 + +assert sum_nested(2000, 1500) == 1499 * 1500 // 2 + + +# A generator iterator inside a try: exhaustion must still end the loop. +def gen(n): + for i in range(n): + yield i + + +def sum_gen(n): + total = 0 + try: + for value in gen(n): + total += value + except ValueError: + return -1 + return total + + +for _ in range(12): + assert sum_gen(1500) == 1499 * 1500 // 2 + +print("OK") diff --git a/pyre/extra_tests/parity_tests/generator_pep479_subclass.py b/pyre/extra_tests/parity_tests/generator_pep479_subclass.py new file mode 100644 index 00000000000..da7ddbcd728 --- /dev/null +++ b/pyre/extra_tests/parity_tests/generator_pep479_subclass.py @@ -0,0 +1,133 @@ +# CPython-suite gap: the suite leaks StopIteration itself out of a generator +# but never a subclass of it, so a flat exception-tag test passes the suite. +# parity-tests reason: PEP 479 conversion selects on the exception MRO, so a +# StopIteration subclass leaking a generator must become RuntimeError exactly +# as its base does -- including under multiple inheritance, where the tag of +# the first base decides nothing. + +try: + import pypyjit + + pypyjit.set_param("threshold=1,function_threshold=1") +except ImportError: + pass + + +class Sub(StopIteration): + pass + + +class SV(StopIteration, ValueError): + pass + + +class VS(ValueError, StopIteration): + pass + + +class SubAsync(StopAsyncIteration): + pass + + +class VSA(ValueError, StopAsyncIteration): + pass + + +class Exhausted: + """An iterator whose exhaustion signal is a StopIteration subclass.""" + + def __init__(self, stop_type): + self.stop_type = stop_type + + def __iter__(self): + return self + + def __next__(self): + raise self.stop_type("inner") + + +def g_raise(stop_type): + yield 1 + raise stop_type("boom") + + +def g_leak(stop_type): + yield 1 + next(Exhausted(stop_type)) + + +def drive_coroutine(coro): + """Step a coroutine to completion without an event loop.""" + try: + while True: + coro.send(None) + except StopIteration as e: + return e.value + + +def classify(fn): + """Run `fn`, reporting how the generator's escaping exception surfaced.""" + try: + return ("ok", fn()) + except RuntimeError as e: + cause = e.__cause__ + return ("runtimeerror", str(e), type(cause).__name__) + except BaseException as e: # noqa: BLE001 - the defect is a leak, so catch it + return ("leaked", type(e).__name__) + + +def exercise(stop_type): + name = stop_type.__name__ + for label, gen in (("raise", g_raise), ("leak", g_leak)): + for drive in ( + lambda g: list(g), + lambda g: [x for x in g], # noqa: C416 - the for loop is its own path + lambda g: tuple(g), + lambda g: (next(g), next(g)), + lambda g: (g.send(None), g.send(None)), + ): + got = classify(lambda: drive(gen(stop_type))) + assert got == ( + "runtimeerror", + "generator raised StopIteration", + name, + ), (label, name, got) + + +def exercise_async(stop_type, expected_message): + async def ag(): + yield 1 + raise stop_type("boom") + + it = ag() + assert drive_coroutine(it.__anext__()) == 1 + got = classify(lambda: drive_coroutine(it.__anext__())) + assert got == ("runtimeerror", expected_message, stop_type.__name__), ( + stop_type.__name__, + got, + ) + + +def exercise_return(): + """A generator that simply returns is untouched by the conversion.""" + + def g(): + yield 1 + return 7 + + assert list(g()) == [1] + + +for _ in range(2000): + exercise(StopIteration) + exercise(Sub) + exercise(SV) + exercise(VS) + exercise_async(StopAsyncIteration, "async generator raised StopAsyncIteration") + exercise_async(SubAsync, "async generator raised StopAsyncIteration") + exercise_async(VSA, "async generator raised StopAsyncIteration") + exercise_async(Sub, "async generator raised StopIteration") + exercise_async(VS, "async generator raised StopIteration") + exercise_return() + +print("OK") diff --git a/pyre/extra_tests/parity_tests/raise_bare_class_bridge_identity.py b/pyre/extra_tests/parity_tests/raise_bare_class_bridge_identity.py new file mode 100644 index 00000000000..68dac0f00c3 --- /dev/null +++ b/pyre/extra_tests/parity_tests/raise_bare_class_bridge_identity.py @@ -0,0 +1,45 @@ +# CPython-suite gap: exception tests do not alternate bare exception classes at +# one hot raise site after a guard bridge has compiled. +# parity-tests reason: a bridge must keep the normalized exception instance +# distinct from the bare class operand that produced it. + +# No `pypyjit.set_param` preamble on purpose. The shape needs three traces in +# sequence — a loop that folds the bare-class raise, a second loop that declines +# it to the normalizing residual, then a bridge off that residual's class guard. +# Lowering the thresholds compiles the first loop before the sequence can form, +# and the fixture then passes on a binary that carries the defect. The default +# thresholds reach every trace well inside `N`. + +N = 200000 + + +class UserError(Exception): + pass + + +def raise_it(cls): + raise cls + + +def probe(cls): + caught = 0 + last = None + i = 0 + while i < N: + try: + raise_it(cls) + except BaseException as exc: + caught += 1 + last = exc + i += 1 + assert caught == N, (cls, caught) + assert type(last) is cls, (cls, type(last)) + return last + + +# Order is the condition: a class the bare-class raise fold accepts, then one it +# declines, then a second accepted class not yet seen at this site. +for exception_class in (ValueError, UserError, RuntimeError): + probe(exception_class) + +print("OK") diff --git a/pyre/extra_tests/parity_tests/raise_class_args_slot_defaults.py b/pyre/extra_tests/parity_tests/raise_class_args_slot_defaults.py new file mode 100644 index 00000000000..6ff725ed436 --- /dev/null +++ b/pyre/extra_tests/parity_tests/raise_class_args_slot_defaults.py @@ -0,0 +1,162 @@ +# CPython-suite gap: the exception tests build `AttributeError(msg)` / +# `NameError(msg)` / `StopIteration()` once each, never from a call site hot +# enough for a JIT construction fold to take over. +# parity-tests reason: pyre folds the called form `Type(args)` into trace IR +# once the argument list leaves every flattened slot defaulted, so those slots +# are written by emitted stores rather than by the runtime `__init__`. + +# The bare-class sibling (`raise_bare_class_slot_defaults.py`) censuses an +# instance built with NO arguments, so every slot it reads is a trace-time +# constant. The called form's census reads an instance built from runtime +# operands, and only the callable is guarded — a slot that reads `None` merely +# because THIS iteration's argument was `None` must not be emitted as a +# constant. `raise StopIteration(x)` covers exactly that: it is traced first +# with `x = None` and then run with a real value. + +N = 3000 + + +def caught(fn, exc_type): + try: + fn() + except exc_type as exc: + return exc + raise AssertionError("expected a raise") + + +def raise_attribute_error(): + raise AttributeError("attr message") + + +def raise_name_error(): + raise NameError("name message") + + +def raise_stop_iteration(): + raise StopIteration + + +def raise_import_error(): + raise ImportError + + +def raise_import_error_pair(): + raise ImportError("import message", "second") + + +def raise_value_error(): + raise ValueError("value message") + + +# `interp_exceptions.py:1134-1137 W_AttributeError` takes `name` / `obj` from +# keywords only, so a lone positional leaves both unset. +for _ in range(N): + exc = caught(raise_attribute_error, AttributeError) +assert type(exc) is AttributeError, type(exc) +assert exc.args == ("attr message",), exc.args +assert exc.name is None, exc.name +assert exc.obj is None, exc.obj + +# `:810-812 W_NameError` likewise takes `name` from a keyword only. +for _ in range(N): + exc = caught(raise_name_error, NameError) +assert type(exc) is NameError, type(exc) +assert exc.args == ("name message",), exc.args +assert exc.name is None, exc.name + +# `:496-499 W_StopIteration` defaults `value` to None at zero arity. +for _ in range(N): + exc = caught(raise_stop_iteration, StopIteration) +assert type(exc) is StopIteration, type(exc) +assert exc.args == (), exc.args +assert exc.value is None, exc.value + +# `:363-377 W_ImportError` fills `msg` from a lone positional and leaves it +# unset at every other arity, so zero and two arguments default it. +for _ in range(N): + exc = caught(raise_import_error, ImportError) +assert type(exc) is ImportError, type(exc) +assert exc.args == (), exc.args +assert exc.name is None, exc.name +assert exc.path is None, exc.path +assert exc.msg is None, exc.msg + +for _ in range(N): + exc = caught(raise_import_error_pair, ImportError) +assert exc.args == ("import message", "second"), exc.args +assert exc.name is None, exc.name +assert exc.path is None, exc.path +assert exc.msg is None, exc.msg + +for _ in range(N): + exc = caught(raise_value_error, ValueError) +assert type(exc) is ValueError, type(exc) +assert exc.args == ("value message",), exc.args + + +# A slot the constructor fills FROM an argument must keep tracking that +# argument. The site below is traced while `payload` is None — the arity-1 +# `value` slot then reads `None`, which is indistinguishable from the default +# unless the fold looks at the argument too — and is then run with a real +# value. `value` has to follow `args[0]` on every later iteration. +payload = None + + +def raise_stop_iteration_value(): + raise StopIteration(payload) + + +for _ in range(N): + exc = caught(raise_stop_iteration_value, StopIteration) +assert exc.args == (None,), exc.args +assert exc.value is None, exc.value + +for i in range(N): + payload = i + exc = caught(raise_stop_iteration_value, StopIteration) + assert exc.value == i, (exc.value, i) + assert exc.args == (i,), exc.args + +# The same hole for `ImportError`'s lone-positional `msg`. +payload = None + + +def raise_import_error_msg(): + raise ImportError(payload) + + +for _ in range(N): + exc = caught(raise_import_error_msg, ImportError) +assert exc.args == (None,), exc.args +assert exc.msg is None, exc.msg + +for i in range(N): + payload = str(i) + exc = caught(raise_import_error_msg, ImportError) + assert exc.msg == str(i), (exc.msg, i) + + +# `raise X(...) from Y` keeps `__cause__` and flips `__suppress_context__`. +cause = ValueError("cause") +for _ in range(N): + try: + raise StopIteration() from cause + except StopIteration as exc: + assert exc.__cause__ is cause + assert exc.__suppress_context__ is True + assert exc.value is None + +# A raise inside an active handler chains `__context__` onto the new instance, +# so the fold's inline `__context__` store has to see the same value. +for _ in range(N): + try: + raise ValueError("outer") + except ValueError as outer: + try: + raise AttributeError("inner") + except AttributeError as inner: + assert inner.__context__ is outer + assert inner.name is None + assert inner.args == ("inner",) + +print("OK") diff --git a/pyre/extra_tests/parity_tests/readonly_descr_attr_raise.py b/pyre/extra_tests/parity_tests/readonly_descr_attr_raise.py new file mode 100644 index 00000000000..91ec506d006 --- /dev/null +++ b/pyre/extra_tests/parity_tests/readonly_descr_attr_raise.py @@ -0,0 +1,102 @@ +# CPython-suite gap: the descriptor tests assign through a read-only data +# descriptor once, never from a store site hot enough for a JIT raise fold to +# take over. +# parity-tests reason: pyre folds this AttributeError into trace IR instead of +# letting the residual `setattr` build it, so the message, `args`, the +# `__context__` chain and the traceback node are produced by emitted stores. + +# `objspace.py:723-740` reaches the descriptor terminal when the receiver keeps +# the default `__setattr__`, the class MRO resolves the name, and the +# descriptor's type resolves no `__set__` but does resolve `__delete__`. The +# rendered message names the DESCRIPTOR's type, and `__name__` can be +# reassigned without touching that type's version tag, so a fold has to shadow +# the name slot separately from the tag. +# +# No message text is asserted here — it differs per runtime. What every +# runtime must agree with is ITSELF: `hot_assign`, which runs often enough to +# compile, and `cold_assign`, a separate code object that never does, have to +# answer the same string and the same `args`. + +N = 3000 + + +class DeleteOnly: + def __delete__(self, obj): + pass + + +class Holder: + d = DeleteOnly() + + +def hot_assign(obj): + obj.d = 1 + + +def cold_assign(obj): + obj.d = 1 + + +def cold_exception(): + try: + cold_assign(Holder()) + except AttributeError as exc: + return exc + raise AssertionError("expected AttributeError") + + +holder = Holder() +expected = cold_exception() + +caught = 0 +last = None +for _ in range(N): + try: + hot_assign(holder) + except AttributeError as exc: + caught += 1 + last = exc +assert caught == N, caught +assert type(last) is AttributeError, type(last) +assert str(last) == str(expected), (str(last), str(expected)) +assert last.args == expected.args, (last.args, expected.args) +assert last.__traceback__ is not None +assert last.__cause__ is None +assert last.__suppress_context__ is False + +# The store must not have happened. +assert "d" not in holder.__dict__, holder.__dict__ +assert type(Holder.__dict__["d"]) is DeleteOnly + +# Renaming the descriptor's type changes the rendered message without changing +# its version tag. A fold that pinned only the tag keeps emitting the +# recording-time name, and the cold twin — which reads the live name — then +# disagrees. +DeleteOnly.__name__ = "RenamedDeleteOnly" +renamed = cold_exception() + +for _ in range(N): + try: + hot_assign(holder) + except AttributeError as exc: + last = exc +assert str(last) == str(renamed), (str(last), str(renamed)) + +# A raise inside an active handler chains `__context__` onto the new instance. +for _ in range(N): + try: + raise ValueError("outer") + except ValueError as outer: + try: + hot_assign(holder) + except AttributeError as inner: + assert inner.__context__ is outer + +# Giving the descriptor's type a `__set__` retires the terminal: the assignment +# now succeeds, and any compiled trace has to side-exit on the version tag. +DeleteOnly.__set__ = lambda self, obj, value: None +for _ in range(N): + hot_assign(holder) +assert "d" not in holder.__dict__, holder.__dict__ + +print("OK") diff --git a/pyre/extra_tests/parity_tests/stop_iteration_subclass_protocol.py b/pyre/extra_tests/parity_tests/stop_iteration_subclass_protocol.py new file mode 100644 index 00000000000..292b5b9e4e6 --- /dev/null +++ b/pyre/extra_tests/parity_tests/stop_iteration_subclass_protocol.py @@ -0,0 +1,62 @@ +# CPython-suite gap: the suite does not exercise every iterator consumer with +# a multiply inherited StopIteration subclass in both base orders. +# parity-tests reason: iterator protocol exhaustion must use the exception MRO, +# not the first base's flattened exception tag. + +try: + import pypyjit + + pypyjit.set_param("threshold=1,function_threshold=1") +except ImportError: + pass + + +class SV(StopIteration, ValueError): + pass + + +class VS(ValueError, StopIteration): + pass + + +class It: + def __init__(self, n, stop_type): + self.n = n + self.stop_type = stop_type + + def __iter__(self): + return self + + def __next__(self): + if self.n <= 0: + raise self.stop_type("done") + self.n -= 1 + return self.n + + +def consume_for(stop_type): + result = [] + for value in It(3, stop_type): + result.append(value) + return result + + +def f(*args): + return args + + +def exercise(stop_type): + assert consume_for(stop_type) == [2, 1, 0] + assert list(It(3, stop_type)) == [2, 1, 0] + assert tuple(It(3, stop_type)) == (2, 1, 0) + assert sum(It(3, stop_type)) == 3 + assert max(It(3, stop_type)) == 2 + assert next(It(0, stop_type), "dflt") == "dflt" + assert f(*It(3, stop_type)) == (2, 1, 0) + + +for _ in range(3000): + exercise(SV) + exercise(VS) + +print("OK") diff --git a/pyre/pyre-interpreter/src/baseobjspace.rs b/pyre/pyre-interpreter/src/baseobjspace.rs index 66c31bdda70..a389a102093 100644 --- a/pyre/pyre-interpreter/src/baseobjspace.rs +++ b/pyre/pyre-interpreter/src/baseobjspace.rs @@ -2062,6 +2062,34 @@ pub unsafe fn getitem_fast_path(w_obj: PyObjectRef) -> Option<(PyObjectRef, u64, } } +/// FOR_ITER fast path: return the receiver's type, its version tag, and the +/// `__next__` [`next`] would dispatch for a user instance, so the full-body +/// walker can inline the method in place of the opaque residual. +/// +/// This mirrors only [`next`]'s instance arm. Iterator layouts claimed by an +/// earlier arm, including the unpack iterator, and the later generic +/// non-instance lookup retain the residual path. +/// +/// # Safety +/// `w_obj` must be a live object. +pub unsafe fn next_fast_path(w_obj: PyObjectRef) -> Option<(PyObjectRef, u64, PyObjectRef)> { + unsafe { + if crate::module::r#struct::is_unpack_iter(w_obj) || !is_instance(w_obj) { + return None; + } + let w_type = w_instance_get_type(w_obj); + if w_type.is_null() { + return None; + } + let method = lookup_in_type_where(w_type, "__next__")?; + let version_tag = w_type_version_tag(w_type); + if version_tag == 0 { + return None; + } + Some((w_type, version_tag, method)) + } +} + #[inline(never)] /// `functional.py W_Range.descr_getitem` — integer index returns /// the member `start + i*step` (negative folded, bounds-checked); a slice @@ -2297,7 +2325,7 @@ pub(crate) fn sequence_index(w_container: PyObjectRef, w_item: PyObjectRef) -> P } index += 1; } - Err(e) if e.kind == PyErrorKind::StopIteration => break, + Err(e) if e.matches_stop_iteration() => break, Err(e) => return Err(e), } } @@ -2326,7 +2354,7 @@ pub(crate) fn sequence_count(w_container: PyObjectRef, w_item: PyObjectRef) -> P count += 1; } } - Err(e) if e.kind == PyErrorKind::StopIteration => break, + Err(e) if e.matches_stop_iteration() => break, Err(e) => return Err(e), } } @@ -2355,7 +2383,7 @@ pub(crate) fn sequence_contains( return Ok(true); } } - Err(e) if e.kind == PyErrorKind::StopIteration => break, + Err(e) if e.matches_stop_iteration() => break, Err(e) => return Err(e), } } @@ -3384,7 +3412,7 @@ unsafe fn pull_iterator_tuple( ); } } - Err(e) if e.kind == PyErrorKind::StopIteration => { + Err(e) if e.matches_stop_iteration() => { if !strict { return Ok(None); } @@ -3414,7 +3442,7 @@ unsafe fn pull_iterator_tuple( .unwrap(); return match next(it1) { Ok(_) => Err(strict_zip_error(func_name, 1, "longer")), - Err(e2) if e2.kind == PyErrorKind::StopIteration => Ok(None), + Err(e2) if e2.matches_stop_iteration() => Ok(None), Err(e2) => Err(e2), }; } @@ -3431,7 +3459,7 @@ unsafe fn pull_iterator_tuple( .unwrap(); match next(itj) { Ok(_) => return Err(strict_zip_error(func_name, j, "longer")), - Err(e2) if e2.kind == PyErrorKind::StopIteration => {} + Err(e2) if e2.matches_stop_iteration() => {} Err(e2) => return Err(e2), } } @@ -4115,7 +4143,7 @@ unsafe fn bytearray_assign_source(value: PyObjectRef) -> Result, PyError loop { match crate::baseobjspace::next(it) { Ok(w_item) => out.push(byte_w(w_item, "byte")?), - Err(e) if e.kind == PyErrorKind::StopIteration => break, + Err(e) if e.matches_stop_iteration() => break, Err(e) => return Err(e), } } @@ -11378,6 +11406,49 @@ pub fn type_immutable_attr_raise_is_stable(obj: PyObjectRef, name: &str, is_dele } } +/// Trace-time predicate for the read-only-data-descriptor STORE_ATTR raise. +/// +/// `objspace.py:723-739` reaches the terminal descriptor error only when the +/// receiver keeps `object.__setattr__`, the class MRO resolves `name`, and the +/// descriptor type resolves no `__set__` but does resolve `__delete__`. The +/// caller guards both type version tags before folding the raise, so later MRO +/// mutation side-exits instead of reusing this answer. +/// +/// The property, member, and getset families are deliberately excluded: their +/// dedicated arms in `set` and `descr_has_delete` have value- and +/// descriptor-specific behaviour rather than the general +/// `descroperation.py:114-126` terminal. +pub fn readonly_descr_attr_raise_is_stable(obj: PyObjectRef, name: &str) -> Option { + unsafe { + if obj.is_null() + || !is_instance(obj) + || pyre_object::is_exception(obj) + || name == "__dict__" + { + return None; + } + let w_type = w_instance_get_type(obj); + if w_type.is_null() || setattr_if_not_from_object(w_type).is_some() { + return None; + } + let descr = lookup_in_type_where(w_type, name)?; + if is_property(descr) + || pyre_object::is_member(descr) + || pyre_object::typedef::is_getset_property(descr) + { + return None; + } + let descr_type = crate::typedef::r#type(descr)?.as_ptr(); + if descr_type.is_null() + || lookup_in_type_where(descr_type, "__set__").is_some() + || lookup_in_type_where(descr_type, "__delete__").is_none() + { + return None; + } + Some(descr) + } +} + /// The `W_BaseException` typedef's attribute writes, shared by the /// per-class `GetSetProperty` descriptors and the instance-attribute store /// path. `PY_NULL` means the name is not one this exception kind @@ -13347,7 +13418,7 @@ fn _unpackiterable_unknown_length( // inside the handler (`e` is bound once, consumed only on the // re-raise path), not as a match guard. Err(e) => { - if e.kind == crate::PyErrorKind::StopIteration { + if e.matches_stop_iteration() { break; } return Err(e); @@ -13962,7 +14033,7 @@ fn _unpackiterable_known_length_jitlook( pyre_object::gc_roots::pin_root(w_item); count += 1; } - Err(e) if e.kind == crate::PyErrorKind::StopIteration => break, + Err(e) if e.matches_stop_iteration() => break, Err(e) => return Err(e), } } @@ -15195,7 +15266,7 @@ pub fn next(obj: PyObjectRef) -> PyResult { } Err(e) if e.kind == crate::PyErrorKind::IndexError - || e.kind == crate::PyErrorKind::StopIteration => + || e.matches_stop_iteration() => { let p = pyre_object::gc_roots::shadow_stack_get(obj_slot) as *mut pyre_object::W_SeqIterObject; @@ -15422,7 +15493,7 @@ pub fn next(obj: PyObjectRef) -> PyResult { let state = &mut *(w_self as *mut pyre_object::interp_itertools::W_ISlice); state.count = state.count.wrapping_add(1); } - Err(e) if e.kind == PyErrorKind::StopIteration => { + Err(e) if e.matches_stop_iteration() => { let w_self = pyre_object::gc_roots::shadow_stack_get(obj_slot); pyre_object::interp_itertools::w_islice_clear_iterable(w_self); return Err(e); @@ -15441,7 +15512,7 @@ pub fn next(obj: PyObjectRef) -> PyResult { let iterable = state.iterable; let w_item = match next(iterable) { Ok(w_item) => w_item, - Err(e) if e.kind == PyErrorKind::StopIteration => { + Err(e) if e.matches_stop_iteration() => { let w_self = pyre_object::gc_roots::shadow_stack_get(obj_slot); pyre_object::interp_itertools::w_islice_clear_iterable(w_self); return Err(e); @@ -15493,7 +15564,7 @@ pub fn next(obj: PyObjectRef) -> PyResult { pyre_object::gc_roots::pin_root(item); item_slots.push(pyre_object::gc_roots::shadow_stack_len() - 1); } - Err(e) if e.kind == PyErrorKind::StopIteration => { + Err(e) if e.matches_stop_iteration() => { if index == 0 { let w_self = pyre_object::gc_roots::shadow_stack_get(obj_slot); pyre_object::interp_itertools::w_batched_set_exhausted(w_self); @@ -16190,7 +16261,7 @@ pub fn next(obj: PyObjectRef) -> PyResult { let node = pyre_object::gc_roots::shadow_stack_get(node_slot); (*(node as *mut pyre_object::interp_itertools::W_TeeChainedListNode)) .running = false; - if err.kind == crate::PyErrorKind::StopIteration { + if err.matches_stop_iteration() { let w_self = pyre_object::gc_roots::shadow_stack_get(obj_slot); (*(w_self as *mut pyre_object::interp_itertools::W_TeeIterable)) .w_chained_list = PY_NULL; @@ -16381,7 +16452,7 @@ pub fn next(obj: PyObjectRef) -> PyResult { } else { match next(w_iter) { Ok(w_obj) => w_obj, - Err(e) if e.kind == PyErrorKind::StopIteration => { + Err(e) if e.matches_stop_iteration() => { let w_self = pyre_object::gc_roots::shadow_stack_get(obj_slot); let state = &mut *(w_self as *mut pyre_object::interp_itertools::W_ZipLongest); @@ -16559,14 +16630,14 @@ pub fn next(obj: PyObjectRef) -> PyResult { let item0 = match next(pyre_object::gc_roots::shadow_stack_get(iterator0_slot)) { Ok(item) => item, - Err(first_stop) if first_stop.kind == PyErrorKind::StopIteration => { + Err(first_stop) if first_stop.matches_stop_iteration() => { if !zo::w_zip_get_strict(pyre_object::gc_roots::shadow_stack_get(obj_slot)) { return Err(first_stop); } return match next(pyre_object::gc_roots::shadow_stack_get(iterator1_slot)) { Ok(_) => Err(strict_zip_error("zip", 1, "longer")), - Err(second_stop) if second_stop.kind == PyErrorKind::StopIteration => { + Err(second_stop) if second_stop.matches_stop_iteration() => { Err(first_stop) } Err(other) => Err(other), @@ -16582,7 +16653,7 @@ pub fn next(obj: PyObjectRef) -> PyResult { ); let item1 = match next(pyre_object::gc_roots::shadow_stack_get(iterator1_slot)) { Ok(item) => item, - Err(second_stop) if second_stop.kind == PyErrorKind::StopIteration => { + Err(second_stop) if second_stop.matches_stop_iteration() => { if zo::w_zip_get_strict(pyre_object::gc_roots::shadow_stack_get(obj_slot)) { return Err(strict_zip_error("zip", 1, "shorter")); } @@ -16725,7 +16796,7 @@ pub fn next(obj: PyObjectRef) -> PyResult { pyre_object::w_list_append(it.saved, w_obj); return Ok(w_obj); } - Err(e) if e.kind == PyErrorKind::StopIteration => { + Err(e) if e.matches_stop_iteration() => { it.index = 1; if pyre_object::w_list_len(it.saved) == 0 { return Err(PyError::stop_iteration()); @@ -16777,7 +16848,7 @@ pub fn next(obj: PyObjectRef) -> PyResult { // `w_iterables = None` before re-raising. let w_iterable = match next(w_iterables) { Ok(w) => w, - Err(e) if e.kind == PyErrorKind::StopIteration => { + Err(e) if e.matches_stop_iteration() => { let w_self = pyre_object::gc_roots::shadow_stack_get(obj_slot); pyre_object::interp_itertools::w_chain_set_iterables( w_self, @@ -16816,7 +16887,7 @@ pub fn next(obj: PyObjectRef) -> PyResult { let w_it = pyre_object::interp_itertools::w_chain_get_it(w_self); match next(w_it) { Ok(w_obj) => return Ok(w_obj), - Err(e) if e.kind == PyErrorKind::StopIteration => { + Err(e) if e.matches_stop_iteration() => { // Sub-iterator exhausted — advance to the next iterable. let w_self = pyre_object::gc_roots::shadow_stack_get(obj_slot); pyre_object::interp_itertools::w_chain_set_it(w_self, std::ptr::null_mut()); @@ -16959,7 +17030,7 @@ pub fn next(obj: PyObjectRef) -> PyResult { } let result = match crate::call::call_function_impl_result(callable, &[]) { Ok(r) => r, - Err(e) if e.kind == crate::PyErrorKind::StopIteration => { + Err(e) if e.matches_stop_iteration() => { // `calliter_iternext`: when the callable itself raises // `StopIteration`, latch `it_callable` to `PY_NULL` so // further `next()` stays stopped. The callable's @@ -17118,8 +17189,7 @@ pub fn next(obj: PyObjectRef) -> PyResult { let obj = pyre_object::gc_roots::shadow_stack_get(obj_slot); ro::w_reversed_set_remaining(obj, -1); ro::w_reversed_set_sequence(obj, pyre_object::PY_NULL); - if e.kind == PyErrorKind::IndexError || e.kind == PyErrorKind::StopIteration - { + if e.kind == PyErrorKind::IndexError || e.matches_stop_iteration() { return Err(PyError::stop_iteration()); } return Err(e); @@ -17448,11 +17518,12 @@ unsafe fn generator_invoke_execute_frame( // `_leak_stopasynciteration`, which differ only in the name they // format after KIND. The second is reachable on async generators // alone, which is why it tests the flavour and the first does not. - let leaked = if e.kind == crate::PyErrorKind::StopIteration { + // generator.py:135-139 selects between them with `e.match(space, + // ...)`, so a subclass of either class leaks the same way its base + // does and a flat `PyErrorKind` comparison would miss it. + let leaked = if e.matches_stop_iteration() { Some("StopIteration") - } else if is_async_generator(gen_obj) - && e.kind == crate::PyErrorKind::StopAsyncIteration - { + } else if is_async_generator(gen_obj) && e.matches_stop_async_iteration() { Some("StopAsyncIteration") } else { None @@ -17612,7 +17683,7 @@ pub(crate) fn resume_yield_from( } Ok(Some(value)) } - Err(err) if err.kind == PyErrorKind::StopIteration => { + Err(err) if err.matches_stop_iteration() => { frame.w_yielding_from = pyre_object::PY_NULL; finish_yield_from(frame, err)?; Ok(None) @@ -17663,8 +17734,7 @@ fn close_yield_from(w_yf: PyObjectRef) -> PyResult { generator_kind(w_yf) ))), Err(err) - if err.kind == PyErrorKind::StopIteration - || err.kind == PyErrorKind::GeneratorExit => + if err.matches_stop_iteration() || err.kind == PyErrorKind::GeneratorExit => { Ok(w_none()) } @@ -18068,7 +18138,7 @@ pub(crate) fn generator_close_method(args: &[PyObjectRef]) -> PyResult { unsafe { generator_kind(gen_obj) } ))) } - Err(mut e) if e.kind == PyErrorKind::StopIteration => { + Err(mut e) if e.matches_stop_iteration() => { // Python 3.13+ / 3.14: close() returns the value produced when // GeneratorExit is caught and the generator executes `return x`. let w_exc = e.to_exc_object(); @@ -19009,7 +19079,7 @@ pub(crate) fn contains_slot(haystack: PyObjectRef, needle: PyObjectRef) -> Resul return Ok(true); } } - Err(e) if e.kind == PyErrorKind::StopIteration => return Ok(false), + Err(e) if e.matches_stop_iteration() => return Ok(false), Err(e) => return Err(e), } } diff --git a/pyre/pyre-interpreter/src/builtins.rs b/pyre/pyre-interpreter/src/builtins.rs index 87c6741ee01..f27d1df99e0 100644 --- a/pyre/pyre-interpreter/src/builtins.rs +++ b/pyre/pyre-interpreter/src/builtins.rs @@ -2488,7 +2488,7 @@ fn memoryview_count(args: &[PyObjectRef]) -> Result count += 1; } } - Err(e) if e.kind == crate::PyErrorKind::StopIteration => break, + Err(e) if e.matches_stop_iteration() => break, Err(e) => return Err(e), } } @@ -5218,7 +5218,7 @@ fn min_max_sequence( let it_now = pyre_object::gc_roots::shadow_stack_get(iterator_slot); let item = match crate::baseobjspace::next(it_now) { Ok(item) => item, - Err(e) if e.kind == crate::PyErrorKind::StopIteration => break, + Err(e) if e.matches_stop_iteration() => break, Err(e) => return Err(e), }; pyre_object::gc_roots::shadow_stack_set(candidate_item_slot, item); @@ -10589,7 +10589,7 @@ pub(crate) fn collect_iterator(it: PyObjectRef) -> Result, crat pyre_object::gc_roots::pin_root(v); count += 1; } - Err(e) if e.kind == crate::PyErrorKind::StopIteration => break, + Err(e) if e.matches_stop_iteration() => break, Err(e) => return Err(e), } } @@ -10934,9 +10934,7 @@ fn builtin_next(args: &[PyObjectRef]) -> Result { default_root.pin_root(args[1]); return match crate::baseobjspace::next(args[0]) { Ok(v) => Ok(v), - Err(e) if e.kind == crate::PyErrorKind::StopIteration => { - Ok(default_root.get(default_base)) - } + Err(e) if e.matches_stop_iteration() => Ok(default_root.get(default_base)), Err(e) => Err(e), }; } @@ -14965,7 +14963,7 @@ fn builtin_any(args: &[PyObjectRef]) -> Result { match crate::baseobjspace::next(it_now) { Ok(item) if crate::baseobjspace::is_true(item)? => return Ok(w_bool_from(true)), Ok(_) => {} - Err(e) if e.kind == crate::PyErrorKind::StopIteration => return Ok(w_bool_from(false)), + Err(e) if e.matches_stop_iteration() => return Ok(w_bool_from(false)), Err(e) => return Err(e), } } @@ -17739,7 +17737,7 @@ fn builtin_all(args: &[PyObjectRef]) -> Result { match crate::baseobjspace::next(it_now) { Ok(item) if !crate::baseobjspace::is_true(item)? => return Ok(w_bool_from(false)), Ok(_) => {} - Err(e) if e.kind == crate::PyErrorKind::StopIteration => return Ok(w_bool_from(true)), + Err(e) if e.matches_stop_iteration() => return Ok(w_bool_from(true)), Err(e) => return Err(e), } } @@ -17879,7 +17877,7 @@ fn builtin_sum(args: &[PyObjectRef]) -> Result { roots.set(item_slot, v); *pending = true; } - Err(e) if e.kind == crate::PyErrorKind::StopIteration => *exhausted = true, + Err(e) if e.matches_stop_iteration() => *exhausted = true, Err(e) => return Err(e), } Ok(()) diff --git a/pyre/pyre-interpreter/src/cpyext/bytesobject.rs b/pyre/pyre-interpreter/src/cpyext/bytesobject.rs index 718ec800ebf..ee612d90dc5 100644 --- a/pyre/pyre-interpreter/src/cpyext/bytesobject.rs +++ b/pyre/pyre-interpreter/src/cpyext/bytesobject.rs @@ -358,7 +358,7 @@ pub(super) fn bytes_of(object: PyObjectRef) -> Result data.push(unsafe { crate::baseobjspace::byte_w(item, "bytes") }?), - Err(error) if error.kind == crate::PyErrorKind::StopIteration => break, + Err(error) if error.matches_stop_iteration() => break, Err(error) => return Err(error), } } diff --git a/pyre/pyre-interpreter/src/error.rs b/pyre/pyre-interpreter/src/error.rs index f9f9fbe00a4..6e7d67acbfa 100644 --- a/pyre/pyre-interpreter/src/error.rs +++ b/pyre/pyre-interpreter/src/error.rs @@ -16,6 +16,47 @@ pub struct OperationError { pub _application_traceback: Option, } +/// Test whether a live exception object is an instance of StopIteration. +/// The class cache is populated only after registry lookup succeeds, so a +/// call before exception-class initialisation returns false without caching +/// absence forever. +#[majit_macros::dont_look_inside] +pub fn exception_object_matches_stop_iteration(exc_object: PyObjectRef) -> bool { + static STOP_ITERATION_CLASS: OnceLock = OnceLock::new(); + let stop_iteration = match STOP_ITERATION_CLASS.get() { + Some(&class) => class as PyObjectRef, + None => { + let Some(class) = crate::builtins::lookup_exc_class("StopIteration") else { + return false; + }; + let _ = STOP_ITERATION_CLASS.set(class as usize); + class + } + }; + crate::eval::check_exc_match_against(exc_object, stop_iteration) +} + +/// Test whether a live exception object is an instance of StopAsyncIteration. +/// Deliberately a separate function rather than a shared parameterised lookup: +/// `exception_object_matches_stop_iteration` is recognised by its body shape in +/// `majit-translate front::result_exc` and addressed by its symbol path in +/// `jit_fnaddr`, both of which a refactor would break silently. +#[majit_macros::dont_look_inside] +pub fn exception_object_matches_stop_async_iteration(exc_object: PyObjectRef) -> bool { + static STOP_ASYNC_ITERATION_CLASS: OnceLock = OnceLock::new(); + let stop_async_iteration = match STOP_ASYNC_ITERATION_CLASS.get() { + Some(&class) => class as PyObjectRef, + None => { + let Some(class) = crate::builtins::lookup_exc_class("StopAsyncIteration") else { + return false; + }; + let _ = STOP_ASYNC_ITERATION_CLASS.set(class as usize); + class + } + }; + crate::eval::check_exc_match_against(exc_object, stop_async_iteration) +} + impl OperationError { pub fn new(w_type: PyObjectRef, w_value: PyObjectRef) -> Self { Self { @@ -574,6 +615,39 @@ impl PyError { } } + /// `pypy/interpreter/pyopcode.py:1303-1316` tests iterator exhaustion with + /// a Python-level MRO match. A flat `PyErrorKind` tag cannot express + /// multiple inheritance, so only exact-tagged errors use the free fast + /// path. An internally built error has no cached exception object and can + /// only name a builtin, making its tag authoritative without materialising + /// an object. This takes `&self` so callers can use it in match guards. + /// The slow-path class cache is populated only after registry lookup + /// succeeds, so a call before exception-class initialisation returns false + /// without caching absence forever. + pub fn matches_stop_iteration(&self) -> bool { + if self.kind == PyErrorKind::StopIteration { + return true; + } + if self.exc_object.is_null() { + return false; + } + exception_object_matches_stop_iteration(self.exc_object) + } + + /// The StopAsyncIteration twin of [`matches_stop_iteration`], with the same + /// tag fast path and object slow path. + /// + /// [`matches_stop_iteration`]: Self::matches_stop_iteration + pub fn matches_stop_async_iteration(&self) -> bool { + if self.kind == PyErrorKind::StopAsyncIteration { + return true; + } + if self.exc_object.is_null() { + return false; + } + exception_object_matches_stop_async_iteration(self.exc_object) + } + pub fn type_error(msg: impl Into) -> Self { Self::new(PyErrorKind::TypeError, msg) } diff --git a/pyre/pyre-interpreter/src/eval.rs b/pyre/pyre-interpreter/src/eval.rs index 67bab1e7af0..c3891c61623 100644 --- a/pyre/pyre-interpreter/src/eval.rs +++ b/pyre/pyre-interpreter/src/eval.rs @@ -3046,7 +3046,7 @@ impl IterOpcodeHandler for PyFrame { // iter_next), not by branching the interpreter opcode implementation. match crate::baseobjspace::next(iter) { Ok(result) => Ok(Some(result)), - Err(e) if e.kind == PyErrorKind::StopIteration => Ok(None), + Err(e) if e.matches_stop_iteration() => Ok(None), Err(e) => Err(e), } } @@ -3443,7 +3443,7 @@ impl OpcodeStepExecutor for PyFrame { fn cleanup_throw(&mut self) -> Result<(), PyError> { let w_exc = self.pop_value()?; let mut err = unsafe { PyError::from_exc_object(w_exc) }; - if err.kind != PyErrorKind::StopIteration { + if !err.matches_stop_iteration() { // CPython 3.14 `CLEANUP_THROW` installs the existing exception and // jumps straight to `exception_unwind`; unlike the ordinary // opcode-error path it does not prepend another traceback entry. @@ -4520,7 +4520,7 @@ impl OpcodeStepExecutor for PyFrame { self.push(result); Ok(()) } - Err(e) if e.kind == crate::PyErrorKind::StopIteration => { + Err(e) if e.matches_stop_iteration() => { if std::ptr::eq(self.w_yielding_from, iter) { self.w_yielding_from = pyre_object::PY_NULL; } @@ -5221,6 +5221,21 @@ mod tests { assert!(!check_exc_match_against(plain, value_error)); } + #[test] + fn test_pyerror_matches_stop_iteration_uses_exception_mro() { + let (result, frame) = + run_exec_frame("class VS(ValueError, StopIteration):\n pass\nexc = VS('done')"); + result.expect("exception subclass setup failed"); + let exc = unsafe { pyre_object::w_dict_getitem_str(frame.get_w_globals(), "exc") } + .expect("missing exc"); + let err = unsafe { PyError::from_exc_object(exc) }; + + assert_eq!(err.kind, PyErrorKind::ValueError); + assert!(err.matches_stop_iteration()); + assert!(PyError::stop_iteration().matches_stop_iteration()); + assert!(!PyError::value_error("not exhausted").matches_stop_iteration()); + } + // pyre materialises the rich-compare and `__iter__` rows in // `UnionType.__dict__`; CPython 3.14 leaves them to the slot table, so // `required <= UT.__dict__.keys()` is false there. Divergent by design, diff --git a/pyre/pyre-interpreter/src/importing.rs b/pyre/pyre-interpreter/src/importing.rs index aff719ea05c..4eee318dd39 100644 --- a/pyre/pyre-interpreter/src/importing.rs +++ b/pyre/pyre-interpreter/src/importing.rs @@ -5692,7 +5692,7 @@ where loop { let w_name = match crate::baseobjspace::next(w_iter) { Ok(v) => v, - Err(e) if e.kind == crate::PyErrorKind::StopIteration => break, + Err(e) if e.matches_stop_iteration() => break, Err(e) => return Err(e), }; // pyopcode.py:2240-2255 — per-name str check. diff --git a/pyre/pyre-interpreter/src/jit_fnaddr.rs b/pyre/pyre-interpreter/src/jit_fnaddr.rs index 64b4132ee08..f3f08fe28ba 100644 --- a/pyre/pyre-interpreter/src/jit_fnaddr.rs +++ b/pyre/pyre-interpreter/src/jit_fnaddr.rs @@ -681,6 +681,17 @@ pub fn jit_trace_fnaddrs() -> Vec<(&'static str, i64)> { "pyre_object::exc_kind_discriminant", crate::opcode_ops::bh_w_exception_get_kind as *const (), ); + // `exception_object_matches_stop_iteration` performs the cached + // StopIteration class lookup and MRO match for the caught exception + // object. Its residual call rides a C-ABI bridge that returns the boolean + // in the integer result slot. Emitted by `try_fuse_drain_match` for the + // drain loop's exception-edge subclass test. + push_alias_pair( + &mut entries, + "pyre_interpreter::error::exception_object_matches_stop_iteration", + "pyre_interpreter::exception_object_matches_stop_iteration", + crate::opcode_ops::bh_exception_object_matches_stop_iteration as *const (), + ); // `pin_root` pushes onto the TLS `SHADOW_STACK` (the `shadow_stack_len` // twin), `dereference` reads the weakref `w_obj_weak` slot // (`@jit.dont_look_inside` upstream, the `proxy_type` twin), and diff --git a/pyre/pyre-interpreter/src/module/_abc/mod.rs b/pyre/pyre-interpreter/src/module/_abc/mod.rs index a79978cc25b..e11f7ffa477 100644 --- a/pyre/pyre-interpreter/src/module/_abc/mod.rs +++ b/pyre/pyre-interpreter/src/module/_abc/mod.rs @@ -395,7 +395,7 @@ fn subclass_of(cls: PyObjectRef, subclass: PyObjectRef) -> Result rcls, - Err(err) if err.kind == crate::PyErrorKind::StopIteration => break, + Err(err) if err.matches_stop_iteration() => break, Err(err) => return Err(err), }; // A registered entry that is not a class cannot be a base @@ -436,7 +436,7 @@ fn subclass_of(cls: PyObjectRef, subclass: PyObjectRef) -> Result scls, - Err(err) if err.kind == crate::PyErrorKind::StopIteration => break, + Err(err) if err.matches_stop_iteration() => break, Err(err) => return Err(err), }; let item_roots = pyre_object::gc_roots::push_roots(); diff --git a/pyre/pyre-interpreter/src/module/_csv/mod.rs b/pyre/pyre-interpreter/src/module/_csv/mod.rs index 2d2272b9172..1a68508f95f 100644 --- a/pyre/pyre-interpreter/src/module/_csv/mod.rs +++ b/pyre/pyre-interpreter/src/module/_csv/mod.rs @@ -689,7 +689,7 @@ fn reader_next_inner(self_obj: PyObjectRef) -> Result { let w_iter = gc_roots::shadow_stack_get(iter_slot); let line = match crate::baseobjspace::next(w_iter) { Ok(l) => l, - Err(e) if e.kind == crate::PyErrorKind::StopIteration => { + Err(e) if e.matches_stop_iteration() => { if state != START_RECORD && state != EAT_CRNL && (field_len > 0 || state == IN_QUOTED_FIELD) @@ -1062,7 +1062,7 @@ fn writer_writerows_impl( let it = gc_roots::shadow_stack_get(it_slot); let row = match crate::baseobjspace::next(it) { Ok(r) => r, - Err(e) if e.kind == crate::PyErrorKind::StopIteration => break, + Err(e) if e.matches_stop_iteration() => break, Err(e) => return Err(e), }; writer_writerow_impl(gc_roots::shadow_stack_get(self_slot), row)?; diff --git a/pyre/pyre-interpreter/src/module/_functools/mod.rs b/pyre/pyre-interpreter/src/module/_functools/mod.rs index c053ecdf9e6..ad291dca9a1 100644 --- a/pyre/pyre-interpreter/src/module/_functools/mod.rs +++ b/pyre/pyre-interpreter/src/module/_functools/mod.rs @@ -169,7 +169,7 @@ fn reduce(args: &[PyObjectRef]) -> crate::PyResult { } else { match crate::baseobjspace::next(pyre_object::gc_roots::shadow_stack_get(iter_slot)) { Ok(value) => value, - Err(err) if err.kind == crate::PyErrorKind::StopIteration => { + Err(err) if err.matches_stop_iteration() => { return Err(crate::PyError::type_error( "reduce() of empty iterable with no initial value", )); @@ -190,7 +190,7 @@ fn reduce(args: &[PyObjectRef]) -> crate::PyResult { let item = match crate::baseobjspace::next(pyre_object::gc_roots::shadow_stack_get(iter_slot)) { Ok(value) => value, - Err(err) if err.kind == crate::PyErrorKind::StopIteration => break, + Err(err) if err.matches_stop_iteration() => break, Err(err) => return Err(err), }; // `next()` returns a raw object reference. Keep this iteration's diff --git a/pyre/pyre-interpreter/src/module/_io/mod.rs b/pyre/pyre-interpreter/src/module/_io/mod.rs index 04751c69a49..19ecce08428 100644 --- a/pyre/pyre-interpreter/src/module/_io/mod.rs +++ b/pyre/pyre-interpreter/src/module/_io/mod.rs @@ -635,7 +635,7 @@ pub(crate) fn iobase_writelines(args: &[PyObjectRef]) -> crate::PyResult { let iterator = pyre_object::gc_roots::shadow_stack_get(sp + 2); let line = match crate::baseobjspace::next(iterator) { Ok(line) => line, - Err(err) if err.kind == crate::PyErrorKind::StopIteration => break, + Err(err) if err.matches_stop_iteration() => break, Err(err) => return Err(err), }; let _line_root = pyre_object::gc_roots::push_roots(); @@ -758,7 +758,7 @@ pub(super) fn iobase_readlines(args: &[PyObjectRef]) -> crate::PyResult { loop { let line = match crate::baseobjspace::next(pyre_object::gc_roots::shadow_stack_get(sp)) { Ok(line) => line, - Err(error) if error.kind == crate::PyErrorKind::StopIteration => break, + Err(error) if error.matches_stop_iteration() => break, Err(error) => return Err(error), }; length = length.saturating_add(crate::baseobjspace::len_w(line)?); diff --git a/pyre/pyre-interpreter/src/module/_json/mod.rs b/pyre/pyre-interpreter/src/module/_json/mod.rs index 438bc456709..f02ff2fceb6 100644 --- a/pyre/pyre-interpreter/src/module/_json/mod.rs +++ b/pyre/pyre-interpreter/src/module/_json/mod.rs @@ -411,7 +411,7 @@ fn scanner_parse_object( byte_index, ) .map_err(|err| { - if err.kind == crate::PyErrorKind::StopIteration { + if err.matches_stop_iteration() { scanner_decode_error( "Expecting value", gc_roots::shadow_stack_get(slot + 2), @@ -543,7 +543,7 @@ fn scanner_parse_array( byte_index, ) .map_err(|err| { - if err.kind == crate::PyErrorKind::StopIteration { + if err.matches_stop_iteration() { scanner_decode_error( "Expecting value", gc_roots::shadow_stack_get(slot + 2), @@ -1015,7 +1015,7 @@ fn encode_sequence( loop { let item = match crate::baseobjspace::next(gc_roots::shadow_stack_get(iter_slot)) { Ok(item) => item, - Err(err) if err.kind == crate::PyErrorKind::StopIteration => break, + Err(err) if err.matches_stop_iteration() => break, Err(err) => return Err(err), }; if first { @@ -1130,7 +1130,7 @@ fn encode_dict( loop { let pair = match crate::baseobjspace::next(gc_roots::shadow_stack_get(iter_slot)) { Ok(pair) => pair, - Err(err) if err.kind == crate::PyErrorKind::StopIteration => break, + Err(err) if err.matches_stop_iteration() => break, Err(err) => return Err(err), }; let pair_items = crate::builtins::collect_iterable(pair)?; diff --git a/pyre/pyre-interpreter/src/module/_pickle/pickler.rs b/pyre/pyre-interpreter/src/module/_pickle/pickler.rs index 5408ed135c8..6634815454a 100644 --- a/pyre/pyre-interpreter/src/module/_pickle/pickler.rs +++ b/pyre/pyre-interpreter/src/module/_pickle/pickler.rs @@ -2206,7 +2206,7 @@ fn pinned_iter_next(iter_slot: usize) -> Result, PyError> { pyre_object::gc_roots::pin_root(item); Ok(Some(pyre_object::gc_roots::shadow_stack_len() - 1)) } - Err(e) if e.kind == crate::PyErrorKind::StopIteration => Ok(None), + Err(e) if e.matches_stop_iteration() => Ok(None), Err(e) => Err(e), } } @@ -2225,7 +2225,7 @@ fn snapshot_pinned_iterable(source_slot: usize) -> Result { let item = match crate::baseobjspace::next(pyre_object::gc_roots::shadow_stack_get(iter_slot)) { Ok(item) => item, - Err(e) if e.kind == crate::PyErrorKind::StopIteration => break, + Err(e) if e.matches_stop_iteration() => break, Err(e) => return Err(e), }; { @@ -2350,7 +2350,7 @@ fn batch_appends( fn pinned_pair_next(iter_slot: usize) -> Result, PyError> { let item = match crate::baseobjspace::next(pyre_object::gc_roots::shadow_stack_get(iter_slot)) { Ok(item) => item, - Err(e) if e.kind == crate::PyErrorKind::StopIteration => return Ok(None), + Err(e) if e.matches_stop_iteration() => return Ok(None), Err(e) => return Err(e), }; pyre_object::gc_roots::pin_root(item); diff --git a/pyre/pyre-interpreter/src/module/_pickle/unpickler.rs b/pyre/pyre-interpreter/src/module/_pickle/unpickler.rs index d7fab684cc1..c768865fd32 100644 --- a/pyre/pyre-interpreter/src/module/_pickle/unpickler.rs +++ b/pyre/pyre-interpreter/src/module/_pickle/unpickler.rs @@ -737,7 +737,7 @@ fn load_next_buffer(slot: usize) -> Result<(), PyError> { } let w_buf = match crate::baseobjspace::next(w_buffers) { Ok(b) => b, - Err(e) if e.kind == crate::PyErrorKind::StopIteration => { + Err(e) if e.matches_stop_iteration() => { return Err(unpickling_error("not enough out-of-band buffers")); } Err(e) => return Err(e), diff --git a/pyre/pyre-interpreter/src/module/_sre/interp_sre.rs b/pyre/pyre-interpreter/src/module/_sre/interp_sre.rs index 6f62cf60ee2..d2168ce03c2 100644 --- a/pyre/pyre-interpreter/src/module/_sre/interp_sre.rs +++ b/pyre/pyre-interpreter/src/module/_sre/interp_sre.rs @@ -1810,7 +1810,7 @@ fn sre_match_groupdict(args: &[PyObjectRef]) -> Result RootedObject::pin(k), - Err(e) if e.kind == crate::PyErrorKind::StopIteration => break, + Err(e) if e.matches_stop_iteration() => break, Err(e) => return Err(e), }; let w_value = RootedObject::pin(crate::baseobjspace::getitem( diff --git a/pyre/pyre-interpreter/src/module/_tokenize/mod.rs b/pyre/pyre-interpreter/src/module/_tokenize/mod.rs index 6fd164943bd..ec35fc5b84f 100644 --- a/pyre/pyre-interpreter/src/module/_tokenize/mod.rs +++ b/pyre/pyre-interpreter/src/module/_tokenize/mod.rs @@ -115,7 +115,7 @@ fn read_line(self_obj: PyObjectRef) -> Result { &[], ) { Ok(value) => value, - Err(err) if err.kind == crate::PyErrorKind::StopIteration => return Ok(String::new()), + Err(err) if err.matches_stop_iteration() => return Ok(String::new()), Err(err) => return Err(err), }; match encoding { diff --git a/pyre/pyre-interpreter/src/module/array/mod.rs b/pyre/pyre-interpreter/src/module/array/mod.rs index 26bd3073954..90c0ac3ae7c 100644 --- a/pyre/pyre-interpreter/src/module/array/mod.rs +++ b/pyre/pyre-interpreter/src/module/array/mod.rs @@ -198,7 +198,7 @@ fn array_extend_iterable( loop { match crate::baseobjspace::next(w_iter) { Ok(w_item) => array_append(obj, w_item)?, - Err(e) if e.kind == PyErrorKind::StopIteration => break, + Err(e) if e.matches_stop_iteration() => break, Err(e) => return Err(e), } } diff --git a/pyre/pyre-interpreter/src/module/math/interp_math.rs b/pyre/pyre-interpreter/src/module/math/interp_math.rs index 59e5e9b4ea4..923a40996ab 100644 --- a/pyre/pyre-interpreter/src/module/math/interp_math.rs +++ b/pyre/pyre-interpreter/src/module/math/interp_math.rs @@ -1075,7 +1075,7 @@ pub fn fsum(args: &[PyObjectRef]) -> PyResult { let w_value = match crate::baseobjspace::next(pyre_object::gc_roots::shadow_stack_get(iter_slot)) { Ok(value) => value, - Err(err) if err.kind == crate::PyErrorKind::StopIteration => break, + Err(err) if err.matches_stop_iteration() => break, Err(err) => return Err(err), }; // `_get_double` can invoke user code. Keep the yielded object rooted diff --git a/pyre/pyre-interpreter/src/opcode_ops.rs b/pyre/pyre-interpreter/src/opcode_ops.rs index fb80f6eb95d..57a45c18392 100644 --- a/pyre/pyre-interpreter/src/opcode_ops.rs +++ b/pyre/pyre-interpreter/src/opcode_ops.rs @@ -1152,6 +1152,16 @@ pub extern "C" fn bh_w_exception_get_kind(evalue: pyre_object::PyObjectRef) -> i pyre_object::interp_exceptions::exc_kind_discriminant(evalue) } +/// C-ABI residual bridge for `exception_object_matches_stop_iteration`: the +/// caught exception value rides in as a `PyObjectRef`; its boolean result rides +/// back in the integer result slot. +#[allow(improper_ctypes_definitions)] +pub extern "C" fn bh_exception_object_matches_stop_iteration( + evalue: pyre_object::PyObjectRef, +) -> i64 { + crate::error::exception_object_matches_stop_iteration(evalue) as i64 +} + #[cfg(test)] mod tests { use super::*; diff --git a/pyre/pyre-interpreter/src/runtime_ops.rs b/pyre/pyre-interpreter/src/runtime_ops.rs index 6161e4b33cd..d83d85a6eb2 100644 --- a/pyre/pyre-interpreter/src/runtime_ops.rs +++ b/pyre/pyre-interpreter/src/runtime_ops.rs @@ -1417,7 +1417,7 @@ pub fn unpack_sequence_exact(seq: PyObjectRef, count: usize) -> Result break, + Err(e) if e.matches_stop_iteration() => break, Err(e) if e.kind == PyErrorKind::TypeError => return Err(non_iterable()), Err(e) => return Err(e), } @@ -1763,7 +1763,7 @@ pub fn via_space_next(iter: PyObjectRef) -> bool { /// Exhaustion is signalled the same way as the range fast path: a null /// return that the trailing for-iter GuardNonnull catches, side-exiting to /// the interpreter which re-runs FOR_ITER and ends the loop (eval.rs -/// `iter_next` maps StopIteration to exhausted). A *real* exception is +/// `iter_next` MRO-matches StopIteration to exhausted). A *real* exception is /// published into BOTH the backend exception cells (so the compiled trace / /// blackhole GuardNoException side-exits) AND `BH_LAST_EXC_VALUE` (so the /// full-body walk's `execute_residual_call` returns Err and records the @@ -1776,7 +1776,7 @@ pub extern "C" fn jit_next(iter: i64) -> i64 { Ok(value) => value as i64, // StopIteration is not a frame-level exception for FOR_ITER; return // null so the GuardNonnull (not GuardNoException) fires. - Err(err) if err.kind == PyErrorKind::StopIteration => 0, + Err(err) if err.matches_stop_iteration() => 0, Err(mut err) => { let exc_obj = err.to_exc_object(); if exc_obj != PY_NULL { @@ -1788,6 +1788,15 @@ pub extern "C" fn jit_next(iter: i64) -> i64 { } } +/// `pyopcode.py:1303-1316` FOR_ITER exception discrimination: +/// `e.match(space, space.w_StopIteration)`. The caught object and match class +/// are Python-level objects, so use the same MRO-aware helper as +/// CHECK_EXC_MATCH. This is infallible and deliberately never publishes a +/// backend or blackhole exception. +pub extern "C" fn jit_exception_match(exc: i64, match_class: i64) -> i64 { + crate::eval::check_exc_match_against(exc as PyObjectRef, match_class as PyObjectRef) as i64 +} + /// Ref-returning bridge for the `next(w_iterator)` residual call in /// `_unpackiterable_unknown_length` (the `unpackiterable_driver` portal). /// diff --git a/pyre/pyre-interpreter/src/type_methods.rs b/pyre/pyre-interpreter/src/type_methods.rs index 461ea038e55..1bcb2416acb 100644 --- a/pyre/pyre-interpreter/src/type_methods.rs +++ b/pyre/pyre-interpreter/src/type_methods.rs @@ -683,7 +683,7 @@ pub fn list_method_extend(args: &[PyObjectRef]) -> Result item, - Err(err) if err.kind == crate::PyErrorKind::StopIteration => break, + Err(err) if err.matches_stop_iteration() => break, Err(err) => return Err(err), }; let _item_roots = pyre_object::gc_roots::push_roots(); diff --git a/pyre/pyre-interpreter/src/typedef.rs b/pyre/pyre-interpreter/src/typedef.rs index 7ec9d5d453a..673b42999fd 100644 --- a/pyre/pyre-interpreter/src/typedef.rs +++ b/pyre/pyre-interpreter/src/typedef.rs @@ -20803,7 +20803,7 @@ fn bytearray_descr_init_value( vec.push(byte); pyre_object::bytearrayobject::w_bytearray_sync_alloc(target, old_size); } - Err(e) if e.kind == crate::PyErrorKind::StopIteration => break, + Err(e) if e.matches_stop_iteration() => break, Err(e) => return Err(e), } } @@ -24122,7 +24122,7 @@ fn bytes_descr_new_impl(args: &[PyObjectRef]) -> Result buf.push(crate::baseobjspace::byte_w(item, "bytes")?), - Err(e) if e.kind == crate::PyErrorKind::StopIteration => break, + Err(e) if e.matches_stop_iteration() => break, Err(e) => return Err(e), } } @@ -24246,7 +24246,7 @@ fn bytearray_method_extend(args: &[PyObjectRef]) -> Result break, + Err(e) if e.matches_stop_iteration() => break, Err(e) => return Err(e), } } diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rs index 23ef8f4bc4d..ce75623139e 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/bridge_subwalk.rs @@ -34,7 +34,7 @@ fn record_bridge_handler_entry_traceback( exc: OpRef, exc_concrete: ConcreteValue, position: usize, -) { +) -> Result<(), DispatchError> { // The handler is part of the trace, so once this bridge runs compiled it // catches the exception itself and the frame never surfaces an error the // interpreter's `handle_exception` could record a node from — hence the @@ -44,9 +44,10 @@ fn record_bridge_handler_entry_traceback( // recorders journal their own attach, so a walk that is later discarded // does not leave the node behind for the metainterp's own delivery to // record on top of. - let emit_runtime = !record_prepend_application_traceback(wc, exc, exc_concrete, position); + let emit_runtime = !record_prepend_application_traceback(wc, exc, exc_concrete, position)?; record_inline_application_traceback(wc, exc, exc_concrete, position, true, emit_runtime); record_top_level_application_traceback(wc, exc, exc_concrete, position, true, emit_runtime); + Ok(()) } /// `executioncontext.py:91-107 leave` for a frame the bridge resumed into @@ -515,7 +516,7 @@ pub fn dispatch_via_miframe( value_op, ConcreteValue::Ref(exc_edge_concrete), position, - ); + )?; // Reconstruct the handler-entry operand stack + push the exc box on // the new TOS (mirrors the mid-walk SubRaise catch routing). vstack_enter_exception_handler(&mut wc, catch_target, value_op); @@ -550,7 +551,7 @@ pub fn dispatch_via_miframe( seed.exc, seed.exc_concrete, position, - ); + )?; vstack_enter_exception_handler(&mut wc, catch_target, seed.exc); catch_target } else { @@ -577,7 +578,7 @@ pub fn dispatch_via_miframe( // holding whatever its entry wrote, and a `tb_frame.f_locals` // or `sys._getframe()` on the way out reads every // post-entry local as unbound. - if !recording_instruction_is_bare_reraise(&mut wc, position) { + if !recording_raise_keeps_existing_traceback(&mut wc, position) { record_top_level_application_traceback( &mut wc, seed.exc, diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/diag.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/diag.rs index c9deec667ff..dfff402c46f 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/diag.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/diag.rs @@ -247,7 +247,7 @@ pub fn skip_python_trivia_forward(code: &pyre_interpreter::CodeObject, mut py_pc /// `parent` marks the second row as a split of the first so the reader does not /// sum them. #[rustfmt::skip] -pub const SPEC_FOLD_ROWS: [(&str, &str, &str); 55] = [ +pub const SPEC_FOLD_ROWS: [(&str, &str, &str); 56] = [ // (label, site, parent) ("truth_int", "residual_call", "-"), ("truth_bool", "residual_call", "-"), @@ -304,6 +304,7 @@ pub const SPEC_FOLD_ROWS: [(&str, &str, &str); 55] = [ ("subscr_specialised_pair", "specialize", "subscr"), ("builtin_divmod_long_int", "specialize", "builtin_divmod"), ("zip_two_tuple_iters", "specialize", "for_iter_next"), + ("instance_next", "residual_call", "-"), ]; const SPEC_FOLD_COUNT: usize = SPEC_FOLD_ROWS.len(); @@ -327,6 +328,12 @@ static SPEC_SUPPRESSED: [std::sync::atomic::AtomicU64; SPEC_FOLD_COUNT] = { /// value means a typo at a call site; the summary reports it rather than /// silently dropping the row. static SPEC_UNKNOWN: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); +static INSTANCE_NEXT_FORITER_ROUTE_GUARDS_KEYED: std::sync::atomic::AtomicU64 = + std::sync::atomic::AtomicU64::new(0); +static INSTANCE_NEXT_FORITER_CALLEE_GUARDS_CAPTURED: std::sync::atomic::AtomicU64 = + std::sync::atomic::AtomicU64::new(0); +static INSTANCE_NEXT_FORITER_CALLEE_GUARDS_KEYED: std::sync::atomic::AtomicU64 = + std::sync::atomic::AtomicU64::new(0); /// `PYRE_FBW_SPEC_CENSUS`: per-fold consulted/fired tallies for the /// hand-written trace-time specializations. Off by default; the gated branch @@ -336,6 +343,23 @@ pub(crate) fn fbw_spec_census_enabled() -> bool { *ENABLED.get_or_init(|| std::env::var_os("PYRE_FBW_SPEC_CENSUS").is_some()) } +pub(crate) fn spec_census_record_instance_next_route_guard_keyed() { + if fbw_spec_census_enabled() { + INSTANCE_NEXT_FORITER_ROUTE_GUARDS_KEYED.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + } +} + +pub(crate) fn spec_census_record_instance_next_callee_guard(keyed: bool) { + if !fbw_spec_census_enabled() { + return; + } + let ordering = std::sync::atomic::Ordering::Relaxed; + INSTANCE_NEXT_FORITER_CALLEE_GUARDS_CAPTURED.fetch_add(1, ordering); + if keyed { + INSTANCE_NEXT_FORITER_CALLEE_GUARDS_KEYED.fetch_add(1, ordering); + } +} + const FBW_DEPTH_HIST_BUCKETS: usize = 32; static FBW_DEPTH_ENTRIES: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); @@ -408,7 +432,7 @@ pub fn fbw_depth_census_summary() -> String { } /// Parse `PYRE_FBW_NO_SPECIALIZE` once into table-index bits and unknown -/// selector tokens. The reserved `all` token turns off these 55 rows and +/// selector tokens. The reserved `all` token turns off these 56 rows and /// nothing else: the `try_walker_fold_*` trio and the 11 /// `try_walker_inline_*` descent entry points all stay live. fn spec_suppression() -> &'static (u64, Vec) { @@ -594,6 +618,12 @@ pub fn spec_census_summary() -> String { rows.len(), SPEC_UNKNOWN.load(ordering), ); + summary.push_str(&format!( + "[spec-census] instance_next_foriter route_guards_keyed={} callee_guards_captured={} callee_guards_keyed={}\n", + INSTANCE_NEXT_FORITER_ROUTE_GUARDS_KEYED.load(ordering), + INSTANCE_NEXT_FORITER_CALLEE_GUARDS_CAPTURED.load(ordering), + INSTANCE_NEXT_FORITER_CALLEE_GUARDS_KEYED.load(ordering), + )); for (label, site, parent, consulted, fired, suppressed) in rows { summary.push_str(&format!( "[spec-census] fold={label} consulted={consulted} fired={fired} suppressed={suppressed} site={site} parent={parent}\n" diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs index 655c75af2c8..10ad24c9747 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/fbw_state.rs @@ -2577,10 +2577,11 @@ pub(crate) fn fbw_callee_body_replay_safety( // nothing to the live heap. The BUILD_TUPLE / BUILD_LIST array // consumers are the same shape one level up: they read a // freshly-built backing array and return a brand-new container. - // `get_current_exception` is the PUSH_EXC_INFO `prev` save, which + // `get_current_exception` is the PUSH_EXC_INFO `prev` save and the + // catch-covered bare `raise` read, which // `try_walker_lower_exc_info_residual` lowers to a bare - // `GETFIELD_GC_R(ec, sys_exc_value)`: a field read, so a replay - // reads the same value again. Its writing twin + // `GETFIELD_GC_R(ec, sys_exc_value)` either way: a field read, so a + // replay reads the same value again. Its writing twin // `SetCurrentException` is not here — it is journalled, and so // reaches the `deferred_call` arm below instead. // `load_deref` is that same shape once more, and it is the one every diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs index 2c870449348..b3d82110c73 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs @@ -2940,16 +2940,14 @@ pub(crate) fn try_walker_inline_builtin_call( } }; match walk_result { - DispatchOutcome::SubReturn { - result: Some(value), - } => { - let concrete = concrete_from_recorded_opref(ctx, value); - write_ref_reg(ctx, op.pc, dst, value, concrete)?; - Ok(Some((DispatchOutcome::Continue, op.next_pc))) - } - DispatchOutcome::SubReturn { result: None } => { - Err(DispatchError::UnexpectedVoidSubReturn { pc: op.pc }) - } + DispatchOutcome::SubReturn { result } => match finish_inline_callee_return(ctx, result) { + Some(value) => { + let concrete = concrete_from_recorded_opref(ctx, value); + write_ref_reg(ctx, op.pc, dst, value, concrete)?; + Ok(Some((DispatchOutcome::Continue, op.next_pc))) + } + None => Err(DispatchError::UnexpectedVoidSubReturn { pc: op.pc }), + }, DispatchOutcome::SubRaise { exc, exc_concrete } => { if let Some(target) = try_catch_exception_at(code, op.next_pc) { ctx.last_exc_value = Some(exc); @@ -3125,6 +3123,7 @@ pub(crate) fn try_walker_inline_resolved_user_call( constructor_result, None, false, + None, ) } @@ -3176,6 +3175,7 @@ fn try_walker_inline_resolved_user_call_inner( constructor_result: Option<(OpRef, ConcreteValue)>, intermediate_result: Option<&mut Option<(OpRef, ConcreteValue)>>, require_exact_int_result: bool, + instance_next_foriter_green_key: Option, ) -> Result, DispatchError> { // `_compute_flatcall` (`pycode.py:256-268`) leaves `fast_natural_arity` // HOPELESS for a `*args` / `**kwargs` / keyword-only callee. The general @@ -3557,7 +3557,9 @@ fn try_walker_inline_resolved_user_call_inner( // A legacy, unseeded inline sub-walk inside a FOR_ITER body resumes a guard // at the caller's CALL boundary, so deopt re-executes the whole callee. // Replaying a live-heap mutation would double it, so a Dirty body stays on - // the residual call path. + // the residual call path. The keyed instance-`__next__` route is excluded: + // it seeds the callee frame and resumes keyed guards through the multi-frame + // snapshot instead of replaying the caller boundary. // // A body whose only unproven ops are Python-level CALL residuals is // admitted too: this same gate re-runs for each callee the lever resolves @@ -3566,9 +3568,12 @@ fn try_walker_inline_resolved_user_call_inner( // deferred body commits nothing either. Without that the whole nest // declines — `helper(i)` calling `add(i, 1, 2)` residualizes both calls // per iteration, though each body on its own is pure arithmetic. + let instance_next_seeded_route = instance_next_foriter_green_key.is_some(); + // The keyed route bypasses the legacy caller-replay classification, but it + // does not inherit that route's DeferredCall admission. let mut foriter_deferred_admit = false; let mut foriter_dirty_bound = false; - if fbw_foriter_inflight_active() { + if fbw_foriter_inflight_active() && !instance_next_seeded_route { let safety = fbw_callee_body_replay_safety( body.code, &exact_numeric_args, @@ -3864,14 +3869,11 @@ fn try_walker_inline_resolved_user_call_inner( } else { fbw_max_multiframe_depth() }; - // FOR_ITER must re-execute its iterator protocol as one unit when a body - // guard fails. Its Clean-only admission above makes caller-boundary - // replay safe, and keeping the callee frame unseeded makes the terminating - // branch resume at FOR_ITER so the residual maps IndexError to exhaustion. - let force_caller_boundary_resume = - call_descr.get_extra_info().pyre_helper == majit_ir::PyreHelperKind::ForIterNext; - let try_multiframe = !force_caller_boundary_resume - && multiframe_eligible + // The instance-`__next__` FOR_ITER route uses the same seeded-frame shape + // as other CALL-entered inlines. Its catch arm owns exception-to-exhaustion + // conversion, so neither replay safety nor an unseeded caller-boundary + // resume is part of that route's deopt discipline. + let try_multiframe = multiframe_eligible && inline_depth < effective_multiframe_depth && callee_fast_path_inlinable_allowing_forward_branch( body.code, @@ -3888,8 +3890,7 @@ fn try_walker_inline_resolved_user_call_inner( // descr_call` owns the discard of `__init__`'s result, and the flattened // frame shape cannot reconstruct that discard from a two-frame in-callee // guard pause. - let strict_seed = !force_caller_boundary_resume - && strict_inlinable + let strict_seed = strict_inlinable && inline_depth < fbw_max_multiframe_depth() && callee_code.cellvars.is_empty() && constructor_result.is_none(); @@ -3912,7 +3913,7 @@ fn try_walker_inline_resolved_user_call_inner( if foriter_dirty_bound && !try_multiframe { return resolved_inline_decline(op.pc, line!()); } - if !strict_inlinable && !try_multiframe && !force_caller_boundary_resume { + if !strict_inlinable && !try_multiframe { // A non-self-recursive loop/branch callee that neither the strict nor // the multiframe fast path can serve declines to interpretation // (`FBW_DECLINED_KEYS`). Self-recursive calls were already routed to @@ -4897,6 +4898,8 @@ fn try_walker_inline_resolved_user_call_inner( fbw_mode: FbwWalkMode { inline_subwalk: true, inline_caller_py_pc, + instance_next_foriter_green_key, + instance_next_foriter_census_active: instance_next_seeded_route, ..ctx.fbw_mode }, session: ctx.session, @@ -5371,65 +5374,93 @@ fn try_walker_inline_resolved_user_call_inner( }; match outcome { - DispatchOutcome::SubReturn { - result: Some(value), - } => { - let concrete_for_shadow = concrete_from_recorded_opref(ctx, value); - if require_str_result - && !matches!( - concrete_for_shadow, - ConcreteValue::Ref(obj) if !obj.is_null() && unsafe { pyre_object::is_str(obj) } - ) - { - // descroperation.py checks the app-level result before - // returning from `space.str` / `space.repr`. Re-run the - // original builtin call at the caller boundary so the - // interpreter raises its faithful TypeError; the inlined - // body has no committed concrete effect at this point. - latch_abort_call_resume( - code, - op, - ctx, - call_descr, - is_top_inline, - unjournaled_before_subwalk, - executed_effects_before, - abort_flush_call_jitcode_coord, - ); - return Err(DispatchError::callee_inline_unsupported(op.pc)); - } - if require_exact_int_result - && !matches!( - concrete_for_shadow, - ConcreteValue::Ref(obj) - if walker_is_exact_machine_int_concrete(obj) - ) - { - // `descroperation.py:608-620 _index` validates the app-level - // result before its caller continues, and a long, a bool or an - // int subclass are all legal there — only the machine-int - // arithmetic downstream cannot take them. Decline instead of - // aborting: the caller rewinds the emission and falls through - // to its residual, which re-runs the whole builtin. That is - // sound because the body is admitted only when re-running it - // observes and changes nothing (`exc_override_sample_safe`), - // and it keeps a legal program from killing the enclosing - // loop's trace, which `callee_inline_unsupported` would. - return resolved_inline_decline(op.pc, line!()); - } - // `descr_call` discards `__init__`'s result after checking it is - // None and returns the instance instead (`check_init_returned_none`). - // A non-None result is a TypeError the inlined body cannot raise, so - // give the callee back to the interpreter, which re-runs the call and - // raises the faithful message. Latch the CALL boundary first, like - // the invalid-`str`/`repr`-result path above: the sub-walk already - // executed the constructor body, so a plain abort would have the - // interpreter replay it and repeat any effect it performed. - let (value, concrete_for_shadow) = match constructor_result { - Some(instance) => { - if !matches!(concrete_for_shadow, + DispatchOutcome::SubReturn { result } => match finish_inline_callee_return(ctx, result) { + Some(value) => { + let concrete_for_shadow = concrete_from_recorded_opref(ctx, value); + if require_str_result + && !matches!( + concrete_for_shadow, + ConcreteValue::Ref(obj) if !obj.is_null() && unsafe { pyre_object::is_str(obj) } + ) + { + // descroperation.py checks the app-level result before + // returning from `space.str` / `space.repr`. Re-run the + // original builtin call at the caller boundary so the + // interpreter raises its faithful TypeError; the inlined + // body has no committed concrete effect at this point. + latch_abort_call_resume( + code, + op, + ctx, + call_descr, + is_top_inline, + unjournaled_before_subwalk, + executed_effects_before, + abort_flush_call_jitcode_coord, + ); + return Err(DispatchError::callee_inline_unsupported(op.pc)); + } + if require_exact_int_result + && !matches!( + concrete_for_shadow, + ConcreteValue::Ref(obj) + if walker_is_exact_machine_int_concrete(obj) + ) + { + // `descroperation.py:608-620 _index` validates the app-level + // result before its caller continues, and a long, a bool or an + // int subclass are all legal there — only the machine-int + // arithmetic downstream cannot take them. Decline instead of + // aborting: the caller rewinds the emission and falls through + // to its residual, which re-runs the whole builtin. That is + // sound because the body is admitted only when re-running it + // observes and changes nothing (`exc_override_sample_safe`), + // and it keeps a legal program from killing the enclosing + // loop's trace, which `callee_inline_unsupported` would. + return resolved_inline_decline(op.pc, line!()); + } + // `descr_call` discards `__init__`'s result after checking it is + // None and returns the instance instead (`check_init_returned_none`). + // A non-None result is a TypeError the inlined body cannot raise, so + // give the callee back to the interpreter, which re-runs the call and + // raises the faithful message. Latch the CALL boundary first, like + // the invalid-`str`/`repr`-result path above: the sub-walk already + // executed the constructor body, so a plain abort would have the + // interpreter replay it and repeat any effect it performed. + let (value, concrete_for_shadow) = match constructor_result { + Some(instance) => { + if !matches!(concrete_for_shadow, ConcreteValue::Ref(obj) if unsafe { pyre_object::is_none(obj) }) - { + { + latch_abort_call_resume( + code, + op, + ctx, + call_descr, + is_top_inline, + unjournaled_before_subwalk, + executed_effects_before, + abort_flush_call_jitcode_coord, + ); + return Err(DispatchError::callee_inline_unsupported(op.pc)); + } + instance + } + None => (value, concrete_for_shadow), + }; + if let Some(result) = intermediate_result { + // The caller stays pinned at its own CALL boundary, so a guard + // it emits after this hand-off resumes by re-entering that CALL + // and running the callee a second time, and its rewinding + // declines cut the trace without undoing what the body already + // did. `FBW_EXECUTED_EFFECT_COUNT` is the odometer that + // answers whether that is survivable: "a nonzero count delta + // means the callee attempt cannot be discarded and re-executed + // without risking a double". Abort rather than decline — + // `latch_abort_call_resume` deliberately declines to latch the + // CALL when effects ran, so the interpreter resumes past it + // instead of re-running the effects. + if fbw_executed_effect_count() != executed_effects_before { latch_abort_call_resume( code, op, @@ -5442,53 +5473,25 @@ fn try_walker_inline_resolved_user_call_inner( ); return Err(DispatchError::callee_inline_unsupported(op.pc)); } - instance + *result = Some((value, concrete_for_shadow)); + return Ok(Some((DispatchOutcome::Continue, op.next_pc))); } - None => (value, concrete_for_shadow), - }; - if let Some(result) = intermediate_result { - // The caller stays pinned at its own CALL boundary, so a guard - // it emits after this hand-off resumes by re-entering that CALL - // and running the callee a second time, and its rewinding - // declines cut the trace without undoing what the body already - // did. `FBW_EXECUTED_EFFECT_COUNT` is the odometer that - // answers whether that is survivable: "a nonzero count delta - // means the callee attempt cannot be discarded and re-executed - // without risking a double". Abort rather than decline — - // `latch_abort_call_resume` deliberately declines to latch the - // CALL when effects ran, so the interpreter resumes past it - // instead of re-running the effects. - if fbw_executed_effect_count() != executed_effects_before { - latch_abort_call_resume( - code, - op, - ctx, - call_descr, - is_top_inline, - unjournaled_before_subwalk, - executed_effects_before, - abort_flush_call_jitcode_coord, - ); - return Err(DispatchError::callee_inline_unsupported(op.pc)); + match dst_bank { + 'r' => write_ref_reg(ctx, op.pc, dst, value, concrete_for_shadow)?, + 'i' => write_int_reg(ctx, op.pc, dst, value, concrete_for_shadow)?, + 'v' => {} + _ => return Ok(None), } - *result = Some((value, concrete_for_shadow)); - return Ok(Some((DispatchOutcome::Continue, op.next_pc))); - } - match dst_bank { - 'r' => write_ref_reg(ctx, op.pc, dst, value, concrete_for_shadow)?, - 'i' => write_int_reg(ctx, op.pc, dst, value, concrete_for_shadow)?, - 'v' => {} - _ => return Ok(None), - } - Ok(Some((DispatchOutcome::Continue, op.next_pc))) - } - DispatchOutcome::SubReturn { result: None } => { - if dst_bank == 'v' { Ok(Some((DispatchOutcome::Continue, op.next_pc))) - } else { - Err(DispatchError::UnexpectedVoidSubReturn { pc: op.pc }) } - } + None => { + if dst_bank == 'v' { + Ok(Some((DispatchOutcome::Continue, op.next_pc))) + } else { + Err(DispatchError::UnexpectedVoidSubReturn { pc: op.pc }) + } + } + }, DispatchOutcome::SubRaise { exc, exc_concrete } => { if let Some(target) = try_catch_exception_at(code, op.next_pc) { // The handler this routes to is part of the trace, so once the @@ -5497,7 +5500,7 @@ fn try_walker_inline_resolved_user_call_inner( // `handle_exception` could record a node from. Emit the node // at runtime as well as applying it for the recording pass. let emit_runtime = - !record_prepend_application_traceback(ctx, exc, exc_concrete, op.pc); + !record_prepend_application_traceback(ctx, exc, exc_concrete, op.pc)?; record_inline_exception_context(ctx.trace_ctx, exc, exc_concrete); record_inline_application_traceback( ctx, @@ -6867,6 +6870,7 @@ pub(crate) fn try_walker_inline_index( None, Some(&mut result), true, + None, )?; match (inlined, result) { (Some((DispatchOutcome::Continue, next_pc)), Some(result)) if next_pc == op.next_pc => { @@ -6988,6 +6992,182 @@ pub(crate) fn try_walker_inline_subscr_getitem( ) } +/// Inline a user instance's Python `__next__` directly under FOR_ITER. +/// +/// The instance arm of `space.next` forwards the method's exception to the +/// FOR_ITER catch arm. The callee is resumed through a seeded multi-frame +/// snapshot, and every route/callee guard is keyed so failure resumes in the +/// blackhole without compiling the unsafe mid-callee exhaustion bridge. +#[allow(clippy::too_many_arguments)] +pub(crate) fn try_walker_specialize_instance_next( + ctx: &mut WalkContext<'_, '_, Sym>, + op: &DecodedOp, + code: &[u8], + funcptr: OpRef, + r_args: &[OpRef], + call_descr: &dyn majit_ir::descr::CallDescr, + dst: usize, + dst_bank: char, +) -> Result, DispatchError> { + if !ctx.is_authoritative_executor + || dst_bank != 'r' + || ctx.fbw_mode.inline_subwalk + || r_args.len() != 1 + { + return Ok(None); + } + + let Some(foriter_green_key) = walker_foriter_green_key(ctx, op.pc) else { + return Ok(None); + }; + if ctx.trace_ctx.is_bridge_trace + && crate::trace::instance_next_foriter_bridge_demoted(foriter_green_key) + { + return Ok(None); + } + + let iter_op = r_args[0]; + let Some(iter_obj) = walker_concrete_ref_object(ctx, iter_op) else { + return Ok(None); + }; + let Some((w_type, version_tag, w_next)) = + (unsafe { pyre_interpreter::baseobjspace::next_fast_path(iter_obj) }) + else { + return Ok(None); + }; + let Some((w_code, nparams, has_closure)) = (unsafe { resolve_inlinable_callee(w_next) }) else { + return Ok(None); + }; + if nparams != 1 { + return Ok(None); + } + let Some(body_facts) = sub_jitcode_body_facts_for_code(w_code) else { + return Ok(None); + }; + if body_facts.owns_loop_header || body_facts.has_exception_table { + return Ok(None); + } + // Preserve any deferred-call denial already attached to this callee. The + // keyed route does not itself admit DeferredCall: its sub-walk leaves the + // deferred-inline guard unarmed. + if fbw_foriter_deferred_call_denied(w_code as usize) { + return Ok(None); + } + + let body_coord = fbw_foriter_body_from_op_pc(ctx, op.pc) + .unwrap_or_else(|| InflightForiterBody::Py(ctx.entry_py_pc() as usize + 1)); + fbw_foriter_inflight_mark_attempt(body_coord); + let pre_emit_pos = ctx.trace_ctx.get_trace_position(); + + let _roots = pyre_object::gc_roots::push_roots(); + let iter_root = pyre_object::gc_roots::shadow_stack_len(); + pyre_object::gc_roots::pin_root(iter_obj); + let type_root = pyre_object::gc_roots::shadow_stack_len(); + pyre_object::gc_roots::pin_root(w_type); + let next_root = pyre_object::gc_roots::shadow_stack_len(); + pyre_object::gc_roots::pin_root(w_next); + let code_root = pyre_object::gc_roots::shadow_stack_len(); + pyre_object::gc_roots::pin_root(w_code as pyre_object::PyObjectRef); + + let iter_obj = pyre_object::gc_roots::shadow_stack_get(iter_root); + let w_type = pyre_object::gc_roots::shadow_stack_get(type_root); + let w_next = pyre_object::gc_roots::shadow_stack_get(next_root); + let w_code = pyre_object::gc_roots::shadow_stack_get(code_root) as *const (); + let iter_layout = unsafe { (*iter_obj).ob_type } as i64; + if !iter_op.is_constant() && !ctx.trace_ctx.heap_cache().is_class_known(iter_op) { + let type_const = ctx.trace_ctx.const_int(iter_layout); + let descr = majit_metainterp::make_resume_guard_descr_instance_next_foriter( + Some(OpCode::GuardClass), + foriter_green_key, + ); + ctx.trace_ctx + .record_guard_with_descr(OpCode::GuardClass, &[iter_op, type_const], descr); + spec_census_record_instance_next_route_guard_keyed(); + walker_capture_snapshot_for_last_guard(ctx, op.pc)?; + } + ctx.trace_ctx + .heap_cache_mut() + .class_now_known(iter_op, iter_layout); + + let next_const = ctx.trace_ctx.const_ref(w_next as i64); + let executed_effects_before = fbw_executed_effect_count(); + let inline = try_walker_inline_resolved_user_call_inner( + ctx, + op, + code, + funcptr, + r_args, + call_descr, + dst_bank, + dst, + w_next, + next_const, + w_next, + vec![ + ConcreteValue::Ref(w_next), + ConcreteValue::Null, + ConcreteValue::Ref(iter_obj), + ], + vec![iter_op], + vec![ConcreteValue::Ref(iter_obj)], + true, + None, + w_code, + nparams, + has_closure, + Some((iter_op, iter_obj, w_type, version_tag)), + None, + true, + false, + None, + None, + false, + Some(foriter_green_key), + ); + let inline_resume_pc = match inline { + Ok(Some((DispatchOutcome::Continue, next))) => next, + outcome => { + // Rewinding the emission and falling through to the caller's + // residual re-runs the whole `__next__`. The sibling decline that + // does the same states its precondition outright — "sound because + // the body is admitted only when re-running it observes and changes + // nothing" — and the legacy route can state it because + // `fbw_callee_body_replay_safety` proved the body `Clean` first. + // This keyed route deliberately bypasses that classification, so + // read the odometer instead: once the sub-walk has executed a + // concrete effect, `cut_trace_with_snapshots` cannot undo it (it + // truncates recorded ops and snapshots, nothing else) and the + // residual `next` compiles a loop that advances the iterator twice + // per iteration. Surface the decline as an abort there, the same + // disposition every other caller of the inliner already takes. + if fbw_executed_effect_count() != executed_effects_before { + return Err(match outcome { + Err(e) => e, + _ => DispatchError::callee_inline_unsupported(op.pc), + }); + } + ctx.trace_ctx.cut_trace_with_snapshots(pre_emit_pos); + ctx.trace_ctx.heap_cache_mut().reset(); + return Ok(None); + } + }; + // `Continue` represents either a value return at `op.next_pc`, with `dst` + // holding the item, or a caught `SubRaise` routed to the caller's handler + // target, where `dst` was not written (`pyjitpl.py:2530-2558`). Only the + // value-return shape can update the FOR_ITER item bookkeeping below. + if inline_resume_pc != op.next_pc { + return Ok(Some((DispatchOutcome::Continue, inline_resume_pc))); + } + + let item_op = ctx.registers_r[dst]; + let Some(concrete_item) = walker_concrete_ref_object(ctx, item_op) else { + return Err(DispatchError::callee_inline_unsupported(op.pc)); + }; + fbw_foriter_inflight_capture(concrete_item, body_coord); + ctx.vstack_last_ref = item_op; + Ok(Some((DispatchOutcome::Continue, op.next_pc))) +} + /// Forward dunder selected by `try_dispatch_binary_special` for a non-inplace /// BINARY_OP. In-place operators have a distinct `__i*__` then binary fallback /// protocol and therefore stay on the generic path until that protocol is @@ -7399,6 +7579,21 @@ pub(crate) fn allocate_callee_register_banks( (regs_r, regs_i, regs_f, concrete_r, concrete_i) } +/// Apply the non-exceptional frame-exit state transition and return the +/// callee's value for caller-side shape handling. +/// +/// `pyjitpl.py:2503-2506 finishframe` clears `last_exc_value` before +/// `popframe()`. Because the walker stores that state per frame, the caller's +/// slot is the one that must observe the null after the callee returns. +pub(crate) fn finish_inline_callee_return( + ctx: &mut WalkContext<'_, '_, Sym>, + result: Option, +) -> Option { + ctx.last_exc_value = None; + ctx.last_exc_value_concrete = ConcreteValue::Null; + result +} + /// Seed a callee jitcode's register banks with positional args and walk /// its body, returning the callee's terminal [`DispatchOutcome`] /// (`SubReturn` / `SubRaise` / `Terminate` / `SwitchToBlackhole`). @@ -7645,50 +7840,50 @@ pub(crate) fn dispatch_inline_call_dr_kind( let callee_outcome = callee_result?; match callee_outcome { - DispatchOutcome::SubReturn { - result: Some(value), - } => { - if dst_bank == 'v' { - // `inline_call_r_v/dR` - // (`bhimpl_inline_call_r_v` `blackhole.py`) - // expects a void-return callee. A `Some` return here is - // a codewriter shape mismatch. - return Err(DispatchError::UnexpectedNonVoidSubReturn { pc: op.pc }); - } - let dst = code[op.pc + 1 + 2 + arg_width] as usize; - // inline_call_* dst writeback — `value` is the callee's - // SubReturn OpRef. The callee's matching concrete shadow - // was dropped at sub-walk exit; `concrete_of_opref` still - // sees through to `constants.get_value` for callees that - // return a constant (e.g. `LoadConst` tail), so route via - // the unified shadow channel. Non-constant returns surface - // as the sentinel `GcRef(usize::MAX)` → Null fallback. - let concrete_for_shadow = concrete_from_recorded_opref(ctx, value); - match dst_bank { - 'r' => { - write_ref_reg(ctx, op.pc, dst, value, concrete_for_shadow)?; + DispatchOutcome::SubReturn { result } => match finish_inline_callee_return(ctx, result) { + Some(value) => { + if dst_bank == 'v' { + // `inline_call_r_v/dR` + // (`bhimpl_inline_call_r_v` `blackhole.py`) + // expects a void-return callee. A `Some` return here is + // a codewriter shape mismatch. + return Err(DispatchError::UnexpectedNonVoidSubReturn { pc: op.pc }); } - 'i' => { - write_int_reg(ctx, op.pc, dst, value, concrete_for_shadow)?; - } - _ => unreachable!( - "dispatch_inline_call_dr_kind dst_bank must be 'r', 'i' or 'v' (\ + let dst = code[op.pc + 1 + 2 + arg_width] as usize; + // inline_call_* dst writeback — `value` is the callee's + // SubReturn OpRef. The callee's matching concrete shadow + // was dropped at sub-walk exit; `concrete_of_opref` still + // sees through to `constants.get_value` for callees that + // return a constant (e.g. `LoadConst` tail), so route via + // the unified shadow channel. Non-constant returns surface + // as the sentinel `GcRef(usize::MAX)` → Null fallback. + let concrete_for_shadow = concrete_from_recorded_opref(ctx, value); + match dst_bank { + 'r' => { + write_ref_reg(ctx, op.pc, dst, value, concrete_for_shadow)?; + } + 'i' => { + write_int_reg(ctx, op.pc, dst, value, concrete_for_shadow)?; + } + _ => unreachable!( + "dispatch_inline_call_dr_kind dst_bank must be 'r', 'i' or 'v' (\ codewriter does not emit dR>f shape today)" - ), + ), + } + Ok((DispatchOutcome::Continue, op.next_pc)) } - Ok((DispatchOutcome::Continue, op.next_pc)) - } - DispatchOutcome::SubReturn { result: None } => { - if dst_bank == 'v' { - // `inline_call_r_v/dR` expects exactly this — callee - // exits via `void_return/`, no SubReturn writeback. - return Ok((DispatchOutcome::Continue, op.next_pc)); + None => { + if dst_bank == 'v' { + // `inline_call_r_v/dR` expects exactly this — callee + // exits via `void_return/`, no SubReturn writeback. + return Ok((DispatchOutcome::Continue, op.next_pc)); + } + // Same shape contract as `_r_r`: a `_r_` variant promises + // a non-void result for the dst's `>X` slot. A void return + // reaching here is a codewriter shape mismatch. + Err(DispatchError::UnexpectedVoidSubReturn { pc: op.pc }) } - // Same shape contract as `_r_r`: a `_r_` variant promises - // a non-void result for the dst's `>X` slot. A void return - // reaching here is a codewriter shape mismatch. - Err(DispatchError::UnexpectedVoidSubReturn { pc: op.pc }) - } + }, DispatchOutcome::SubRaise { exc, exc_concrete } => { if let Some(target) = try_catch_exception_at(code, op.next_pc) { // The handler this routes to is part of the trace, so once the @@ -7697,7 +7892,7 @@ pub(crate) fn dispatch_inline_call_dr_kind( // `handle_exception` could record a node from. Emit the node // at runtime as well as applying it for the recording pass. let emit_runtime = - !record_prepend_application_traceback(ctx, exc, exc_concrete, op.pc); + !record_prepend_application_traceback(ctx, exc, exc_concrete, op.pc)?; record_inline_exception_context(ctx.trace_ctx, exc, exc_concrete); record_inline_application_traceback( ctx, @@ -7854,36 +8049,38 @@ pub(crate) fn dispatch_inline_call_dir_kind( )?; match callee_outcome { - DispatchOutcome::SubReturn { - result: Some(value), - } => { - if dst_bank == 'v' { - return Err(DispatchError::UnexpectedNonVoidSubReturn { pc: op.pc }); - } - // dst register byte sits after descr (2B) + I-list (int_width) - // + R-list (ref_width) bytes. - let dst = code[op.pc + 1 + 2 + int_width + ref_width] as usize; - // See dispatch_inline_call_dr_kind: route the SubReturn - // OpRef through the unified shadow channel so constant - // return values propagate. - let concrete_for_shadow = concrete_from_recorded_opref(ctx, value); - match dst_bank { - 'r' => { - write_ref_reg(ctx, op.pc, dst, value, concrete_for_shadow)?; + DispatchOutcome::SubReturn { result } => match finish_inline_callee_return(ctx, result) { + Some(value) => { + if dst_bank == 'v' { + return Err(DispatchError::UnexpectedNonVoidSubReturn { pc: op.pc }); } - 'i' => { - write_int_reg(ctx, op.pc, dst, value, concrete_for_shadow)?; + // dst register byte sits after descr (2B) + I-list (int_width) + // + R-list (ref_width) bytes. + let dst = code[op.pc + 1 + 2 + int_width + ref_width] as usize; + // See dispatch_inline_call_dr_kind: route the SubReturn + // OpRef through the unified shadow channel so constant + // return values propagate. + let concrete_for_shadow = concrete_from_recorded_opref(ctx, value); + match dst_bank { + 'r' => { + write_ref_reg(ctx, op.pc, dst, value, concrete_for_shadow)?; + } + 'i' => { + write_int_reg(ctx, op.pc, dst, value, concrete_for_shadow)?; + } + _ => unreachable!( + "dispatch_inline_call_dir_kind dst_bank must be 'r', 'i' or 'v'" + ), } - _ => unreachable!("dispatch_inline_call_dir_kind dst_bank must be 'r', 'i' or 'v'"), + Ok((DispatchOutcome::Continue, op.next_pc)) } - Ok((DispatchOutcome::Continue, op.next_pc)) - } - DispatchOutcome::SubReturn { result: None } => { - if dst_bank == 'v' { - return Ok((DispatchOutcome::Continue, op.next_pc)); + None => { + if dst_bank == 'v' { + return Ok((DispatchOutcome::Continue, op.next_pc)); + } + Err(DispatchError::UnexpectedVoidSubReturn { pc: op.pc }) } - Err(DispatchError::UnexpectedVoidSubReturn { pc: op.pc }) - } + }, DispatchOutcome::SubRaise { exc, exc_concrete } => { if let Some(target) = try_catch_exception_at(code, op.next_pc) { // The handler this routes to is part of the trace, so once the @@ -7892,7 +8089,7 @@ pub(crate) fn dispatch_inline_call_dir_kind( // `handle_exception` could record a node from. Emit the node // at runtime as well as applying it for the recording pass. let emit_runtime = - !record_prepend_application_traceback(ctx, exc, exc_concrete, op.pc); + !record_prepend_application_traceback(ctx, exc, exc_concrete, op.pc)?; record_inline_exception_context(ctx.trace_ctx, exc, exc_concrete); record_inline_application_traceback( ctx, @@ -8020,49 +8217,48 @@ pub(crate) fn dispatch_inline_call_dirf_kind( let callee_outcome = callee_result?; match callee_outcome { - DispatchOutcome::SubReturn { - result: Some(value), - } => { - if dst_bank == 'v' { - return Err(DispatchError::UnexpectedNonVoidSubReturn { pc: op.pc }); - } - let dst = code[op.pc + 1 + 2 + int_width + ref_width + float_width] as usize; - // See dispatch_inline_call_dr_kind: route the SubReturn - // OpRef through the unified shadow channel so constant - // return values propagate. - let concrete_for_shadow = concrete_from_recorded_opref(ctx, value); - match dst_bank { - 'i' => { - write_int_reg(ctx, op.pc, dst, value, concrete_for_shadow)?; + DispatchOutcome::SubReturn { result } => match finish_inline_callee_return(ctx, result) { + Some(value) => { + if dst_bank == 'v' { + return Err(DispatchError::UnexpectedNonVoidSubReturn { pc: op.pc }); } - 'r' => { - write_ref_reg(ctx, op.pc, dst, value, concrete_for_shadow)?; - } - 'f' => { - let len = ctx.registers_f.len(); - let slot = - ctx.registers_f - .get_mut(dst) - .ok_or(DispatchError::RegisterOutOfRange { + let dst = code[op.pc + 1 + 2 + int_width + ref_width + float_width] as usize; + // See dispatch_inline_call_dr_kind: route the SubReturn + // OpRef through the unified shadow channel so constant + // return values propagate. + let concrete_for_shadow = concrete_from_recorded_opref(ctx, value); + match dst_bank { + 'i' => { + write_int_reg(ctx, op.pc, dst, value, concrete_for_shadow)?; + } + 'r' => { + write_ref_reg(ctx, op.pc, dst, value, concrete_for_shadow)?; + } + 'f' => { + let len = ctx.registers_f.len(); + let slot = ctx.registers_f.get_mut(dst).ok_or( + DispatchError::RegisterOutOfRange { pc: op.pc, reg: dst, len, bank: "f", - })?; - *slot = value; + }, + )?; + *slot = value; + } + _ => unreachable!( + "dispatch_inline_call_dirf_kind dst_bank must be 'i', 'r', 'f' or 'v'" + ), } - _ => unreachable!( - "dispatch_inline_call_dirf_kind dst_bank must be 'i', 'r', 'f' or 'v'" - ), + Ok((DispatchOutcome::Continue, op.next_pc)) } - Ok((DispatchOutcome::Continue, op.next_pc)) - } - DispatchOutcome::SubReturn { result: None } => { - if dst_bank == 'v' { - return Ok((DispatchOutcome::Continue, op.next_pc)); + None => { + if dst_bank == 'v' { + return Ok((DispatchOutcome::Continue, op.next_pc)); + } + Err(DispatchError::UnexpectedVoidSubReturn { pc: op.pc }) } - Err(DispatchError::UnexpectedVoidSubReturn { pc: op.pc }) - } + }, DispatchOutcome::SubRaise { exc, exc_concrete } => { if let Some(target) = try_catch_exception_at(code, op.next_pc) { // The handler this routes to is part of the trace, so once the @@ -8071,7 +8267,7 @@ pub(crate) fn dispatch_inline_call_dirf_kind( // `handle_exception` could record a node from. Emit the node // at runtime as well as applying it for the recording pass. let emit_runtime = - !record_prepend_application_traceback(ctx, exc, exc_concrete, op.pc); + !record_prepend_application_traceback(ctx, exc, exc_concrete, op.pc)?; record_inline_exception_context(ctx.trace_ctx, exc, exc_concrete); record_inline_application_traceback( ctx, diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs index a5d13978203..e7b40c9401d 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/mod.rs @@ -31,10 +31,10 @@ //! | `int_return/i` | PARITY | int-bank counterpart of `ref_return/r` — top-level records `Finish(reg) descr=done_with_this_frame_descr_int` (`pyjitpl.py:3206-3208`), sub-walk surfaces `SubReturn{Some(value)}`. RPython `pyjitpl.py:463 opimpl_int_return = _opimpl_any_return`. | //! | `float_return/f` | PARITY | float-bank counterpart — top-level records `Finish(reg) descr=done_with_this_frame_descr_float` (`pyjitpl.py:3212-3214`), sub-walk surfaces `SubReturn{Some(value)}`. RPython `pyjitpl.py:465 opimpl_float_return = _opimpl_any_return`. | //! | `void_return/` | PARITY | void return — top-level records `Finish([]) descr=done_with_this_frame_descr_void` (`pyjitpl.py:3202-3205`, `exits = []` branch), sub-walk surfaces `SubReturn{None}`. RPython `pyjitpl.py:467-469 opimpl_void_return → finishframe(None)`. | -//! | `inline_call_r_r/dR>r` | PARITY (per-frame catch) | recurses into sub-jitcode via `JitCodeDescr::jitcode_index()`, populates callee `registers_r` (`setup_call_r`, OOR surfaces `InlineCallArityMismatch`), writes `SubReturn{value}` into caller dst (Ref bank), scans caller's `op.next_pc` for `live/` + `catch_exception/L` on `SubRaise` (`pyjitpl.py:2506-2522 finishframe_exception`). Sub-walk reaching `Terminate` is unexpected (top-level should never fire from a sub-walk); `SubReturn{None}` into a `_r_*` slot surfaces `UnexpectedVoidSubReturn`. | -//! | `inline_call_r_i/dR>i` | PARITY | int-result sibling of `inline_call_r_r/dR>r`. Same recursion + arglist + raise routing; only the dst bank changes (`registers_i[dst] = subreturn_value`). RPython `pyjitpl.py:1266-1324 _opimpl_inline_call*` is generated through `_opimpl_any_inline_call` decorator that varies on the result type — pyre's walker shares the body via `dispatch_inline_call_dr_kind(dst_bank)`. | -//! | `inline_call_ir_r/dIR>r`, `inline_call_ir_i/dIR>i` | PARITY | extended-arglist siblings — descr + I-list + R-list + dst. RPython `setup_call(argboxes_i, argboxes_r, argboxes_f)` (pyjitpl.py:230-260) populates the callee's int + ref banks from the two lists. Walker uses `dispatch_inline_call_dir_kind(dst_bank)` which reads `read_int_var_list` then `read_ref_var_list` and surfaces per-bank arity overflow as `InlineCallIntArityMismatch` / `InlineCallArityMismatch`. | -//! | `inline_call_irf_r/dIRF>r`, `inline_call_irf_f/dIRF>f` | PARITY | full-arglist variants — descr + I-list + R-list + F-list + dst. RPython same `setup_call` distribution; walker uses `dispatch_inline_call_dirf_kind(dst_bank)` extending the dIR helper with `read_float_var_list` + float-bank arg setup. Float arity overflow surfaces `InlineCallFloatArityMismatch`. | +//! | `inline_call_r_r/dR>r` | PARITY (per-frame catch) | recurses into sub-jitcode via `JitCodeDescr::jitcode_index()`, populates callee `registers_r` (`setup_call_r`, OOR surfaces `InlineCallArityMismatch`), clears the caller's `last_exc_value` before writing `SubReturn{value}` into its Ref dst (`pyjitpl.py:2503-2510 finishframe`), and scans caller's `op.next_pc` for `live/` + `catch_exception/L` on `SubRaise` (`pyjitpl.py:2530-2558 finishframe_exception`). Sub-walk reaching `Terminate` is unexpected (top-level should never fire from a sub-walk); `SubReturn{None}` clears before surfacing `UnexpectedVoidSubReturn`. | +//! | `inline_call_r_i/dR>i` | PARITY | int-result sibling of `inline_call_r_r/dR>r`. Same recursion, arglist, normal-return clear, and raise routing; only the dst bank changes (`registers_i[dst] = subreturn_value`). RPython `pyjitpl.py:1266-1324 _opimpl_inline_call*` is generated through `_opimpl_any_inline_call` decorator that varies on the result type — pyre's walker shares the body via `dispatch_inline_call_dr_kind(dst_bank)`. | +//! | `inline_call_ir_r/dIR>r`, `inline_call_ir_i/dIR>i` | PARITY | extended-arglist siblings — descr + I-list + R-list + dst. RPython `setup_call(argboxes_i, argboxes_r, argboxes_f)` (pyjitpl.py:230-260) populates the callee's int + ref banks from the two lists. Walker uses `dispatch_inline_call_dir_kind(dst_bank)` which reads `read_int_var_list` then `read_ref_var_list`, clears the caller exception slot on normal return, and surfaces per-bank arity overflow as `InlineCallIntArityMismatch` / `InlineCallArityMismatch`. | +//! | `inline_call_irf_r/dIRF>r`, `inline_call_irf_f/dIRF>f` | PARITY | full-arglist variants — descr + I-list + R-list + F-list + dst. RPython same `setup_call` distribution; walker uses `dispatch_inline_call_dirf_kind(dst_bank)` extending the dIR helper with `read_float_var_list` + float-bank arg setup and the same normal-return clear. Float arity overflow surfaces `InlineCallFloatArityMismatch`. | //! | `int_copy/i>i` | PARITY | `registers_i[dst] = registers_i[src]` SSA rename, no IR op emitted (`pyjitpl.py:471-477 _opimpl_any_copy + >i` decorator) | //! | `ref_copy/r>r` | PARITY | Ref-bank sibling — `registers_r[dst] = registers_r[src]` SSA rename, no IR op. Const-source variants (codewriter `emit_ref_copy!` with `ConstRef`) resolve through the constants window of `registers_r` (pre-populated by `setposition` in [`num_regs_r, num_regs_and_consts_r)`). | //! | `int_/ii>i` | PARITY | int_add/int_sub/int_mul/int_and/int_or/int_xor/int_lshift/int_rshift + comparisons int_eq/int_ne/int_lt/int_le/int_gt/int_ge (14 ops). Reads two `i`-coded regs, records `OpCode::Int` with `[a, b]`, writes recorder result into dst (`pyjitpl.py:279-336`). Mixed shapes such as `int_lshift/ri>i` stay unwired: those are kind-flow kind-flow bugs and must stay unsupported. | @@ -1070,7 +1070,7 @@ fn finish_current_frame_execution(ctx: &mut WalkContext<'_, '_, Sy } } -fn recording_instruction_is_bare_reraise( +fn recording_raise_keeps_existing_traceback( ctx: &WalkContext<'_, '_, Sym>, opcode_position: usize, ) -> bool { @@ -1080,7 +1080,7 @@ fn recording_instruction_is_bare_reraise( ctx.inline_callee_consts .map_or(-1, |consts| consts.jitcode_index) }; - crate::state::jitcode_pc_is_bare_reraise(jitcode_index, opcode_position as i32) + crate::state::jitcode_pc_raise_keeps_existing_traceback(jitcode_index, opcode_position as i32) } /// The frame identity, code object and instruction coordinate one @@ -1180,7 +1180,8 @@ fn emit_traceback_node( kind: pyre_object::interp_exceptions::ExcKind, site: &TracebackNodeSite, w_next: OpRef, -) { + opcode_position: usize, +) -> Result<(), DispatchError> { let traceback = ctx.trace_ctx.record_op_with_descr( OpCode::NewWithVtable, &[], @@ -1211,22 +1212,40 @@ fn emit_traceback_node( // `f_lineno` resolves through `offset2lineno(pycode, last_instr)` on every // read, so the frame itself has to carry the coordinate — the node's own - // `tb_lasti` answers a different question and is frozen. The interpreter - // gets this for free from `pyopcode.py`'s per-opcode `last_instr` store; - // compiled code does not run it, and `fbw_publish_exit_last_instr` only - // reaches the virtualizable, so an inlined callee frame would keep the `-1` - // initialization sentinel and report its `def` line. A frame that goes on - // running has this overwritten by its own later publish, exactly as the - // per-opcode store would. + // `tb_lasti` answers a different question and is frozen. Preserve the + // exact operation PyPy traces from `pyopcode.py`'s per-opcode + // `self.last_instr = ...`: `pyjitpl.py:1188-1199 + // _opimpl_setfield_vable` updates the shadow for the standard frame and + // falls through to SETFIELD_GC only for a nonstandard/inlined frame. A raw + // SETFIELD_GC here wrote the root frame on every caught-exception bridge, + // even though `last_instr` is declared virtualizable by + // `interp_jit.py:25-30`. let last_instr_value = ctx.trace_ctx.const_int(i64::from(site.last_instruction)); - let last_instr_descr = crate::descr::pyframe_next_instr_descr(); - ctx.trace_ctx.record_op_with_descr( - OpCode::SetfieldGc, - &[site.frame, last_instr_value], - last_instr_descr.clone(), + let unavailable = || DispatchError::TracebackNodeVableFieldUnavailable { + pc: opcode_position, + field: "last_instr", + }; + let vinfo = ctx.trace_ctx.virtualizable_info().ok_or_else(unavailable)?; + let last_instr_index = vinfo + .static_field_index_by_name("last_instr") + .ok_or_else(unavailable)?; + let last_instr_descr = vinfo.static_field_descr(last_instr_index); + // A traceback node names an inlined callee's frame as often as the walk's + // own, and a frame that is not `virtualizable_boxes[-1]` sends + // `vable_setfield` down the `_nonstandard_virtualizable` path, which mints + // a PTR_EQ promote `GuardValue` internally with no resume snapshot. Every + // other vable emit site pairs the call with this capture for that reason; + // without it the promote reaches the decoder holding + // `UNSTAMPED_JITCODE_INDEX` and `frame_value_count_at` fails loud. + let guards_before = ctx.trace_ctx.num_guards(); + let write = ctx.trace_ctx.vable_setfield( + opcode_position, + site.frame, + last_instr_descr, + last_instr_value, + Some(Value::Int(i64::from(site.last_instruction))), ); - ctx.trace_ctx - .heapcache_setfield_cached(site.frame, last_instr_descr.index(), last_instr_value); + walker_capture_inline_nonstandard_vable_guard(ctx, opcode_position, guards_before, write)?; let traceback_descr = crate::descr::w_exception_traceback_descr(kind); ctx.trace_ctx.record_op_with_descr( @@ -1236,6 +1255,7 @@ fn emit_traceback_node( ); ctx.trace_ctx .heapcache_setfield_cached(exc, traceback_descr.index(), traceback); + Ok(()) } /// IR-virtual PREPEND of one `PyTraceback` node — the general-case sibling @@ -1270,23 +1290,23 @@ fn record_prepend_application_traceback( exc: OpRef, exc_concrete: ConcreteValue, opcode_position: usize, -) -> bool { +) -> Result { if exc.is_none() || exc.is_constant() { // A `Const` exception box freezes the RECORDING iteration's address // (`walker_record_guard_exception` pins every raise after the first // one in a walk), so reading and writing `w_traceback` through it // would chain onto a stale object on every later iteration. The // opaque hook takes the live exception as an argument instead. - return false; + return Ok(false); } let ConcreteValue::Ref(exc_ptr) = exc_concrete else { - return false; + return Ok(false); }; if exc_ptr.is_null() || unsafe { !pyre_object::is_exception(exc_ptr) } { - return false; + return Ok(false); } let Some(site) = traceback_node_site(ctx, opcode_position) else { - return false; + return Ok(false); }; if site.frame.is_none() { // No materialized frame for this level. The opaque inline hook @@ -1302,15 +1322,15 @@ fn record_prepend_application_traceback( // this walk lacks, and the vable box is not a substitute (a traceback // outlives the frame, so storing it demands the escape marking this // path does not perform). - return false; + return Ok(false); } let kind = unsafe { pyre_object::interp_exceptions::w_exception_get_kind(exc_ptr) }; // `tb = operror.get_traceback()`. MUST precede the node's own store, or // the heapcache answers with the node being built and `w_next` self-links. let traceback_descr = crate::descr::w_exception_traceback_descr(kind); let w_next = crate::state::opimpl_getfield_gc_r(ctx.trace_ctx, exc, traceback_descr); - emit_traceback_node(ctx, exc, kind, &site, w_next); - true + emit_traceback_node(ctx, exc, kind, &site, w_next, opcode_position)?; + Ok(true) } /// IR-virtual traceback record for an exception the walk itself built: the @@ -1326,27 +1346,27 @@ fn record_fresh_application_traceback( exc: OpRef, exc_concrete: ConcreteValue, opcode_position: usize, -) -> bool { +) -> Result { let ConcreteValue::Ref(exc_ptr) = exc_concrete else { - return false; + return Ok(false); }; if exc_ptr.is_null() || unsafe { !pyre_object::is_exception(exc_ptr) } { - return false; + return Ok(false); } let Some(site) = traceback_node_site(ctx, opcode_position) else { - return false; + return Ok(false); }; if site.frame.is_none() { // Same disposition as the prepend sibling, for the same reason: the // opaque inline hook fabricates a frame from the promoted callee // metadata, while a null one answers None and breaks every consumer // that follows `tb_frame.f_code` — `traceback.print_exc` among them. - return false; + return Ok(false); } let kind = unsafe { pyre_object::interp_exceptions::w_exception_get_kind(exc_ptr) }; let w_next = ctx.trace_ctx.const_ref(0); - emit_traceback_node(ctx, exc, kind, &site, w_next); - true + emit_traceback_node(ctx, exc, kind, &site, w_next, opcode_position)?; + Ok(true) } /// Compile-time-constant frame fields of an inlined callee. @@ -1426,6 +1446,15 @@ pub struct FbwWalkMode { /// bridge's own resume coordinate, restoring the RPython positional /// semantics. pub bridge_entry_merge_pc: Option, + /// FOR_ITER key inherited only by the user-instance `__next__` sub-walk. + /// Every guard it emits is tagged so failure resumes in the blackhole + /// instead of compiling a bridge from the middle of the callee. + pub instance_next_foriter_green_key: Option, + /// Census marker paired with `instance_next_foriter_green_key`. Keeping + /// the route identity separate from the optional descr key lets the + /// specialization census prove that every captured callee guard was + /// actually stamped. + pub instance_next_foriter_census_active: bool, } impl Clone for FbwWalkMode { @@ -1470,6 +1499,8 @@ impl Default for FbwWalkMode { current_exception_seed_from_walk_store: false, class_of_last_exc_is_const: false, bridge_entry_merge_pc: None, + instance_next_foriter_green_key: None, + instance_next_foriter_census_active: false, } } } @@ -1637,7 +1668,9 @@ pub struct WalkContext<'frame, 'static_a: 'frame, Sym: WalkSym> { /// because each recursive frame has its own context. The flow /// (callee raise → caller catch → caller handler reads) only /// touches the caller's slot, so per-frame storage is equivalent - /// to RPython's metainterp-level slot for the catch path. + /// to RPython's metainterp-level slot for the catch path. The other + /// half is normal return: `finishframe` clears before `popframe()`, so + /// each caller-side `SubReturn` handler clears this caller slot. pub last_exc_value: Option, /// Concrete shadow companion to [`last_exc_value`]. /// Holds the live `PyObjectRef` of the standing @@ -2498,6 +2531,16 @@ pub enum DispatchError { /// `pyjitpl.py:2865 _interpret` calls `blackhole_if_trace_too_long()` right /// after `run_one_step()` — and raises `SwitchToBlackhole(ABORT_TOO_LONG)`. TraceTooLong { pc: usize, ops: usize }, + /// The traceback node's `last_instr` store could not resolve the + /// virtualizable static field named `field`: either the trace carries no + /// `VirtualizableInfo` at all, or the registered one declares no such + /// field. `interp_jit.py:25-30` declares `last_instr` virtualizable and + /// `pyjitpl.py:1188-1199 _opimpl_setfield_vable` routes the write through + /// it, so an unresolvable descr means the walk is recording against a + /// frame layout the jitdriver never registered. Abort rather than fall + /// back to a raw SETFIELD_GC, which writes the root frame's shadow on + /// every caught-exception bridge. + TracebackNodeVableFieldUnavailable { pc: usize, field: &'static str }, } impl DispatchError { @@ -2569,6 +2612,7 @@ impl DispatchError { } Self::ExcEdgeNoInFrameCatch { .. } => "ExcEdgeNoInFrameCatch", Self::TraceTooLong { .. } => "TraceTooLong", + Self::TracebackNodeVableFieldUnavailable { .. } => "TracebackNodeVableFieldUnavailable", } } @@ -2631,7 +2675,8 @@ impl DispatchError { | Self::BranchGuardUnrestorableKeptStackPermanent { pc, .. } | Self::InplaceContainerMutationUnsupported { pc, .. } | Self::ExcEdgeNoInFrameCatch { pc, .. } - | Self::TraceTooLong { pc, .. } => *pc, + | Self::TraceTooLong { pc, .. } + | Self::TracebackNodeVableFieldUnavailable { pc, .. } => *pc, } } @@ -3281,7 +3326,7 @@ pub fn walk( exc, exc_concrete, node_position, - ); + )?; record_inline_exception_context(ctx.trace_ctx, exc, exc_concrete); record_inline_application_traceback( ctx, @@ -3331,7 +3376,7 @@ pub fn walk( if ctx.is_top_level { let recording_opcode_position = ctx.session.borrow().recording_opcode_position; - if !recording_instruction_is_bare_reraise(ctx, opcode_position) { + if !recording_raise_keeps_existing_traceback(ctx, opcode_position) { // Emit at runtime too, not only for the recording pass. // Leaving the node to the interpreter holds only for a // trace the interpreter entered: `CALL_ASSEMBLER` enters @@ -3390,7 +3435,7 @@ pub fn walk( // the trace bridges and guard failures for no gain. The // publish above has already settled `last_instr`, which the // recorder falls back to. - if !recording_instruction_is_bare_reraise(ctx, opcode_position) { + if !recording_raise_keeps_existing_traceback(ctx, opcode_position) { record_top_level_application_traceback( ctx, exc, @@ -3412,7 +3457,7 @@ pub fn walk( fbw_terminate_with_raise(exc, exc_concrete); return Ok((DispatchOutcome::Terminate, pc)); } else { - if !recording_instruction_is_bare_reraise(ctx, opcode_position) { + if !recording_raise_keeps_existing_traceback(ctx, opcode_position) { // Emit the node at runtime as well as applying it for // the recording pass, for the same reason the top-level // arm above does — here because an inlined callee has @@ -3426,7 +3471,7 @@ pub fn walk( exc, exc_concrete, opcode_position, - ); + )?; record_inline_application_traceback( ctx, exc, @@ -4949,13 +4994,31 @@ fn funcptr_concrete_int( } } -/// Returns `true` when the jitcode body contains any `catch_exception/L` -/// op — i.e. the source function has a `try`/`except` handler. Used by -/// the residual-call fast paths that conservatively decline a handler- -/// bearing body to the generic walk (which resumes a `GUARD_NO_EXCEPTION` -/// deopt into the handler correctly) rather than to their concrete fold. -fn jitcode_has_exception_handler(code: &[u8]) -> bool { - crate::jitcode_runtime::decoded_ops(code).any(|op| op.opname == "catch_exception") +/// Returns `true` when the body being walked has a Python `try`/`except` +/// handler. Used by the residual-call fast paths that conservatively decline +/// a handler-bearing body to the generic walk (which resumes a +/// `GUARD_NO_EXCEPTION` deopt into the handler correctly) rather than to their +/// concrete fold. +/// +/// The question is about the source function, so it reads `co_exceptiontable`. +/// Scanning the jitcode for `catch_exception` ops answers a different one: the +/// codewriter also emits that op for can-raise sites it routes itself — the +/// FOR_ITER exception-match arm emits one at every `for` loop — and those have +/// no Python handler for a deopt to resume into. Falls back to the scan when +/// the walk's jitcode index resolves to no `CodeObject`. +fn walk_body_has_exception_handler( + ctx: &WalkContext<'_, '_, Sym>, + code: &[u8], +) -> bool { + let jitcode_index = if ctx.is_top_level { + ctx.session.borrow().recording_jitcode_index + } else { + ctx.inline_callee_consts + .map_or(-1, |consts| consts.jitcode_index) + }; + crate::state::jitcode_source_has_exception_handler(jitcode_index).unwrap_or_else(|| { + crate::jitcode_runtime::decoded_ops(code).any(|op| op.opname == "catch_exception") + }) } /// Maps a freshly-boxed `W_Bool` opref (the `jit_bool_value_from_truth(t)` @@ -11489,7 +11552,7 @@ fn handle( ctx.last_exc_value_concrete = concrete_exc; ctx.fbw_mode.class_of_last_exc_is_const = true; let freshly_normalized = fbw_built_exc_take(exc); - if !recording_instruction_is_bare_reraise(ctx, op.pc) { + if !recording_raise_keeps_existing_traceback(ctx, op.pc) { let caught_in_frame = try_catch_exception_at(code, op.next_pc).is_some(); if caught_in_frame { // Unless this walk built the exception it may already @@ -11502,9 +11565,9 @@ fn handle( // weaker node than the live one — still a node whose // `tb_frame.f_code` answers the right code object. let emit_runtime = if freshly_normalized { - !record_fresh_application_traceback(ctx, exc, concrete_exc, op.pc) + !record_fresh_application_traceback(ctx, exc, concrete_exc, op.pc)? } else { - !record_prepend_application_traceback(ctx, exc, concrete_exc, op.pc) + !record_prepend_application_traceback(ctx, exc, concrete_exc, op.pc)? }; record_inline_application_traceback( ctx, diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs index 5d5f64effff..6315f463833 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs @@ -5281,7 +5281,7 @@ pub(crate) fn dispatch_residual_call_iRd_kind( ei.pyre_helper, majit_ir::PyreHelperKind::StoreName | majit_ir::PyreHelperKind::StoreGlobal ) - && !jitcode_has_exception_handler(code) + && !walk_body_has_exception_handler(ctx, code) { if let (Some(&frame_opref), Some(&name_opref), Some(&value_opref)) = (r_args.first(), r_args.get(1), r_args.get(2)) @@ -5496,7 +5496,8 @@ pub(crate) fn dispatch_residual_call_iRd_kind( // Range FOR_ITER is a C-level iterator advance. Re-emit its field // updates so the opaque ForIterNext residual cannot invalidate optheap; - // other iterator families retain the residual and its Python semantics. + // The user-instance route below separately enters a Python `__next__`; + // remaining iterator families retain the residual and its Python semantics. // In particular, a generic W_SeqIterObject must stay residual: its PyPy // `W_SeqIterObject.descr_next` frame catches IndexError from `__getitem__` // and turns it into iterator exhaustion. Inlining only `__getitem__` @@ -5511,6 +5512,13 @@ pub(crate) fn dispatch_residual_call_iRd_kind( write_residual_call_result_to_dst(ctx, op.pc, dst, dst_bank, item_op)?; return Ok((DispatchOutcome::Continue, op.next_pc)); } + if let Some(inlined) = spec_gate("instance_next", || { + try_walker_specialize_instance_next( + ctx, op, code, funcptr, &r_args, call_descr, dst, dst_bank, + ) + })? { + return Ok(inlined); + } } // Emit MAKE_FUNCTION's `Function.__init__` as New + SetField so a `def` in a @@ -5917,7 +5925,8 @@ pub(crate) fn dispatch_residual_call_iRd_kind( } // B3 piece 3: lower the PUSH_EXC_INFO / POP_EXCEPT // exc-info-stack residuals to GETFIELD_GC_R / SETFIELD_GC on the EC's - // `sys_exc_value` slot. Recognised by the + // `sys_exc_value` slot, and consume the interpreter-only propagation-root + // clear without recording a runtime CallN. Recognised by the // codewriter-stamped `pyre_helper` tag (not a funcptr address — the // residual calls the cross-crate `cpu.{get,set}_current_exception_fn` // wrappers). A balanced PUSH save + POP restore on the same descr- @@ -5930,6 +5939,7 @@ pub(crate) fn dispatch_residual_call_iRd_kind( ei.pyre_helper, majit_ir::PyreHelperKind::GetCurrentException | majit_ir::PyreHelperKind::SetCurrentException + | majit_ir::PyreHelperKind::ClearInFlightException ) && try_walker_lower_exc_info_residual( ctx, @@ -6321,6 +6331,16 @@ pub(crate) fn dispatch_residual_call_iIRd_kind( )? { return Ok(outcome); } + if let Some(outcome) = try_walker_trace_readonly_descr_attr_raise( + ctx, + op, + obj_opref, + value_opref, + w_code_ptr, + namei as usize, + )? { + return Ok(outcome); + } if let Some(specialization) = spec_gate_store_attr(|| { try_walker_specialize_store_attr( ctx, @@ -6611,7 +6631,7 @@ pub(crate) fn dispatch_residual_call_iIRd_kind( // its 4x pypy gate, and four benches' jit-stats move (most visibly // `exception_reraise_tb_depth_hot`, `loops_aborted 0 -> 63`). // - // The scan is whole-body, so one `try` anywhere in a module also charges + // The gate is whole-body, so one `try` anywhere in a module also charges // every name access in it a live dict lookup (~83ns each, linear in the // count). Measured on a 200k-iteration // `bench/synth/exc_info_module_loop_hot`, folding it is worth 1.87us -> @@ -6619,7 +6639,7 @@ pub(crate) fn dispatch_residual_call_iIRd_kind( // sits under this box's noise floor. if ctx.is_authoritative_executor && ei.pyre_helper == majit_ir::PyreHelperKind::LoadName - && !jitcode_has_exception_handler(code) + && !walk_body_has_exception_handler(ctx, code) { if let (Some(&frame_opref), Some(&name_opref)) = (r_args.first(), r_args.get(1)) { if let ( diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs index 01edb56704e..5458cc1f74b 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/resume_snapshot.rs @@ -326,6 +326,34 @@ pub(crate) fn walker_capture_snapshot_for_last_guard_impl( after_residual_call: bool, scope: GuardCaptureScope<'_>, ) -> Result<(), DispatchError> { + let instance_next_foriter_keyed = + if let Some(green_key) = ctx.fbw_mode.instance_next_foriter_green_key { + // Mint the marker as the subtype this op's opcode requires: the + // whole inlined `__next__` body is tagged, so a may-force or + // can-raise residual inside it brings its `GuardNotForced` / + // `GuardNoException` through here, and the stamp is what + // `store_final_boxes_in_guard` finds instead of inventing. + let opcode = match scope.guard_stamp { + GuardStampTarget::LastOp => ctx.trace_ctx.last_op_opcode(), + GuardStampTarget::GuardFromEnd(from_end) => { + ctx.trace_ctx.guard_op_opcode_from_end(from_end) + } + }; + let descr = + majit_metainterp::make_resume_guard_descr_instance_next_foriter(opcode, green_key); + match scope.guard_stamp { + GuardStampTarget::LastOp => ctx.trace_ctx.set_last_op_descr(descr), + GuardStampTarget::GuardFromEnd(from_end) => { + ctx.trace_ctx.set_guard_op_descr_from_end(from_end, descr) + } + } + true + } else { + false + }; + if ctx.fbw_mode.instance_next_foriter_census_active { + spec_census_record_instance_next_callee_guard(instance_next_foriter_keyed); + } // A guard whose resume snapshot cannot be built must abort the trace, // not panic. `build_vable_snapshot_boxes` requires every virtualizable // box (including the identity at `[-1]`) to carry `OpRef::ty()`; a @@ -1550,7 +1578,7 @@ pub(crate) fn collect_call_stack_overrides( // stack slot. The live vstack/color sources above emit a genuine // null-or-self sentinel only where they hold a box for the slot at // all — the `PUSH_NULL` position has none, which is what the - // `call_null_or_self_slot` pass below exists to cover. A slot that + // CALL operand resolution below exists to cover. A slot that // reaches this shadow fallback with a NULL Ref is one the walk could // not resolve — e.g. an // unmaterialized `LOAD_CONST` operand whose concrete value was @@ -1563,59 +1591,80 @@ pub(crate) fn collect_call_stack_overrides( } } } - // The `null_or_self` operand a `PUSH_NULL` leaves under a `CALL` is the one - // stack slot NO source above can speak for: nothing pushes a value there, - // so the walk holds no box and no live color for it, and the shadow's NULL - // is the same NULL an unmirrored slot reads back as. It therefore stays - // absent and declines the outer-call flush — on a slot whose correct value - // is exactly that null. The CALL's own operand layout names it without - // guessing: `[callable, null_or_self, arg0 .. arg_{argc-1}]` ends at - // `stack_end`, so the sentinel sits `argc + 1` below it, right under the - // arguments and right above the callable. - let null_or_self_slot = call_null_or_self_slot(caller_sym, call_jitcode_pc, stack_end)?; - if null_or_self_slot >= nlocals - && !overrides - .iter() - .any(|&(present, _)| present == null_or_self_slot) - { - overrides.push(( - null_or_self_slot, - std::ptr::null_mut::() as pyre_object::PyObjectRef, - )); + // A CALL's `[callable, null_or_self, arg0 .. arg_{argc-1}]` operands end at + // `stack_end`. The null_or_self slot is the one stack slot no source above + // can speak for, so synthesize its known null and require the callable + // immediately below it to prove that reconstruction reached the operand + // region. FOR_ITER has only the iterator at `stack_end - 1`: it needs no + // synthesized sentinel, and that iterator itself is the proof slot. + let operand_slots = caller_operand_slots(caller_sym, call_jitcode_pc, stack_end)?; + let (sentinel_slot, proof_slot) = match operand_slots { + CallerOperandSlots::Call { + null_or_self, + callable, + } => (Some(null_or_self), callable), + CallerOperandSlots::ForIter { iterator } => (None, iterator), + }; + if let Some(sentinel_slot) = sentinel_slot { + if sentinel_slot >= nlocals + && !overrides + .iter() + .any(|&(present, _)| present == sentinel_slot) + { + overrides.push(( + sentinel_slot, + std::ptr::null_mut::() as pyre_object::PyObjectRef, + )); + } } - // The slot immediately below null_or_self is the callable. Ref(0) is a - // valid value only in the null_or_self slot above; in the callable slot it - // means the sparse vstack/color reconstruction is incomplete. Decline - // the parent-frame image instead of publishing a CALL that will dispatch - // through a null object. - let callable_slot = null_or_self_slot.checked_sub(1)?; + // Ref(0) is valid only for the synthesized null_or_self sentinel. A null + // proof value means the sparse vstack/color reconstruction is incomplete; + // decline instead of publishing an invalid operand region. overrides .iter() - .find_map(|&(slot, value)| (slot == callable_slot).then_some(value)) + .find_map(|&(slot, value)| (slot == proof_slot).then_some(value)) .filter(|value| !value.is_null())?; Some(overrides) } -/// Absolute frame slot holding the `null_or_self` operand of the `CALL` whose -/// JitCode coordinate is `call_jitcode_pc`, for a caller whose operand stack -/// ends at `stack_end`. `None` when that coordinate does not invert to a plain -/// `CALL` — every other resume shape keeps the conservative decline. -fn call_null_or_self_slot( +enum CallerOperandSlots { + Call { + null_or_self: usize, + callable: usize, + }, + ForIter { + iterator: usize, + }, +} + +/// Absolute frame slots proving the operand region at `call_jitcode_pc`, for a +/// caller whose operand stack ends at `stack_end`. CALL names its synthetic +/// `null_or_self` sentinel and callable proof; FOR_ITER names its iterator +/// proof. `None` keeps the conservative decline for every other resume shape. +fn caller_operand_slots( caller_sym: &Sym, call_jitcode_pc: usize, stack_end: usize, -) -> Option { +) -> Option { let jc = unsafe { caller_sym.jitcode().as_ref()? }; let code = unsafe { (jc.payload.code_ptr as *const pyre_interpreter::CodeObject).as_ref()? }; let py_pc = crate::py_coord::containing_py_pc_for_jitcode_pc(&jc.payload.metadata, call_jitcode_pc) as usize; - let (pyre_interpreter::Instruction::Call { argc }, op_arg) = - pyre_interpreter::decode_instruction_at(code, py_pc)? - else { - return None; - }; - stack_end.checked_sub(argc.get(op_arg) as usize + 1) + let (instruction, op_arg) = pyre_interpreter::decode_instruction_at(code, py_pc)?; + match instruction { + pyre_interpreter::Instruction::Call { argc } => { + let null_or_self = stack_end.checked_sub(argc.get(op_arg) as usize + 1)?; + Some(CallerOperandSlots::Call { + null_or_self, + callable: null_or_self.checked_sub(1)?, + }) + } + pyre_interpreter::Instruction::ForIter { .. } => Some(CallerOperandSlots::ForIter { + iterator: stack_end.checked_sub(1)?, + }), + _ => None, + } } fn capture_inline_parent_blackhole( diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs index c619ee8edc6..c5c31b5f12d 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs @@ -10354,7 +10354,11 @@ pub(crate) fn orthodox_list_append_commit( ctx.sub_jitcode_lookup = saved_lookup; match walk_result? { - DispatchOutcome::SubReturn { result: None } => {} + DispatchOutcome::SubReturn { result } => { + if finish_inline_callee_return(ctx, result).is_some() { + return Err(DispatchError::UnexpectedNonVoidSubReturn { pc: op.pc }); + } + } _ => return Err(DispatchError::UnexpectedNonVoidSubReturn { pc: op.pc }), } @@ -10712,12 +10716,8 @@ pub(crate) fn orthodox_list_pop_commit( ctx.sub_jitcode_lookup = saved_lookup; let result = match walk_result? { - DispatchOutcome::SubReturn { - result: Some(result), - } => result, - DispatchOutcome::SubReturn { result: None } => { - return Err(DispatchError::UnexpectedVoidSubReturn { pc: op.pc }); - } + DispatchOutcome::SubReturn { result } => finish_inline_callee_return(ctx, result) + .ok_or(DispatchError::UnexpectedVoidSubReturn { pc: op.pc })?, _ => return Err(DispatchError::UnexpectedVoidSubReturn { pc: op.pc }), }; // The Integer arm commits `ll_list_int_set_len` before it boxes, so a guard @@ -11022,8 +11022,47 @@ pub(crate) fn try_walker_trace_exception_new( // Unicode constructors still require their dedicated parsing and remain // residual. let fills_os_error_slots = is_os_error_family && (2..=5).contains(&args.len()); - if !kind.has_trivial_args_constructor() && !is_os_error_family && !is_system_exit { - return Ok(None); + + // Admit the kind exactly when the concretely built instance left its extra + // slots defaulted — the slot-content test [`try_walker_trace_raise_bare_class`] + // already runs, in place of a per-kind tag that rejected a whole kind on + // faith and so kept `AttributeError(msg)` / `NameError(msg)` / + // `StopIteration()` on the opaque constructor residual. A `NULL` slot needs + // no store; a `None` one takes an explicit `SetfieldGc` below. + // + // The bare-class sibling censuses an instance built with NO arguments, so + // every slot it sees is a trace-time constant. Here the instance is built + // from the runtime operands `args`, which nothing pins — only the callable + // is guarded. A slot that reads `None` because an ARGUMENT was `None` + // would therefore be emitted as a constant `None` store while `args_w` + // keeps the live operand: `StopIteration(x)` traced with `x is None` would + // answer `e.value is None` for every later `x`. Each of these + // constructors fills a slot with either a constant default or one of the + // passed values, so requiring every argument to be non-`None` makes a + // `None` slot provably a default. The check is read only once a defaulted + // slot is actually found, leaving the all-`NULL` kinds this fold already + // admitted on their existing path. + // + // OSError / SystemExit fill their slots from the arguments, and the emit + // tail writes them from the argument OpRefs; they skip the census. + let w_none = pyre_object::w_none(); + let mut w_none_slot_descrs = Vec::new(); + if !is_os_error_family && !is_system_exit { + let any_none_arg = concrete_args.iter().any(|a| std::ptr::eq(*a, w_none)); + for (offset, value) in + unsafe { pyre_object::interp_exceptions::w_exception_traced_construction_slots(exc) } + { + if value.is_null() { + continue; + } + if !std::ptr::eq(value, w_none) || any_none_arg { + return Ok(None); + } + let Some(descr) = crate::descr::w_exception_slot_descr(kind, offset) else { + return Ok(None); + }; + w_none_slot_descrs.push(descr); + } } // `interp_exceptions.py:993-998 W_SystemExit.descr_init` stores one @@ -11202,6 +11241,18 @@ pub(crate) fn try_walker_trace_exception_new( let new_op = crate::helpers::emit_exception_new_inline(ctx.trace_ctx, kind, emitted_w_class, args_list); + // The slots the constructor defaulted to `None`. `NewWithVtable` leaves + // them null, which reads as "unset" rather than `None`, so each one the + // census collected needs its own store. + let w_none_const = ctx.trace_ctx.const_ref(w_none as i64); + for descr in w_none_slot_descrs { + let descr_index = descr.index(); + ctx.trace_ctx + .record_op_with_descr(OpCode::SetfieldGc, &[new_op, w_none_const], descr); + ctx.trace_ctx + .heapcache_setfield_cached(new_op, descr_index, w_none_const); + } + if let Some((direct_code, tuple_shape)) = system_exit_code { let code = if let Some((specialised_oo, concrete_code)) = tuple_shape { let code = if specialised_oo { @@ -11707,6 +11758,15 @@ pub(crate) fn try_walker_trace_immutable_type_attr_raise( return Ok(None); } + // Resolve the EC while declining is still free, for the `__context__` tail + // below. `walker_ensure_execution_context` returns `None` on a null + // snapshot sym or a frameless walk, and its recovery records a + // `GETFIELD_GC_R` that must not land after a guard referencing it + // (`try_walker_trace_raise_bare_class` resolves it at the same boundary). + let Some(ec) = walker_ensure_execution_context(ctx) else { + return Ok(None); + }; + // --- commit: pin the receiver, run the authentic raise, emit inline --- // The stability predicate makes the raise a pure function of `(obj, // name)`; `GuardValue` pins the one live input (`name` is a co_names @@ -11782,8 +11842,14 @@ pub(crate) fn try_walker_trace_immutable_type_attr_raise( // slot is forwarded across minor collections by the op-graph walker // and rooted by the compiled loop's gcref table thereafter. let _roots = pyre_object::gc_roots::push_roots(); + let exc_root = pyre_object::gc_roots::shadow_stack_len(); pyre_object::gc_roots::pin_root(exc); let msg = pyre_object::w_str_from_wtf8(err.message.clone()); + // The root keeps the exception alive across that allocation but does not + // fix its address: a minor collection moves the object and rewrites the + // slot, which leaves this local naming a forwarded corpse. Read the + // address back out of the slot the pin claimed. + let exc = pyre_object::gc_roots::shadow_stack_get(exc_root); let msg_const = ctx.trace_ctx.const_ref(msg as i64); let args_list = crate::helpers::emit_object_list_inline(ctx.trace_ctx, &[msg_const]); // Stamp the canonical list class exactly as `w_list_new` does (the @@ -11811,6 +11877,35 @@ pub(crate) fn try_walker_trace_immutable_type_attr_raise( .class_now_known(new_op, exc_type_ptr as usize as i64); ctx.trace_ctx .set_opref_concrete(new_op, majit_ir::Value::Ref(majit_ir::GcRef(exc as usize))); + + // `__context__` chaining on the still-virtual exception, the tail + // `try_walker_trace_raise_bare_class` carries: `active = GETFIELD_GC_R(ec, + // sys_exc_value)` then `SETFIELD_GC(exc, active, w_context)`. Without it + // the catch-side `record_inline_exception_context` compensation finds the + // context unchained and passes this exception to the resolver call, which + // forces the very allocation this fold exists to keep virtual. + let active = ctx.trace_ctx.record_op_with_descr( + OpCode::GetfieldGcR, + &[ec], + crate::descr::ec_sys_exc_value_descr(), + ); + ctx.trace_ctx.record_op_with_descr( + OpCode::SetfieldGc, + &[new_op, active], + crate::descr::w_exception_context_descr(kind), + ); + fbw_context_chained_insert(new_op); + // Apply the same context write to the concrete exception, which the + // registration above stops the compensation from performing, so Python + // code reached later in this authoritative walk observes the + // `__context__` the recorded SETFIELD performs on compiled iterations. + let active_concrete = pyre_interpreter::eval::get_current_exception(); + if !active_concrete.is_null() { + unsafe { + pyre_object::interp_exceptions::w_exception_set_context(exc, active_concrete); + } + } + // Inline-built marker: the downstream raise routing records the frame // node via the virtual `record_fresh_application_traceback` instead of // the forcing runtime hook (mirrors `try_walker_trace_raise_bare_class`). @@ -11836,15 +11931,253 @@ pub(crate) fn try_walker_trace_immutable_type_attr_raise( ))) } +/// Walker-native fold for the read-only-data-descriptor STORE_ATTR raise. +/// +/// `objspace.py:723-739` and `descroperation.py:114-126` raise +/// AttributeError after resolving a descriptor with no `__set__` and a +/// reachable `__delete__`. The interpreter predicate excludes every shortcut +/// and user-code branch; class-version guards pin the two MRO lookups, while +/// the descriptor type's `w_name` guard pins the rendered message across a +/// `type.__name__` assignment that does not change its version tag. +pub(crate) fn try_walker_trace_readonly_descr_attr_raise( + ctx: &mut WalkContext<'_, '_, Sym>, + op: &DecodedOp, + obj_op: OpRef, + value_op: OpRef, + w_code_ptr: usize, + name_idx: usize, +) -> Result, DispatchError> { + if !ctx.is_authoritative_executor || w_code_ptr == 0 { + return Ok(None); + } + let Some(concrete_obj) = walker_concrete_ref_object(ctx, obj_op) else { + return Ok(None); + }; + let concrete_value = + walker_concrete_ref_object(ctx, value_op).unwrap_or_else(pyre_object::w_none); + let name = unsafe { + let code_ptr = pyre_interpreter::w_code_get_ptr(w_code_ptr as pyre_object::PyObjectRef); + if code_ptr.is_null() { + return Ok(None); + } + let code = &*(code_ptr as *const pyre_interpreter::CodeObject); + match pyre_interpreter::pyframe::load_name_from_code(code, name_idx) { + Some(n) => n.to_string(), + None => return Ok(None), + } + }; + let Some(descr) = + pyre_interpreter::baseobjspace::readonly_descr_attr_raise_is_stable(concrete_obj, &name) + else { + return Ok(None); + }; + + let w_type = unsafe { pyre_object::w_instance_get_type(concrete_obj) }; + let Some(descr_type) = (unsafe { pyre_interpreter::typedef::r#type(descr) }) else { + return Ok(None); + }; + let descr_type = descr_type.as_ptr(); + let w_type_version_tag = unsafe { pyre_object::w_type_get_version_tag(w_type) }; + let descr_type_version_tag = unsafe { pyre_object::w_type_get_version_tag(descr_type) }; + if w_type_version_tag == 0 || descr_type_version_tag == 0 { + return Ok(None); + } + let descr_type_w_name = unsafe { pyre_object::typeobject::w_type_peek_name_obj(descr_type) }; + + // Resolve the execution context while declining is still effect-free. Its + // recovery may record an op, which must precede the fold's guard sequence. + let Some(ec) = walker_ensure_execution_context(ctx) else { + return Ok(None); + }; + + // --- commit: pin both MRO decisions, run the authentic raise, emit inline --- + // GuardClass pins the receiver payload without pinning its identity. + let physical_type = unsafe { (*concrete_obj).ob_type } as i64; + let physical_type_const = ctx.trace_ctx.const_int(physical_type); + walker_emit_fold_guard_with_snapshot( + ctx, + op.pc, + OpCode::GuardClass, + &[obj_op, physical_type_const], + )?; + ctx.trace_ctx + .heap_cache_mut() + .class_now_known(obj_op, physical_type); + + // `typeobject.py:293-301` promotes the version tag before an MRO lookup. + // Pinning the receiver type covers both the named descriptor resolution + // and the default-`__setattr__` answer. + let w_type_const = ctx.trace_ctx.const_ref(w_type as i64); + let w_type_vt_op = walker_record_getfield_gc_i_uncached( + ctx, + w_type_const, + crate::descr::type_version_tag_descr(), + ); + let w_type_vt_const = ctx.trace_ctx.const_int(w_type_version_tag as i64); + ctx.trace_ctx + .record_guard(OpCode::GuardValue, &[w_type_vt_op, w_type_vt_const], 0); + walker_capture_snapshot_for_last_guard(ctx, op.pc)?; + ctx.trace_ctx + .heap_cache_mut() + .replace_box(w_type_vt_op, w_type_vt_const); + + // The descriptor type's tag pins its general `__set__` / `__delete__` MRO + // answers (`descroperation.py:117-125`). + let descr_type_const = ctx.trace_ctx.const_ref(descr_type as i64); + let descr_type_vt_op = walker_record_getfield_gc_i_uncached( + ctx, + descr_type_const, + crate::descr::type_version_tag_descr(), + ); + let descr_type_vt_const = ctx.trace_ctx.const_int(descr_type_version_tag as i64); + ctx.trace_ctx.record_guard( + OpCode::GuardValue, + &[descr_type_vt_op, descr_type_vt_const], + 0, + ); + walker_capture_snapshot_for_last_guard(ctx, op.pc)?; + ctx.trace_ctx + .heap_cache_mut() + .replace_box(descr_type_vt_op, descr_type_vt_const); + + // `typeobject.py:1046-1058` rewrites `w_name` without mutating the class + // dictionary or its version tag. Pin the raw slot, including its initial + // null state, because it shadows the type name rendered in the message. + let descr_type_name_op = ctx.trace_ctx.record_op_with_descr( + OpCode::GetfieldGcR, + &[descr_type_const], + crate::descr::type_name_obj_descr(), + ); + let descr_type_name_const = ctx.trace_ctx.const_ref(descr_type_w_name as i64); + ctx.trace_ctx.record_guard( + OpCode::GuardValue, + &[descr_type_name_op, descr_type_name_const], + 0, + ); + walker_capture_snapshot_for_last_guard(ctx, op.pc)?; + ctx.trace_ctx + .heap_cache_mut() + .replace_box(descr_type_name_op, descr_type_name_const); + + // Pyre stores the Python-visible class separately from the physical class + // GuardClass reads. Pin that class after the mandated MRO/name guard + // sequence; unlike GuardValue on `obj_op`, this still accepts every + // receiver of the same class and ties `w_type_const` to the receiver. + walker_guard_exact_w_class(ctx, op.pc, obj_op, w_type)?; + + let result = { + let _plain_guard = pyre_interpreter::call::force_plain_eval(); + pyre_interpreter::baseobjspace::setattr_str(concrete_obj, &name, concrete_value) + }; + let Err(mut err) = result else { + // The concrete store has already run, so falling through would execute + // it twice. The predicate promises the descriptor terminal instead. + return Err(DispatchError::UnsupportedOpname { + pc: op.pc, + key: "read-only descriptor attr raise fold: stable raise unexpectedly succeeded", + }); + }; + let exc = err.to_exc_object(); + let kind = unsafe { + if !pyre_object::is_exception(exc) { + return Ok(None); + } + pyre_object::interp_exceptions::w_exception_get_kind(exc) + }; + if kind != pyre_object::interp_exceptions::ExcKind::AttributeError { + return Ok(None); + } + let exc_type_ptr = unsafe { + (*(exc as *const pyre_object::interp_exceptions::W_BaseException)) + .ob_header + .ob_type + }; + if !std::ptr::eq( + exc_type_ptr, + pyre_object::interp_exceptions::exc_kind_to_pytype(kind), + ) { + return Ok(None); + } + + let _roots = pyre_object::gc_roots::push_roots(); + let exc_root = pyre_object::gc_roots::shadow_stack_len(); + pyre_object::gc_roots::pin_root(exc); + let msg = pyre_object::w_str_from_wtf8(err.message.clone()); + // The allocation may move the exception and leave the local pointer naming + // its forwarded corpse; the shadow slot contains the live address. + let exc = pyre_object::gc_roots::shadow_stack_get(exc_root); + let msg_const = ctx.trace_ctx.const_ref(msg as i64); + let args_list = crate::helpers::emit_object_list_inline(ctx.trace_ctx, &[msg_const]); + let list_w_class = pyre_object::get_instantiate(&pyre_object::pyobject::LIST_TYPE); + let list_w_class = ctx.trace_ctx.const_ref(list_w_class as i64); + let list_w_class_descr = crate::descr::list_w_class_descr(); + let list_w_class_index = list_w_class_descr.index(); + ctx.trace_ctx.record_op_with_descr( + OpCode::SetfieldGc, + &[args_list, list_w_class], + list_w_class_descr, + ); + ctx.trace_ctx + .heapcache_setfield_cached(args_list, list_w_class_index, list_w_class); + + let class_const = ctx + .trace_ctx + .const_ref(pyre_object::interp_exceptions::lookup_exc_class_for_kind(kind) as i64); + let new_op = + crate::helpers::emit_exception_new_inline(ctx.trace_ctx, kind, class_const, args_list); + ctx.trace_ctx + .heap_cache_mut() + .class_now_known(new_op, exc_type_ptr as usize as i64); + ctx.trace_ctx + .set_opref_concrete(new_op, majit_ir::Value::Ref(majit_ir::GcRef(exc as usize))); + + let active = ctx.trace_ctx.record_op_with_descr( + OpCode::GetfieldGcR, + &[ec], + crate::descr::ec_sys_exc_value_descr(), + ); + ctx.trace_ctx.record_op_with_descr( + OpCode::SetfieldGc, + &[new_op, active], + crate::descr::w_exception_context_descr(kind), + ); + fbw_context_chained_insert(new_op); + let active_concrete = pyre_interpreter::eval::get_current_exception(); + if !active_concrete.is_null() { + unsafe { + pyre_object::interp_exceptions::w_exception_set_context(exc, active_concrete); + } + } + + fbw_built_exc_insert(new_op); + fbw_count_executed_residual(true, true); + ctx.last_exc_value = Some(new_op); + ctx.last_exc_value_concrete = ConcreteValue::Ref(exc); + ctx.fbw_mode.class_of_last_exc_is_const = true; + majit_metainterp::blackhole::BH_LAST_EXC_VALUE.with(|c| c.set(exc as i64)); + + Ok(Some(( + DispatchOutcome::SubRaise { + exc: new_op, + exc_concrete: ConcreteValue::Ref(exc), + }, + op.next_pc, + ))) +} + /// B3 piece 3: lower the PUSH_EXC_INFO / POP_EXCEPT /// exc-info-stack residuals to GETFIELD_GC_R / SETFIELD_GC on the EC's -/// `sys_exc_value` slot (`ec_sys_exc_value_descr`). +/// `sys_exc_value` slot (`ec_sys_exc_value_descr`), and consume pyre's +/// propagation-root clear without recording a runtime call. /// Recognised by the codewriter-stamped `pyre_helper` tag, NOT a funcptr /// address (the residual calls the cross-crate `cpu.{get,set}_current_ /// exception_fn` wrappers in `pyre-jit`, which `pyre-jit-trace` cannot name). /// /// * `GetCurrentException` — `get_current_exception()` (`[]→Ref`, -/// dst_bank `'r'`): the PUSH_EXC_INFO `prev` save. Emit +/// dst_bank `'r'`): the PUSH_EXC_INFO `prev` save, and also the read a +/// catch-covered bare `raise` uses to obtain the exception it re-raises. +/// Only the first owns a matching store and POP_EXCEPT restore, so only +/// the first pushes onto the saved-prev stack. Emit /// `GETFIELD_GC_R(ec, sys_exc_value)`, stamp the live `prev` concrete /// (the residual executor would have returned it) so a downstream read /// of the dst sees the right value. @@ -11852,6 +12185,17 @@ pub(crate) fn try_walker_trace_immutable_type_attr_raise( /// dst_bank `'v'`): the PUSH_EXC_INFO store and the POP_EXCEPT restore. /// Emit `SETFIELD_GC(ec, exc, sys_exc_value)` and apply the concrete /// write the authoritative walk's residual executor would have done. +/// * `ClearInFlightException` — `set_in_flight_exception(PY_NULL)` +/// (`[]→void`, dst_bank `'v'`): apply the clear to the authoritative +/// recording walk, but emit no IR. PyPy keeps the propagating exception +/// in the local `OperationError` and PUSH_EXC_INFO transfers it directly +/// to `ExecutionContext.sys_exc_operror` (`pyopcode.py:123-185, 836-863`), +/// so there is no equivalent residual clear in its compiled trace. Pyre's +/// extra TLS carrier only exposes the Rust `PyError`'s GC children while +/// the interpreter unwinds. The walk's inline traceback construction +/// never publishes that carrier at compiled runtime; leaving its clear as +/// a CallN would therefore execute an unmatched TLS write on every caught +/// exception. /// /// A balanced save (`GETFIELD`) + store + restore (`SETFIELD`) on the same /// descr-identity field with no intervening read is dead-store-eliminated, @@ -11867,13 +12211,26 @@ pub(crate) fn try_walker_lower_exc_info_residual( dst_bank: char, dst: usize, ) -> Result, DispatchError> { + if pyre_helper == majit_ir::PyreHelperKind::ClearInFlightException { + // The authoritative walk executed record_application_traceback and + // published its concrete exception in the interpreter-only carrier. + // Complete that concrete ownership transfer now. Compiled traceback + // recording is emitted as GC IR and never publishes the carrier, so + // there is deliberately no corresponding runtime operation to record. + if !r_args.is_empty() || dst_bank != 'v' { + return Ok(None); + } + pyre_interpreter::eval::set_in_flight_exception(pyre_object::PY_NULL); + return Ok(Some(())); + } + if pyre_helper == majit_ir::PyreHelperKind::GetCurrentException { // PUSH_EXC_INFO `prev = ec.sys_exc_value` — `[]→Ref`. if !r_args.is_empty() || dst_bank != 'r' { return Ok(None); } // Two Python instructions lower to this helper, and they want - // different things from a bridge seed. A bare `raise` / `RERAISE` wants + // different things from a bridge seed. A bare `raise` wants // the exception the bridge is resuming with — the compiled loop is free // to elide its `sys_exc_value` store (a balanced save/store/restore // DCEs), so the live slot is not a source there and only the seed @@ -11887,8 +12244,15 @@ pub(crate) fn try_walker_lower_exc_info_residual( // entry, so the slot is current. A seed this walk stored itself is a // view of the field either way, and reusing its OpRef keeps the // save/store/restore triple balanced. - let seed_answers_this_read = ctx.fbw_mode.current_exception_seed_from_walk_store - || super::recording_instruction_is_bare_reraise(ctx, op.pc); + // The predicate is true for `RAISE_VARARGS 0`, `RERAISE` and `FOR_ITER`, + // but only the first can reach here: `RERAISE` reads its exception off + // the vable stack and `FOR_ITER` re-raises the value its own + // `catch_exception` caught, so neither emits this helper. The name + // records the one shape that does. + let is_covered_bare_raise_read = + super::recording_raise_keeps_existing_traceback(ctx, op.pc); + let seed_answers_this_read = + ctx.fbw_mode.current_exception_seed_from_walk_store || is_covered_bare_raise_read; let (prev, prev_obj) = if let Some(seed) = ctx .fbw_mode .current_exception_seed @@ -11912,14 +12276,21 @@ pub(crate) fn try_walker_lower_exc_info_residual( prev, majit_ir::Value::Ref(majit_ir::GcRef(prev_obj as usize)), ); - // Save (OpRef, concrete) for the matching POP_EXCEPT restore, and mark - // the immediately-following `set_current_exception` as this PUSH's slot - // store (not a restore). The codewriter pushes `prev` then `exc` onto - // the operand stack and POP_EXCEPT pops them, but the walker resolves - // the popped `prev` operand to the caught exception, not the saved - // prev; the LIFO stack carries the authoritative value instead. - FBW_EXC_PREV.with(|s| s.borrow_mut().push((prev, prev_obj))); - FBW_EXC_PENDING_PUSH_SET.with(|c| c.set(true)); + // Only PUSH_EXC_INFO owns a matching set + POP_EXCEPT pair. A covered + // bare raise uses the same read helper to obtain the exception it + // re-raises, but has no following PUSH store. Treating that read as a + // save arms the next POP as a PUSH and leaves the bare raise's value on + // this stack, so a second enclosing POP restores the inner exception. + // For PUSH_EXC_INFO, save (OpRef, concrete) for the matching restore and + // mark the immediately-following set as this PUSH's slot store. The + // codewriter pushes `prev` then `exc` onto the operand stack and + // POP_EXCEPT pops them, but the walker resolves the popped `prev` + // operand to the caught exception, not the saved prev; the LIFO stack + // carries the authoritative value instead. + if !is_covered_bare_raise_read { + FBW_EXC_PREV.with(|s| s.borrow_mut().push((prev, prev_obj))); + FBW_EXC_PENDING_PUSH_SET.with(|c| c.set(true)); + } write_residual_call_result_to_dst(ctx, op.pc, dst, 'r', prev)?; return Ok(Some(())); } diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/tests.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/tests.rs index 44139b24d66..9b76fca0d7a 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/tests.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/tests.rs @@ -103,6 +103,19 @@ fn vstack_permuted_for_iter_entry_uses_block_head_target() { assert_eq!(vstack_step_py_pc(&pyjit.metadata, 120, 18), 18); } +#[test] +fn exact_py_pc_preserves_a_later_disjoint_emission_region() { + let mut pyjit = crate::PyJitCode::skeleton(std::ptr::null()); + pyjit.metadata.py_floor_by_jit_pc = vec![(0, 0), (10, 11), (20, 21)]; + pyjit.metadata.py_exact_by_jit_pc = vec![(0, 0), (10, 11), (20, 21), (30, 11)]; + + assert_eq!(vstack_containing_py_pc(&pyjit.metadata, 35), 21); + assert_eq!( + crate::pyjitcode::exact_py_pc_for_jitcode_pc(&pyjit.metadata.py_exact_by_jit_pc, 35), + Some(11), + ); +} + /// Build a fresh `TraceCtx`. Uses the public `for_test_types` + /// `const_ref` / `make_fail_descr` factories so the fixture stays /// out of `pub(crate)` API. @@ -2779,8 +2792,8 @@ fn inline_call_recursion_writes_subreturn_into_caller_dst_register() { trace_ctx: &mut tc, is_top_level: true, sub_jitcode_lookup: &lookup, - last_exc_value: None, - last_exc_value_concrete: ConcreteValue::Null, + last_exc_value: Some(arg_value), + last_exc_value_concrete: ConcreteValue::Int(123), entry_py_pc: EntryPyPc::Py(0), outer_resume_marker_jit_pc: None, outer_jitcode_index: 0, @@ -2801,6 +2814,15 @@ fn inline_call_recursion_writes_subreturn_into_caller_dst_register() { let (outcome, end_pc) = walk(&caller_code, 0, &mut wc).expect("caller must walk to terminator"); assert_eq!(outcome, DispatchOutcome::Terminate); assert_eq!(end_pc, caller_code.len()); + assert_eq!( + wc.last_exc_value, None, + "normal inline return must clear the caller's stale exception", + ); + assert_eq!( + wc.last_exc_value_concrete, + ConcreteValue::Null, + "normal inline return must clear the caller's concrete exception shadow", + ); drop(wc); // dst register r5 must equal the arg the caller passed (since // callee's `ref_return r0` returns its registers_r[0] which @@ -9878,6 +9900,7 @@ fn inline_call_r_v_accepts_void_returning_callee() { ]; let mut tc = fresh_trace_ctx(); let mut regs_r = distinct_const_refs(&mut tc, 4); + let stale_exc = regs_r[0]; let descr = done_descr_ref_for_tests(); let mut descr_pool: Vec = (0..16).map(|i| make_fail_descr(1 + i)).collect(); descr_pool[7] = make_jitcode_descr(7); @@ -9898,8 +9921,8 @@ fn inline_call_r_v_accepts_void_returning_callee() { trace_ctx: &mut tc, is_top_level: true, sub_jitcode_lookup: &lookup, - last_exc_value: None, - last_exc_value_concrete: ConcreteValue::Null, + last_exc_value: Some(stale_exc), + last_exc_value_concrete: ConcreteValue::Int(123), entry_py_pc: EntryPyPc::Py(0), outer_resume_marker_jit_pc: None, outer_jitcode_index: 0, @@ -9919,6 +9942,8 @@ fn inline_call_r_v_accepts_void_returning_callee() { let (outcome, _) = walk(&caller_code, 0, &mut wc).expect("inline_call_r_v with void callee must succeed"); assert_eq!(outcome, DispatchOutcome::Terminate); + assert_eq!(wc.last_exc_value, None); + assert_eq!(wc.last_exc_value_concrete, ConcreteValue::Null); } #[test] diff --git a/pyre/pyre-jit-trace/src/py_coord.rs b/pyre/pyre-jit-trace/src/py_coord.rs index c2fbc566096..67d158305a9 100644 --- a/pyre/pyre-jit-trace/src/py_coord.rs +++ b/pyre/pyre-jit-trace/src/py_coord.rs @@ -61,6 +61,19 @@ pub fn containing_py_pc_for_jitcode_pc_public(jitcode_index: i32, offset: i32) - Some(containing_py_pc_for_jitcode_pc(&payload.metadata, offset as usize) as i32) } +/// Return the per-emission-run Python owner of a stored JitCode offset. +/// +/// Unlike the floor tier, this preserves a Python instruction that emits in +/// multiple disjoint JitCode regions. Empty skeleton metadata returns `None`. +pub fn exact_py_pc_for_jitcode_pc_public(jitcode_index: i32, offset: i32) -> Option { + let payload = crate::state::pyjitcode_for_jitcode_index(jitcode_index)?; + crate::pyjitcode::exact_py_pc_for_jitcode_pc( + &payload.metadata.py_exact_by_jit_pc, + offset as usize, + ) + .map(|py| py as i32) +} + /// Advance a Python instruction coordinate past resume trivia when code is available. pub fn skip_python_trivia_forward_public( jitcode_index: i32, diff --git a/pyre/pyre-jit-trace/src/state.rs b/pyre/pyre-jit-trace/src/state.rs index c41d1c7640a..513a0efbd83 100644 --- a/pyre/pyre-jit-trace/src/state.rs +++ b/pyre/pyre-jit-trace/src/state.rs @@ -1239,23 +1239,48 @@ pub fn on_live_marker( clear_dead_ref_registers_at_live_marker(bh, marker_pc); } -/// Whether a JitCode exception exit came from the Python bare-reraise -/// instruction path. `RAISE_VARARGS 0` and `RERAISE` both use -/// RaiseWithExplicitTraceback and skip record_application_traceback. -pub fn jitcode_pc_is_bare_reraise(jitcode_index: i32, offset: i32) -> bool { +/// Whether an exception exit emitted for the Python instruction at `py_pc` +/// re-raises a value that already carries this frame's traceback node, so +/// `record_application_traceback` must be skipped. +/// +/// `RAISE_VARARGS 0` and `RERAISE` both use RaiseWithExplicitTraceback. +/// FOR_ITER's exception-match mismatch arm re-raises the value its own +/// `catch_exception` caught — `pyopcode.py:1310` re-raises `e` untouched. It +/// keeps this frame's existing node only when the `space.next` residual +/// attached that node; an inlined `__next__` instead leaves the callee's node +/// at the chain head. +pub fn raise_at_py_pc_keeps_existing_traceback(raw_code: &CodeObject, py_pc: usize) -> bool { + match unsafe { pyre_interpreter::decode_instruction_at(raw_code, py_pc) } { + Some((Instruction::RaiseVarargs { .. }, op_arg)) => u32::from(op_arg) == 0, + Some((Instruction::Reraise { .. }, _)) | Some((Instruction::ForIter { .. }, _)) => true, + _ => false, + } +} + +/// [`raise_at_py_pc_keeps_existing_traceback`] resolved from a jitcode +/// coordinate. +pub fn jitcode_pc_raise_keeps_existing_traceback(jitcode_index: i32, offset: i32) -> bool { let Some(raw_code) = raw_code_for_jitcode_index(jitcode_index) else { return false; }; - let Some(py_pc) = - crate::py_coord::containing_py_pc_for_jitcode_pc_public(jitcode_index, offset) + let Some(py_pc) = crate::py_coord::exact_py_pc_for_jitcode_pc_public(jitcode_index, offset) + .or_else(|| crate::py_coord::containing_py_pc_for_jitcode_pc_public(jitcode_index, offset)) else { return false; }; - match unsafe { pyre_interpreter::decode_instruction_at(&*raw_code, py_pc as usize) } { - Some((Instruction::RaiseVarargs { .. }, op_arg)) => u32::from(op_arg) == 0, - Some((Instruction::Reraise { .. }, _)) => true, - _ => false, - } + raise_at_py_pc_keeps_existing_traceback(unsafe { &*raw_code }, py_pc as usize) +} + +/// Whether the source function behind `jitcode_index` carries a Python +/// `try`/`except` handler, read from `co_exceptiontable` (`pycode.py:145`) — +/// the same table the codewriter's `decode_exception_catch_sites` builds its +/// `catch_for_pc` map from. +/// +/// `None` when the index resolves to no `CodeObject`: a native drain portal +/// carries a null `code_ptr`. +pub fn jitcode_source_has_exception_handler(jitcode_index: i32) -> Option { + let raw_code = raw_code_for_jitcode_index(jitcode_index)?; + Some(!unsafe { &*raw_code }.exceptiontable.is_empty()) } /// `framework.py` `root_walker.walk_roots` hook for the boxed `Ref` @@ -10199,6 +10224,8 @@ impl JitState for PyreJitState { // This is deferred until after `maps` is read (below) via a // re-adjustment of the semantic mirror length. let stack_only = bridge_valuestackdepth.saturating_sub(nlocals); + let resume_maps = + crate::state::bridge_semantic_maps_from_jitcode_pc(frame0.jitcode_index, frame0.pc); let bridge_reg_len = nlocals + stack_only; let mut bridge_registers_r = vec![OpRef::NONE; bridge_reg_len]; // RPython parity: after A.1 the guard-recovery path calls @@ -10245,9 +10272,13 @@ impl JitState for PyreJitState { // consume stage directly — matching `resume.py:1052-1055 // rebuild_from_resumedata` → `consume_boxes(f.get_current_position_info(), // registers_i, registers_r, registers_f)`, which fills all three banks - // uniformly at the guard's resume position. - let seed_deferred_to_overlay = - crate::state::frame_pc_is_resolved_offset_at(frame0.jitcode_index, frame0.pc); + // uniformly at the guard's resume position. A resolved `-live-` + // offset alone does not identify that branch shape: after-residual + // guards carry one too. The branch is the shape whose guard-time + // pcdep depth is deeper than the resumed virtualizable stack; when the + // depths agree, the frame stream is authoritative and must be stamped + // directly even if the frame-array image is still pre-call. + let seed_deferred_to_overlay = resume_maps.stack_depth_at_pc > stack_only; let mut bridge_stamp_orphans = seed_deferred_to_overlay.then(Vec::new); let mut value_cursor = 0usize; for ®_idx in ®_indices.int { @@ -10348,8 +10379,7 @@ impl JitState for PyreJitState { // `[0,nlocals)` prefix to identity colors (now retired). Invert each // live color to its slot via `semantic_ref_slot_for_reg_color` so the // mirror is correct under freely-colored locals. - let maps = - crate::state::bridge_semantic_maps_from_jitcode_pc(frame0.jitcode_index, frame0.pc); + let maps = resume_maps; // For a kept-stack branch guard, the vable's runtime // `valuestackdepth` reflects the merge-target depth (post // consumption) rather than the guard's deeper live depth. The @@ -10778,9 +10808,50 @@ impl JitState for PyreJitState { bridge_array_len, &vable_array_values, ); + // resume.py:1042-1057 `rebuild_from_resumedata` fills the live MIFrame + // register banks with `consume_boxes`; operand-stack boxes are the + // authoritative values at an after-residual guard. The separately + // decoded virtualizable array can still contain the pre-call operand + // because the residual result has not passed through a frame-array + // write. Locals and cells keep the vable-image authority established + // above: a register color at an interior resume point can hold a dead + // or stale local value. Kept-stack branch guards retain their existing + // post-overlay path: their deeper pcdep stack is not the resumed + // virtualizable depth. + let mut bridge_array_items = vable_array_items.clone(); + let mut bridge_array_values = live_array_values.clone(); + bridge_array_values.resize( + bridge_array_len, + majit_ir::Value::Ref(majit_ir::GcRef::NULL), + ); + if !seed_deferred_to_overlay { + let semantic_array_len = sym.registers_r.len().min(bridge_array_len); + if bridge_array_items.len() < semantic_array_len { + let null_ref = ctx.const_ref(pyre_object::PY_NULL as i64); + bridge_array_items.resize(semantic_array_len, null_ref); + } + for (slot, &opref) in sym + .registers_r + .iter() + .take(semantic_array_len) + .enumerate() + .skip(nlocals) + { + if opref.is_none() { + continue; + } + bridge_array_items[slot] = opref; + if let Some(value) = ctx.box_value(opref) { + if !matches!(value, majit_ir::Value::Void) { + bridge_array_values[slot] = value; + store_live_frame_array_slot(sym.concrete_vable_ptr as usize, slot, value); + } + } + } + } sym.concrete_locals = (0..nlocals) .map(|i| { - live_array_values + bridge_array_values .get(i) .copied() .map(concrete_value_from_ir_value) @@ -10789,7 +10860,7 @@ impl JitState for PyreJitState { .collect(); sym.concrete_stack = (0..stack_only) .map(|i| { - live_array_values + bridge_array_values .get(nlocals + i) .copied() .map(concrete_value_from_ir_value) @@ -10798,17 +10869,13 @@ impl JitState for PyreJitState { .collect(); let mut concrete_values = Vec::with_capacity(vable_scalar_values.len() + bridge_array_len); concrete_values.extend_from_slice(&vable_scalar_values); - let taken_concrete = live_array_values.len().min(bridge_array_len); - concrete_values.extend_from_slice(&live_array_values[..taken_concrete]); - while concrete_values.len() < vable_scalar_values.len() + bridge_array_len { - concrete_values.push(majit_ir::Value::Ref(majit_ir::GcRef::NULL)); - } + concrete_values.extend_from_slice(&bridge_array_values); crate::state::seed_virtualizable_boxes( ctx, sym.frame, vable_ref_value, &scalar_oprefs, - &vable_array_items, + &bridge_array_items, bridge_array_len, &concrete_values, sym.concrete_vable_ptr as *const u8, @@ -13711,7 +13778,7 @@ mod tests { } #[test] - fn test_setup_bridge_sym_preserves_resumed_stack_tail() { + fn test_setup_bridge_sym_preserves_vable_locals_and_resumed_stack() { use majit_ir::resumedata::{RebuiltFrame, RebuiltValue}; use majit_metainterp::jitcode::JitCodeBuilder; use pyre_interpreter::pyframe::PyFrame; @@ -13757,9 +13824,9 @@ mod tests { abort_permanent_py_pc_by_jit_pc: Vec::new(), merge_entry_by_green: Vec::new(), pcdep_by_jit_pc: vec![(0, Vec::new())], - depth_pred_by_jit_pc: vec![(0, 2)], - depth_trivia_marker_by_jit_pc: vec![(0, Some(2))], - depth_trivia_pred_by_jit_pc: vec![(0, Some(2))], + depth_pred_by_jit_pc: vec![(0, 1)], + depth_trivia_marker_by_jit_pc: vec![(0, Some(1))], + depth_trivia_pred_by_jit_pc: vec![(0, Some(1))], depth_containing_by_jit_pc: Vec::new(), depth_block_head_by_jit_pc: Vec::new(), pcdep_trivia_marker_by_jit_pc: Vec::new(), @@ -13804,8 +13871,9 @@ mod tests { Type::Ref, // debugdata Type::Ref, // w_globals Type::Ref, // local0 + Type::Ref, // stale cell Type::Ref, // stack0 - Type::Ref, // stack1 + Type::Ref, // live cell from the vable image ]; let mut ctx = TraceCtx::for_test_types(&input_types); // Slots 0 (frame) and 1 (ec) are both Ref-typed per `input_types` @@ -13816,14 +13884,15 @@ mod tests { let mut sym = PyreSym::new_uninit(OpRef::input_arg_ref(0)); sym.frame = OpRef::input_arg_ref(0); sym.execution_context = OpRef::input_arg_ref(1); - sym.nlocals = 1; - sym.valuestackdepth = 1; + sym.nlocals = 2; + sym.valuestackdepth = 2; sym.concrete_vable_ptr = frame_ptr as *mut u8; let local0 = w_int_new(41) as i64; - let stack0 = w_int_new(42) as i64; - let stack1 = w_int_new(43) as i64; + let stale_cell = w_int_new(42) as i64; + let stack0 = w_int_new(43) as i64; let globals = w_int_new(44) as i64; + let live_cell = w_int_new(45) as i64; let fail_values = [ frame_ptr as i64, 0, @@ -13833,8 +13902,9 @@ mod tests { 0, globals, local0, + stale_cell, stack0, - stack1, + live_cell, ]; let fail_types = [ Type::Ref, @@ -13847,6 +13917,7 @@ mod tests { Type::Ref, Type::Ref, Type::Ref, + Type::Ref, ]; let resume_data = majit_metainterp::ResumeDataResult { frames: vec![RebuiltFrame { @@ -13854,11 +13925,16 @@ mod tests { pc: 0, py_pc: 0, values: vec![ - RebuiltValue::Box(7, Type::Ref), + RebuiltValue::Const(majit_ir::Const::Ref(majit_ir::GcRef::NULL)), RebuiltValue::Box(8, Type::Ref), RebuiltValue::Box(9, Type::Ref), ], }], + // Exercise both authorities in one bridge setup. The first local's + // register is a dead null and the second local/cell's register is a + // stale non-null object; both vable entries are live. Conversely, + // the stack entry in the vable image is stale while the frame + // register contains the after-call result. virtualizable_values: vec![ RebuiltValue::Box(0, Type::Ref), RebuiltValue::Box(2, Type::Int), @@ -13867,8 +13943,8 @@ mod tests { RebuiltValue::Box(5, Type::Ref), RebuiltValue::Box(6, Type::Ref), RebuiltValue::Box(7, Type::Ref), - RebuiltValue::Box(8, Type::Ref), - RebuiltValue::Box(9, Type::Ref), + RebuiltValue::Box(10, Type::Ref), + RebuiltValue::Box(7, Type::Ref), ], virtualref_values: Vec::new(), storage: None, @@ -13896,12 +13972,45 @@ mod tests { vec![ OpRef::input_arg_ref(7), OpRef::input_arg_ref(8), - OpRef::input_arg_ref(9) + OpRef::input_arg_ref(9), ] ); - assert_eq!(sym.symbolic_local_types, vec![Type::Ref]); - assert_eq!(sym.symbolic_stack_types, vec![Type::Ref, Type::Ref]); - assert_eq!(sym.bridge_local_oprefs, Some(vec![OpRef::input_arg_ref(7)])); + assert_eq!(sym.symbolic_local_types, vec![Type::Ref, Type::Ref]); + assert_eq!(sym.symbolic_stack_types, vec![Type::Ref]); + assert_eq!( + sym.bridge_local_oprefs, + Some(vec![OpRef::input_arg_ref(7), OpRef::input_arg_ref(8)]) + ); + assert_eq!( + sym.concrete_locals, + vec![ConcreteValue::Int(41), ConcreteValue::Int(45)] + ); + let array_base = crate::virtualizable_gen::NUM_VABLE_SCALARS; + assert_eq!( + ctx.virtualizable_entry_at(array_base), + Some(( + OpRef::input_arg_ref(7), + majit_ir::Value::Ref(majit_ir::GcRef(local0 as usize)), + )), + ); + assert_eq!( + ctx.virtualizable_entry_at(array_base + 1), + Some(( + OpRef::input_arg_ref(10), + majit_ir::Value::Ref(majit_ir::GcRef(live_cell as usize)), + )), + ); + assert_eq!( + ctx.virtualizable_entry_at(array_base + 2), + Some(( + OpRef::input_arg_ref(9), + majit_ir::Value::Ref(majit_ir::GcRef(stack0 as usize)), + )), + ); + assert_eq!( + ctx.box_value(OpRef::input_arg_ref(9)), + Some(majit_ir::Value::Ref(majit_ir::GcRef(stack0 as usize))), + ); } #[test] diff --git a/pyre/pyre-jit-trace/src/trace.rs b/pyre/pyre-jit-trace/src/trace.rs index b4ee7c291ce..ef0059ef8a4 100644 --- a/pyre/pyre-jit-trace/src/trace.rs +++ b/pyre/pyre-jit-trace/src/trace.rs @@ -431,6 +431,12 @@ thread_local! { /// executing thread's trace; no cross-thread mutable JIT state is needed. static RANGE_FORITER_DEMOTED: std::cell::RefCell> = std::cell::RefCell::new(std::collections::HashSet::new()); + /// FOR_ITER sites whose user-instance `__next__` inline has reached a + /// guard-failure bridge. Only bridge walks consult this set: the primary + /// loop retains its inline, while the bridge records generic `jit_next` so + /// exhaustion is converted by the caller opcode. + static INSTANCE_NEXT_FORITER_BRIDGE_DEMOTED: std::cell::RefCell> = + std::cell::RefCell::new(std::collections::HashSet::new()); /// The current bridge trace's full-body walk hit a deterministic /// structural decline. The walker only knows `(w_code, start_pc)`; the /// bridge launcher still has the originating guard descr and consumes this @@ -483,6 +489,17 @@ pub fn range_foriter_demote_once(site_key: u64) -> bool { RANGE_FORITER_DEMOTED.with(|s| s.borrow_mut().insert(site_key)) } +pub(crate) fn instance_next_foriter_bridge_demoted(key: u64) -> bool { + INSTANCE_NEXT_FORITER_BRIDGE_DEMOTED.with(|s| s.borrow().contains(&key)) +} + +/// Mark a user-instance `__next__` guard source so its bridge walk keeps the +/// generic FOR_ITER residual. The set is deliberately bridge-only; it does +/// not retire the optimized loop or suppress the inline on a later root trace. +pub fn instance_next_foriter_bridge_demote_once(green_key: u64) -> bool { + INSTANCE_NEXT_FORITER_BRIDGE_DEMOTED.with(|s| s.borrow_mut().insert(green_key)) +} + fn midbody_post_marker_is_effect_free(code: &CodeObject, start_pc: usize) -> bool { (start_pc..code.instructions.len()).all(|pc| { let Some((instruction, _)) = pyre_interpreter::decode_instruction_at(code, pc) else { diff --git a/pyre/pyre-jit/src/call_jit.rs b/pyre/pyre-jit/src/call_jit.rs index 8973aaa4c68..35f804edd2d 100644 --- a/pyre/pyre-jit/src/call_jit.rs +++ b/pyre/pyre-jit/src/call_jit.rs @@ -721,18 +721,40 @@ pub(crate) extern "C" fn record_caught_blackhole_traceback( let Ok(opcode_position) = i32::try_from(opcode_position) else { return; }; - if pyre_jit_trace::state::jitcode_pc_is_bare_reraise(jitcode_index, opcode_position) { - return; - } + let forwards_existing = pyre_jit_trace::state::jitcode_pc_raise_keeps_existing_traceback( + jitcode_index, + opcode_position, + ); let frame_ptr = frame_value as *mut PyFrame; if frame_ptr.is_null() || exc_value == 0 { return; } - let last_instruction = pyre_jit_trace::py_coord::containing_py_pc_for_jitcode_pc_public( + // A forwarding raise keeps the chain it arrived with only when this frame + // attached the head. When the raise came out of an inlined callee, the + // head names the callee and this frame still owes one node. The ownership + // check preserves `pyopcode.py:147-148 handle_operation_error` as the + // one-node-per-frame-per-delivery authority. + if forwards_existing { + let owns_head = unsafe { + let head = + pyre_object::interp_exceptions::w_exception_get_traceback(exc_value as PyObjectRef); + !head.is_null() + && pyre_interpreter::pytraceback::is_pytraceback(head) + && pyre_interpreter::pytraceback::w_pytraceback_get_frame(head) == frame_ptr + }; + if owns_head { + return; + } + } + let resolved = pyre_jit_trace::py_coord::containing_py_pc_for_jitcode_pc_public( jitcode_index, opcode_position, - ) - .map_or(unsafe { (*frame_ptr).last_instr as i64 }, i64::from); + ); + let exact = + pyre_jit_trace::py_coord::exact_py_pc_for_jitcode_pc_public(jitcode_index, opcode_position); + let last_instruction = exact + .or(resolved) + .map_or(unsafe { (*frame_ptr).last_instr as i64 }, i64::from); // One catch can be reached by two recorders: this hook, emitted into the // loop trace by the in-trace handler-entry arm, and the IR-virtual node the // bridge handler-entry arm builds when the exception edge deopts into a @@ -819,10 +841,13 @@ pub(crate) extern "C" fn record_inline_traceback_for_recording( let Ok(opcode_position) = i32::try_from(opcode_position) else { return; }; - // Inline frames follow the same RaiseWithExplicitTraceback rule as - // concrete blackhole frames: a bare reraise preserves the traceback - // already attached by the original raising instruction. - if pyre_jit_trace::state::jitcode_pc_is_bare_reraise(jitcode_index, opcode_position) { + // Inline frames follow the same rule as concrete blackhole frames: a raise + // that forwards an already-propagating exception preserves the traceback + // attached by the original raising instruction. + if pyre_jit_trace::state::jitcode_pc_raise_keeps_existing_traceback( + jitcode_index, + opcode_position, + ) { return; } if exc_value == 0 || w_code_value == 0 { @@ -909,18 +934,12 @@ pub(crate) extern "C" fn record_discarded_level_traceback( if raw_code.is_null() { return; } - // Same RaiseWithExplicitTraceback rule the other two recorders follow: a - // bare reraise preserves the traceback the original raise attached. - let bare_reraise = match unsafe { - pyre_interpreter::decode_instruction_at(&*raw_code, last_instruction as usize) - } { - Some((pyre_interpreter::Instruction::RaiseVarargs { .. }, op_arg)) => { - u32::from(op_arg) == 0 - } - Some((pyre_interpreter::Instruction::Reraise { .. }, _)) => true, - _ => false, - }; - if bare_reraise { + // Same rule the other two recorders follow: a raise that forwards an + // already-propagating exception keeps the traceback it arrived with. + if pyre_jit_trace::state::raise_at_py_pc_keeps_existing_traceback( + unsafe { &*raw_code }, + last_instruction as usize, + ) { return; } let w_globals = unsafe { pyre_interpreter::w_code_get_w_globals(w_code) }; @@ -2728,26 +2747,32 @@ pub fn blackhole_resume_via_rd_numb( ) }); if !frame_ptr.is_null() { - let last_instruction = jitcode_index - .and_then(|index| { - pyre_jit_trace::py_coord::containing_py_pc_for_jitcode_pc_public( - index, + match jitcode_index { + // Every indexed frame recorded during blackhole + // propagation goes through the guarded recorder; + // `pyopcode.py:147-148 handle_operation_error` is + // upstream's single traceback attach point. + Some(jitcode_index) => record_caught_blackhole_traceback( + err.exc_object as i64, + frame_ptr as i64, + i64::from(jitcode_index), + last_opcode_position as i64, + ), + None => { + m73_lastinstr_audit( + "exit_guard_exc", + jitcode_index, last_opcode_position as i32, - ) - }) - .map_or(unsafe { (*frame_ptr).last_instr as i64 }, i64::from); - m73_lastinstr_audit( - "exit_guard_exc", - jitcode_index, - last_opcode_position as i32, - frame_ptr, - ); - unsafe { - pyre_interpreter::pytraceback::record_application_traceback( - err.exc_object, - frame_ptr, - last_instruction, - ); + frame_ptr, + ); + unsafe { + pyre_interpreter::pytraceback::record_application_traceback( + err.exc_object, + frame_ptr, + (*frame_ptr).last_instr as i64, + ); + } + } } } // `guard_exc` is now owned by the typed @@ -2882,10 +2907,10 @@ pub fn blackhole_resume_via_rd_numb( .get(last_opcode_position.saturating_sub(10)..=last_opcode_position + 1) .unwrap_or(&[]) .to_vec(); - let bare_reraise = bh_opcode_at + let keeps_existing_traceback = bh_opcode_at .is_some_and(|opcode| opcode == majit_metainterp::jitcode::insns::BC_RERAISE) || jitcode_index.is_some_and(|index| { - pyre_jit_trace::state::jitcode_pc_is_bare_reraise( + pyre_jit_trace::state::jitcode_pc_raise_keeps_existing_traceback( index, last_opcode_position as i32, ) @@ -2894,8 +2919,8 @@ pub fn blackhole_resume_via_rd_numb( // BlackholeInterpreter frame at a time. Record each exiting // frame before advancing to nextblackholeinterp, matching // pytraceback.py record_application_traceback at every Python - // frame boundary. A bare reraise preserves the existing chain. - if !bare_reraise && !frame_ptr.is_null() { + // frame boundary. A forwarding raise preserves the existing chain. + if !keeps_existing_traceback && !frame_ptr.is_null() { if let Some(jitcode_index) = jitcode_index { record_caught_blackhole_traceback( exc_value, @@ -2928,7 +2953,8 @@ pub fn blackhole_resume_via_rd_numb( last_opcode_position={last_opcode_position} opcode={:?} \ operand_reg={bh_raise_reg:?} registers_r.len={bh_regs_r_len} \ regs_holding_exception={:?} \ - code[-10..]={bh_code_window:?} bare_reraise={bare_reraise} \ + code[-10..]={bh_code_window:?} \ + keeps_existing_traceback={keeps_existing_traceback} \ guard_exc={guard_exc:x} py_pc={:?} entry_py_pc={:?} \ deadframe_types={deadframe_types:?} deadframe={deadframe:x?} \ registers_r={bh_regs_r:x?}", diff --git a/pyre/pyre-jit/src/eval.rs b/pyre/pyre-jit/src/eval.rs index 0391846894b..bae407664ff 100644 --- a/pyre/pyre-jit/src/eval.rs +++ b/pyre/pyre-jit/src/eval.rs @@ -6653,7 +6653,7 @@ fn drain_error_from_exc_ref(exc: i64) -> Option { // StopIteration = drain complete; `ln` re-derives its own // loop-exit StopIteration on its next `next()`. - if err.kind != pyre_interpreter::PyErrorKind::StopIteration { + if !err.matches_stop_iteration() { pending_err = Some(err); } break; @@ -6894,12 +6894,12 @@ fn drive_unpack_iterable_trace( // producer today) and is promoted instead of discarded; an exit that // already picked one keeps it, since that one is the earlier failure. if let Err(err) = pyre_interpreter::stack_check::drain_jit_pending_exception() - && err.kind != pyre_interpreter::PyErrorKind::StopIteration + && !err.matches_stop_iteration() { pending_err.get_or_insert(err); } if let Some(err) = crate::call_jit::take_ca_exception() - && err.kind != pyre_interpreter::PyErrorKind::StopIteration + && !err.matches_stop_iteration() { pending_err.get_or_insert(err); } @@ -9470,6 +9470,16 @@ fn handle_fail( return HandleFailOutcome::ResumeInBlackhole; } + // A keyed failure proves this FOR_ITER site's instance-`__next__` + // specialization unsuitable. Deliberately discard the marker's bool: + // the first insertion retires the specialization, while later failures + // must leave it retired so the next walk compiles the generic residual. + // Do not override the ordinary bridge gate; `compile.py:702-703` makes + // `must_compile() && !stack_almost_full()` the complete decision. + if let Some(foriter_key) = descr_arc.instance_next_foriter_green_key() { + let _ = pyre_jit_trace::trace::instance_next_foriter_bridge_demote_once(foriter_key); + } + // compile.py:702-703: must_compile() AND not stack_almost_full() if should_bridge && !stack_almost_full() { let is_tracing = { diff --git a/pyre/pyre-jit/src/jit/codewriter.rs b/pyre/pyre-jit/src/jit/codewriter.rs index d7dc997b4a5..6986b61298c 100644 --- a/pyre/pyre-jit/src/jit/codewriter.rs +++ b/pyre/pyre-jit/src/jit/codewriter.rs @@ -3281,51 +3281,17 @@ fn new_shadow_graph(code: &CodeObject) -> super::flow::FunctionGraph { new_shadow_graph_with_portal_inputs(code, FrameInputs::None) } -fn attach_catch_exception_edge( - code: &CodeObject, +fn attach_materialized_exception_edge( graph: &mut super::flow::FunctionGraph, block: &super::flow::BlockRef, target: &SpamBlockRef, - source_state: &FrameState, - site: &ExceptionCatchSite, + edge_state: &FrameState, ) -> super::flow::LinkRef { - // `flowcontext.py:148-149 guessexception` sets - // `block.exitswitch = c_last_exception` before the link is - // attached. Run the source-block side first so that the link - // construction below sees a stable target/source pair. - { - let mut block_mut = block.borrow_mut(); - block_mut.exitswitch = Some(super::flow::ExitSwitch::Value( - super::flow::c_last_exception().into(), - )); - } - - // `flowcontext.py:130-134 guessexception` synthesises the - // `(last_exception, last_exc_value)` Variable pair for this - // edge. `exception_landing_state` clones `source_state` and - // sets `last_exception` to the fresh pair, so the same - // Variables can be threaded into BOTH `link.args` (via - // `getoutputargs` below) AND `link.extravars`. - // - // Reshape the cloned state to the handler-entry layout: unwind the - // operand stack to the handler's try-level `stack_depth`, push the - // `lasti` box (when flagged) and the exception value, and retarget - // `next_offset` to the handler PC — the exact transform - // `handler_entry_state_from_catch_site` applies when it builds the - // catch landing's framestate / inputargs. A `raise` reached from a - // DEEPER operand-stack PC (e.g. mid-expression, inside a call's - // argument build-up) otherwise leaves `edge_state` at the raise - // point's stack depth and `next_offset`, so it is NOT union- - // compatible with the landing (`FrameState::union` declines on the - // `next_offset` / stack-length mismatch and `update_catch_landing_ - // state` silently keeps the landing as-is). The positional - // `getoutputargs` then shifts every arg past the stack delta, - // pairing a Ref slot with the landing's `last_exception` Int slot — - // surfacing downstream as an `int_copy` kind-mismatch at assemble - // time and as an `enforce_input_args` colour collision when the - // shifted CFG coalesce pair merges two landing inputargs. - let seeded = exception_landing_state(graph, source_state); - let edge_state = handler_entry_state_from_catch_site(code, graph, &seeded, site); + // `flowcontext.py:148-149 guessexception` marks the source block as + // can-raise before attaching the materialized exception link. + block.borrow_mut().exitswitch = Some(super::flow::ExitSwitch::Value( + super::flow::c_last_exception().into(), + )); // Update the landing block's framestate / inputargs from the // edge state. Note: RPython models each @@ -3339,7 +3305,7 @@ fn attach_catch_exception_edge( // arity invariant below is satisfied either way because // `getoutputargs` walks `target_state.mergeable()` — the // same mergeable layout as `target.inputargs`. - update_catch_landing_state(graph, target, &edge_state); + update_catch_landing_state(graph, target, edge_state); // `model.py:114-116 Link.__init__` enforces // `len(args) == len(target.inputargs)`. Build `link.args` via @@ -3363,16 +3329,52 @@ fn attach_catch_exception_edge( // BOTH `link.args` (via `getoutputargs` at the // `last_exception` mergeable position) AND `link.extravars` // — matching `flowcontext.py:141-143`. - let (exc_type, exc_value) = exception_edge_extravars(&edge_state); + let (exc_type, exc_value) = exception_edge_extravars(edge_state); let mut link = super::flow::Link::new(link_args, Some(target.block()), None); link.extravars(Some(exc_type), Some(exc_value)); let link = link.into_ref(); - let _ = source_state; append_exit(block, link.clone()); target.add_incoming_exception_link(link.clone()); link } +fn attach_catch_exception_edge( + code: &CodeObject, + graph: &mut super::flow::FunctionGraph, + block: &super::flow::BlockRef, + target: &SpamBlockRef, + source_state: &FrameState, + site: &ExceptionCatchSite, +) -> super::flow::LinkRef { + // `flowcontext.py:130-134 guessexception` synthesises the + // `(last_exception, last_exc_value)` Variable pair for this + // edge. `exception_landing_state` clones `source_state` and + // sets `last_exception` to the fresh pair, so the same + // Variables can be threaded into BOTH `link.args` (via + // `getoutputargs` below) AND `link.extravars`. + // + // Reshape the cloned state to the handler-entry layout: unwind the + // operand stack to the handler's try-level `stack_depth`, push the + // `lasti` box (when flagged) and the exception value, and retarget + // `next_offset` to the handler PC — the exact transform + // `handler_entry_state_from_catch_site` applies when it builds the + // catch landing's framestate / inputargs. A `raise` reached from a + // DEEPER operand-stack PC (e.g. mid-expression, inside a call's + // argument build-up) otherwise leaves `edge_state` at the raise + // point's stack depth and `next_offset`, so it is NOT union- + // compatible with the landing (`FrameState::union` declines on the + // `next_offset` / stack-length mismatch and `update_catch_landing_ + // state` silently keeps the landing as-is). The positional + // `getoutputargs` then shifts every arg past the stack delta, + // pairing a Ref slot with the landing's `last_exception` Int slot — + // surfacing downstream as an `int_copy` kind-mismatch at assemble + // time and as an `enforce_input_args` colour collision when the + // shifted CFG coalesce pair merges two landing inputargs. + let seeded = exception_landing_state(graph, source_state); + let edge_state = handler_entry_state_from_catch_site(code, graph, &seeded, site); + attach_materialized_exception_edge(graph, block, target, &edge_state) +} + fn restore_canraise_exit_order(block: &super::flow::BlockRef) { let mut block_mut = block.borrow_mut(); if block_mut.exits.len() < 2 { @@ -3596,6 +3598,7 @@ struct FnPtrIndices { store_slice_fn: HelperHandle, get_iter_fn: HelperHandle, for_iter_next_fn: HelperHandle, + for_iter_exception_match_fn: HelperHandle, call_kw_fn_0: HelperHandle, call_kw_fn_1: HelperHandle, call_kw_fn_2: HelperHandle, @@ -4317,6 +4320,15 @@ fn register_helper_fn_pointers( cpu.clear_in_flight_exception_fn as *const (), CallFlavor::PlainCannotRaiseNoHeap, ); + // `check_exc_match_against` only reads the exception's Python class and + // class MRO. It cannot raise or run user code, but it does read GC heap + // state, so use `PlainCannotRaise` rather than the no-heap flavor. Bind it + // last so every pre-existing helper index remains stable. + let for_iter_exception_match_fn = bind( + assembler, + cpu.for_iter_exception_match_fn as *const (), + CallFlavor::PlainCannotRaise, + ); FnPtrIndices { call_fn, load_global_fn, @@ -4387,6 +4399,7 @@ fn register_helper_fn_pointers( unpack_ex_fn, get_iter_fn, for_iter_next_fn, + for_iter_exception_match_fn, unary_positive_fn, load_common_constant_fn, set_add_fn, @@ -6494,6 +6507,11 @@ impl CodeWriter { idx: for_iter_next_fn_idx, flavor: _for_iter_next_fn_flavor, }, + for_iter_exception_match_fn: + HelperHandle { + idx: for_iter_exception_match_fn_idx, + flavor: _for_iter_exception_match_fn_flavor, + }, call_kw_fn_0: HelperHandle { idx: call_kw_fn_0_idx, @@ -10229,10 +10247,12 @@ impl CodeWriter { ); // `eval.rs::push_exc_info` clears the propagation // carrier immediately after publishing `exc` as - // current. Preserve that ownership transfer in - // both tracing and compiled execution; otherwise - // the carrier keeps the handled exception and its - // traceback/frame alive indefinitely. + // current. Keep the operation explicit for the + // interpreter/blackhole path. The authoritative + // full-body walker applies the concrete clear but + // emits no runtime CallN: its compiled traceback + // IR never publishes this interpreter-only carrier + // (PyPy's local OperationError needs no such call). let _ = residual_call!( clear_in_flight_exception_fn_idx, CallFlavor::PlainCannotRaiseNoHeap, @@ -11101,33 +11121,183 @@ impl CodeWriter { let next_value = next_var .map(super::flow::FlowValue::from) .unwrap_or_else(|| fresh_ref_value(&mut graph)); - // `for_iter_next_fn` is `CallFlavor::MayForce`: a user - // `__next__` may raise a non-StopIteration exception. - // When the FOR_ITER sits inside a `try` range the - // residual's `GUARD_NO_EXCEPTION` needs a byte-adjacent - // `catch_exception/L`, else a real raise deopts and the - // blackhole's `handle_exception_in_frame` - // (`blackhole.py:396`) finds no catch and escapes the - // enclosing `try` (`ExitFrameWithExceptionRef`). The - // generic per-PC catch emission below is skipped here - // because the exhaustion branch closes this block - // first, so split off a dedicated residual block now: - // block A holds the call + the exception edge to the - // handler + a normal fallthrough to a fresh block B; - // the ptr_nonzero two-way exhaustion split then emits - // into B. Both blocks keep the orthodox single- - // bool-or-single-exception exit shape `flatten.py: - // 275-296 insert_switch_exits` requires. StopIteration - // still returns null and takes the exhaustion arm on B - // — the catch fires only on a non-null backend - // exception. - if let Some(catch_label) = catch_for_pc[py_pc] { - emit_catch_exception_and_split!( - catch_label, - py_pc, - [next_value.clone()] + // `pyopcode.py:1303-1316` catches the interpreter-level + // OperationError and then tests + // `e.match(space, space.w_StopIteration)`. Pyre's raised + // object is already the Python exception, so materialize + // the catch unconditionally and perform the Python-level + // MRO match in its landing block. The matched edge + // supplies NULL and rejoins the existing `ptr_nonzero` + // exhaustion split below. + let stop_match = SpamBlockRef::new(graph.new_block(Vec::new()), None); + all_walker_blocks.push(stop_match.clone()); + let stop_match_edge_state = + exception_landing_state(&mut graph, ¤t_state); + attach_materialized_exception_edge( + &mut graph, + ¤t_block.block(), + &stop_match, + &stop_match_edge_state, + ); + + // `guessexception` closes the can-raise block at the + // residual and puts the normal exhaustion branch in a + // fresh successor. Thread the residual result through + // that successor exactly like `split_block`; the matched + // handler predecessor substitutes NULL for it. + let next_result = next_value + .as_variable() + .expect("FOR_ITER residual result must be a Ref Variable"); + let mut next_state = current_state.clone(); + next_state.next_offset = py_pc; + next_state.blocklist = frame_blocks_for_offset(code, py_pc); + let next_block = SpamBlockRef::new( + graph.new_block(Vec::new()), + Some(next_state.clone()), + ); + all_walker_blocks.push(next_block.clone()); + let mut inputargs = next_state.getvariables(); + inputargs.push(next_result.into()); + next_block.block().borrow_mut().inputargs = inputargs.clone(); + append_exit( + ¤t_block.block(), + super::flow::Link::new(inputargs, Some(next_block.block()), None) + .into_ref(), + ); + restore_canraise_exit_order(¤t_block.block()); + + let stop_match_state = stop_match + .framestate() + .expect("FOR_ITER catch must have a FrameState"); + let (_, stop_exc_value) = exception_edge_extravars(&stop_match_state); + let stop_iteration = pyre_interpreter::builtins::lookup_exc_class( + "StopIteration", + ) + .expect("StopIteration class must be initialized before JIT codegen"); + let matched = record_residual_call_graph_op( + &mut graph, + &stop_match.block(), + for_iter_exception_match_fn_idx, + CallFlavor::PlainCannotRaise, + majit_ir::PyreHelperKind::ForIterExceptionMatch, + vec![], + vec![ + stop_exc_value.into(), + pyobject_const_ref_value(stop_iteration), + ], + vec![], + vec![Kind::Ref, Kind::Ref], + ResKind::Int, + py_pc as i64, + ) + .expect("FOR_ITER exception match must return an Int Variable"); + stop_match.block().borrow_mut().exitswitch = + Some(super::flow::ExitSwitch::Value(matched.into())); + + // False re-raises the value materialized at this + // landing, forwarded as an ordinary SSA value. A PC + // covered by a Python handler pairs the raise with a + // byte-adjacent `catch_exception`; an uncovered one + // leaves the frame through `exceptblock`. + let mismatch_link = if let Some(catch_label) = catch_for_pc[py_pc] { + let site = catch_sites + .iter() + .find(|site| site.landing_label == catch_label) + .expect("catch_sites entry for catch_label") + .clone(); + // `pyopcode.py:1310` re-raises the caught `e` + // when it is not a StopIteration, so the + // ordinary handler machinery delivers it. The + // match residual already drained the exception + // slot, and the catch landing materializes its + // exception from that slot, so the arm cannot + // jump into the landing directly. Raise the + // forwarded value from its own block and let + // the byte-adjacent `catch_exception` route it + // — the covered-raise shape of `emit_raise!`. + let mismatch_block = SpamBlockRef::new( + graph.new_block(Vec::new()), + Some(stop_match_state.clone()), ); + all_walker_blocks.push(mismatch_block.clone()); + let inputargs: Vec = + stop_match_state.getvariables(); + mismatch_block.block().borrow_mut().inputargs = inputargs.clone(); + let link = super::flow::Link::new( + inputargs, + Some(mismatch_block.block()), + None, + ) + .into_ref(); + append_exit(&stop_match.block(), link.clone()); + + let (_, exc_value) = exception_edge_extravars(&stop_match_state); + record_graph_op( + &mismatch_block.block(), + "raise", + vec![exc_value.into()], + None, + py_pc as i64, + ); + attach_catch_exception_edge( + code, + &mut graph, + &mismatch_block.block(), + &site.landing, + &stop_match_state, + &site, + ); + link + } else { + let (exc_type, exc_value) = + exception_edge_extravars(&stop_match_state); + let link = super::flow::Link::new( + vec![exc_type.into(), exc_value.into()], + Some(graph.exceptblock.clone()), + None, + ) + .into_ref(); + append_exit(&stop_match.block(), link.clone()); + link + }; + { + let false_case: super::flow::FlowValue = + super::flow::Constant::bool(false).into(); + let mut link = mismatch_link.borrow_mut(); + link.exitcase = Some(false_case.clone()); + link.llexitcase = Some(false_case); } + + // True consumes StopIteration and converges on the + // existing exhaustion split with a NULL next result. + // + // The last two `mergeable()` entries are the + // `last_exception` pair, so taking them from the + // catch state would hand the successor the + // StopIteration this edge just consumed. A + // FOR_ITER inside an `except` body runs with the + // handled exception live in that pair, and the + // bare-`raise` lowering re-raises it directly when + // the PC is not itself catch-covered. Carry the + // enclosing pair across instead: after exhaustion + // the frame's exception state is what it was before + // the loop. + let mut matched_source = stop_match_state.clone(); + matched_source + .last_exception + .clone_from(¤t_state.last_exception); + let mut stop_match_args = matched_source.getoutputargs(¤t_state); + stop_match_args.push(null_stack_sentinel()); + let matched_link = super::flow::Link::new( + stop_match_args, + Some(next_block.block()), + None, + ) + .with_exitcase(super::flow::Constant::bool(true).into()) + .with_llexitcase(super::flow::Constant::bool(true).into()) + .into_ref(); + append_exit(&stop_match.block(), matched_link); + current_block = next_block; // Emit the exhaustion branch: ptr_nonzero(next) // selects between the continue arm (non-null → // push next, fall to PC+1) and the exhaustion arm @@ -16212,6 +16382,145 @@ mod tests { .expect("expected nested function code object") } + #[test] + fn for_iter_jitcode_emits_stop_iteration_catch_arm() { + let code = first_nested_function_code( + "def f(it):\n count = 0\n for _ in it:\n count += 1\n return count\n", + ); + let w_code = pyre_interpreter::box_code_constant(&code); + let code_ptr = unsafe { + pyre_interpreter::w_code_get_ptr(w_code) as *const pyre_interpreter::CodeObject + }; + let writer = CodeWriter::new(); + writer.setup_jitdriver(crate::jit::call::JitDriverStaticData { + portal_graph: code_ptr, + mainjitcode: None, + }); + writer.make_jitcodes(); + + let pyjit = writer + .callcontrol() + .find_compiled_jitcode_arc(code_ptr) + .expect("FOR_ITER portal must produce a jitcode"); + let ops: Vec<_> = + pyre_jit_trace::jitcode_runtime::decoded_ops(&pyjit.jitcode.code).collect(); + let catch_positions: Vec<_> = ops + .iter() + .enumerate() + .filter_map(|(index, op)| (op.key == "catch_exception/L").then_some(index)) + .collect(); + assert_eq!( + catch_positions.len(), + 1, + "a loop outside a Python try has exactly the internal FOR_ITER catch" + ); + let catch_index = catch_positions[0]; + assert_eq!(ops[catch_index - 1].key, "live/"); + assert!( + ops[catch_index - 2].key.starts_with("residual_call_"), + "catch_exception must be adjacent to the FOR_ITER residual call" + ); + + let ptr_nonzero_index = ops + .iter() + .position(|op| op.key == "ptr_nonzero/r>i") + .expect("FOR_ITER must retain its existing exhaustion split"); + let last_exc_value_index = ops + .iter() + .position(|op| op.key == "last_exc_value/>r") + .expect("FOR_ITER catch must materialize the caught exception value"); + let match_call_index = ops + .iter() + .enumerate() + .skip(last_exc_value_index + 1) + .find_map(|(index, op)| { + (op.key.starts_with("residual_call_") && op.key.ends_with(">i")).then_some(index) + }) + .expect("FOR_ITER catch must call the Python-level exception matcher"); + let bool_split_index = ops + .iter() + .enumerate() + .skip(match_call_index + 1) + .find_map(|(index, op)| (op.key == "goto_if_not/iL").then_some(index)) + .expect("FOR_ITER exception match must feed an ordinary bool split"); + let raise_index = ops + .iter() + .position(|op| op.key == "raise/r") + .expect("a FOR_ITER outside a Python try must raise mismatches"); + assert!(catch_index < ptr_nonzero_index); + assert!(ptr_nonzero_index < last_exc_value_index); + assert!(last_exc_value_index < match_call_index); + assert!(match_call_index < bool_split_index); + assert!(bool_split_index < raise_index); + assert!( + !ops[match_call_index + 1..] + .iter() + .any(|op| matches!(op.key, "last_exception/>i" | "last_exc_value/>r")), + "the match residual clears the active exception, so its boolean arms must forward the materialized pair" + ); + assert!( + !ops.iter() + .any(|op| op.key == "goto_if_exception_mismatch/iL"), + "FOR_ITER must not use the RPython-class typed catch opcode" + ); + } + + #[test] + fn for_iter_inside_try_keeps_bool_landing_exitswitch() { + let code = first_nested_function_code( + "def f(n):\n total = 0\n try:\n for value in range(n):\n total += value\n except ValueError:\n return -1\n return total\n", + ); + let w_code = pyre_interpreter::box_code_constant(&code); + let code_ptr = unsafe { + pyre_interpreter::w_code_get_ptr(w_code) as *const pyre_interpreter::CodeObject + }; + let writer = CodeWriter::new(); + writer.setup_jitdriver(crate::jit::call::JitDriverStaticData { + portal_graph: code_ptr, + mainjitcode: None, + }); + writer.make_jitcodes(); + + let pyjit = writer + .callcontrol() + .find_compiled_jitcode_arc(code_ptr) + .expect("FOR_ITER inside a try range must produce a jitcode"); + let ops: Vec<_> = + pyre_jit_trace::jitcode_runtime::decoded_ops(&pyjit.jitcode.code).collect(); + assert!( + ops.iter().any(|op| op.key == "goto_if_not/iL"), + "FOR_ITER's exception-match landing must retain its bool exitswitch" + ); + + // The mismatch arm re-raises the forwarded value and pairs the raise + // with a byte-adjacent `catch_exception` so the handler landing is + // entered with the exception slot refilled. Jumping into the landing + // instead reaches its `last_exc_value` read with the slot already + // drained by the match residual. + let raise_index = ops + .iter() + .position(|op| op.key == "raise/r") + .expect("a mismatched FOR_ITER exception inside a try must re-raise"); + assert_eq!( + ops[raise_index + 1].key, + "catch_exception/L", + "the re-raise must carry the catch dispatch its handler entry needs" + ); + let match_call_index = ops + .iter() + .enumerate() + .find_map(|(index, op)| { + (op.key.starts_with("residual_call_") && op.key.ends_with(">i")).then_some(index) + }) + .expect("FOR_ITER catch must call the Python-level exception matcher"); + assert!( + !ops[match_call_index + 1..raise_index] + .iter() + .any(|op| matches!(op.key, "last_exception/>i" | "last_exc_value/>r")), + "the match residual drains the exception slot, so its arms must not re-read it" + ); + } + // Minimal `ExceptionCatchSite` for the `attach_catch_exception_edge` // tests: a try-level stack depth of 0 with no `lasti` push and a // handler PC at offset 0. `handler_entry_state_from_catch_site` diff --git a/pyre/pyre-jit/src/jit/cpu.rs b/pyre/pyre-jit/src/jit/cpu.rs index 2eb6dfaee53..791736eaf12 100644 --- a/pyre/pyre-jit/src/jit/cpu.rs +++ b/pyre/pyre-jit/src/jit/cpu.rs @@ -256,6 +256,9 @@ pub struct Cpu { /// GuardNonnull catches it), or publishes a real exception into the /// backend exception cells on error. pub for_iter_next_fn: extern "C" fn(i64) -> i64, + /// MRO-aware Python exception match used by the FOR_ITER catch arm. + /// Returns a raw bool-as-int and cannot raise. + pub for_iter_exception_match_fn: extern "C" fn(i64, i64) -> i64, /// `bh_unary_negative_fn(value)` — UNARY_NEGATIVE `-value` residual /// (a user `__neg__` may run Python → fallible). pub unary_negative_fn: extern "C" fn(i64) -> i64, @@ -515,6 +518,7 @@ impl Cpu { set_function_attribute_fn: pyre_interpreter::runtime_ops::jit_set_function_attribute, get_iter_fn: crate::call_jit::bh_get_iter_fn, for_iter_next_fn: pyre_interpreter::runtime_ops::jit_next, + for_iter_exception_match_fn: pyre_interpreter::runtime_ops::jit_exception_match, unary_negative_fn: crate::call_jit::bh_unary_negative_fn, unary_invert_fn: crate::call_jit::bh_unary_invert_fn, unary_positive_fn: crate::call_jit::bh_unary_positive_fn, diff --git a/pyre/pyre-object/src/interp_exceptions.rs b/pyre/pyre-object/src/interp_exceptions.rs index 5a47cb2f693..c278dd71944 100644 --- a/pyre/pyre-object/src/interp_exceptions.rs +++ b/pyre/pyre-object/src/interp_exceptions.rs @@ -237,53 +237,6 @@ impl ExcKind { /// value whose provenance is not proven must go through /// `w_exception_kind_checked`, which range-checks against this. pub const MAX_DISCRIMINANT: u8 = ExcKind::EOFError as u8; - - /// True when this kind's constructor is the trivial - /// `W_BaseException.descr_init` (`self.args_w = args_w`) — i.e. it - /// stores nothing beyond `args_w`. - /// - /// False for the kinds whose `descr_init` parses arguments and stores - /// extra flattened fields (and, for `OSError`, rewrites `args_w`): - /// `OSError` / `FileNotFoundError` set `errno` / `strerror` / - /// `filename` / `filename2` (`builtins.rs::os_error_init`, - /// interp_exceptions.py:552/629); `UnicodeDecodeError` / - /// `UnicodeEncodeError` / `UnicodeTranslateError` set `w_object` / - /// `start` / `end` / `reason` (and `encoding` for the codec errors) - /// (`builtins.rs::exc_unicode_*_error_init`, - /// interp_exceptions.py:433/1041/1159); `SyntaxError` sets `msg` / - /// `filename` / `lineno` / `offset` / `text` / `end_lineno` / - /// `end_offset` (interp_exceptions.py:836); `StopIteration` sets - /// `value` (:496); `AttributeError` sets `name` / `obj` (:1134); - /// `NameError` sets `name` (:810); `SystemExit` sets `code` (:993); - /// `ImportError` / `ModuleNotFoundError` set `name` / `path` / - /// `name_from` (:363). - /// - /// The subclasses that inherit one of these initializers share their - /// parent's kind — `UnboundLocalError` is a `NameError`, - /// `IndentationError` and `TabError` are `SyntaxError`s — so naming the - /// parent covers them. - /// - /// A caller that reconstructs an exception from only - /// `kind` / `w_class` / `args_w` (e.g. the traced inline - /// constructor) must reject the non-trivial kinds and defer to the - /// full runtime constructor, which initializes those fields. - pub fn has_trivial_args_constructor(self) -> bool { - !matches!( - self, - ExcKind::OSError - | ExcKind::FileNotFoundError - | ExcKind::UnicodeDecodeError - | ExcKind::UnicodeEncodeError - | ExcKind::UnicodeTranslateError - | ExcKind::SyntaxError - | ExcKind::StopIteration - | ExcKind::AttributeError - | ExcKind::NameError - | ExcKind::SystemExit - | ExcKind::ImportError - | ExcKind::ModuleNotFoundError - ) - } } /// Layout: `[ob_header | kind: ExcKind | args_w: PyObjectRef | …]`