diff --git a/majit/majit-metainterp/src/pyjitpl.rs b/majit/majit-metainterp/src/pyjitpl.rs index 5a729f7cd5c..3fb5c9b4398 100644 --- a/majit/majit-metainterp/src/pyjitpl.rs +++ b/majit/majit-metainterp/src/pyjitpl.rs @@ -5204,18 +5204,22 @@ impl MetaInterp { let outermost_merge_key = ctx.current_merge_points_first_greenkey(); // pyjitpl.py:2793: find_biggest_function — if an inlined function // caused the bloat, disable just that function. - let huge_fn_key = self.find_biggest_function(); + let huge_fn = self.find_biggest_function(); // pyjitpl.py:2795: `self.portal_trace_positions = None` marks the // abort boundary so post-abort consumers (e.g. test inspections // at pyjitpl.py:3547) can detect a terminated trace session. self.portal_trace_positions = None; - if let Some(huge_fn_key) = huge_fn_key { + if let Some((huge_fn_jd_no, huge_fn_key)) = huge_fn { + // pyjitpl.py:2822 disables through `jd_sd.warmstate`, the warmstate + // of the driver that owns the oversized frame. Pyre keeps one + // `WarmEnterState` on the MetaInterp rather than one per + // `JitDriverStaticData`, so the disable lands on that single state; + // the owning index is still carried through below. self.warm_state.disable_noninlinable_function(huge_fn_key); // pyjitpl.py:2799-2800: stash the aborted jd_sd + greenkey so // `aborted_tracing(reason)` can fire `on_trace_too_long` when - // the hook is ported. Pyre only registers a single jitdriver - // so jd_sd.index is always 0. - self.aborted_tracing_jitdriver = Some(0); + // the hook is ported. + self.aborted_tracing_jitdriver = Some(huge_fn_jd_no); self.aborted_tracing_greenkey = Some(huge_fn_key); // pyjitpl.py:2801-2804: only boost retrace for the outermost // loop (when `current_merge_points` is non-empty). Bridge @@ -14208,41 +14212,50 @@ impl MetaInterp { /// `size` is the distance between two `TracePosition::_pos` cursors, the /// `pos[0]` upstream subtracts (opencoder.py:475). /// - /// Returns the green key of the largest frame, or `None` when the log + /// Returns the owning jitdriver's index and the green key of the largest + /// frame — `max_jdsd, max_key` upstream, which the caller needs both of + /// (`pyjitpl.py:2821-2824` disables through the frame's OWN driver and + /// stashes that driver in `aborted_tracing_jitdriver`). `None` when the log /// holds no closed or open portal frame — the root frame is created by /// `initialize_state_from_start` without a greenkey and never enters the /// log, so a trace that inlined nothing answers `None` and the caller /// falls through to `prepare_trace_segmenting`. - pub fn find_biggest_function(&self) -> Option { + pub fn find_biggest_function(&self) -> Option<(usize, u64)> { let positions = self.portal_trace_positions.as_ref()?; - let mut start_stack: Vec<(u64, usize)> = Vec::new(); + let mut start_stack: Vec<(usize, u64, usize)> = Vec::new(); let mut max_size = 0usize; let mut max_key = None; - for &(_jd_no, key, pos) in positions { + for &(jd_no, key, pos) in positions { match key { // pyjitpl.py:3547-3548 `if key is not None: start_stack.append`. - Some(key) => start_stack.push((key, pos._pos)), + Some(key) => start_stack.push((jd_no, key, pos._pos)), // pyjitpl.py:3549-3559 the closing entry sizes the frame it // closes. An unmatched close cannot happen while `newframe` / // `popframe` are the only writers, so it is left to `pop`'s // `None` rather than given a recovery path. None => { - if let Some((green_key, start_pos)) = start_stack.pop() { + if let Some((jd_no, green_key, start_pos)) = start_stack.pop() { let size = pos._pos.saturating_sub(start_pos); if size > max_size { max_size = size; - max_key = Some(green_key); + max_key = Some((jd_no, green_key)); } } } } } // pyjitpl.py:3560-3570 `if start_stack:` — one frame, the outermost, - // measured against where the trace stopped. - if let Some(&(green_key, start_pos)) = start_stack.first() { - let current = self.tracing.as_ref()?.get_trace_position()._pos; + // measured against where the trace stopped. Upstream reads + // `self.history` there unconditionally; pyre's recorder is an `Option`, + // and a `?` on it would return `None` for the whole function and throw + // away a `max_key` the closed frames above already produced. Only the + // open frame is unmeasurable without a recorder, so only it is skipped. + if let Some(&(jd_no, green_key, start_pos)) = start_stack.first() + && let Some(tracing) = self.tracing.as_ref() + { + let current = tracing.get_trace_position()._pos; if current.saturating_sub(start_pos) > max_size { - max_key = Some(green_key); + max_key = Some((jd_no, green_key)); } } max_key @@ -20478,7 +20491,7 @@ mod metainterp_static_data_tests { assert_eq!( meta.find_biggest_function(), - Some(0xa11), + Some((0, 0xa11)), "the larger frame wins even though both have returned" ); } @@ -20498,7 +20511,32 @@ mod metainterp_static_data_tests { meta.perform_call(jc, &[], Some(0xb22)).unwrap_err(); record_ops(&mut meta, 5); - assert_eq!(meta.find_biggest_function(), Some(0xb22)); + assert_eq!(meta.find_biggest_function(), Some((0, 0xb22))); + } + + #[test] + fn find_biggest_function_keeps_a_closed_frame_when_the_recorder_is_gone() { + // pyjitpl.py:3560-3570 reads `self.history` unconditionally, so a + // closed frame's size always survives to the return. pyre's recorder is + // an `Option`: an unmatched open entry plus `tracing = None` must skip + // only the open frame's measurement, not discard `max_key`. + let (mut meta, jc) = meta_with_recursive_portal(); + start_tracing(&mut meta); + + meta.perform_call(jc.clone(), &[], Some(0xa11)).unwrap_err(); + record_ops(&mut meta, 5); + meta.popframe(true); + + meta.perform_call(jc, &[], Some(0xb22)).unwrap_err(); + record_ops(&mut meta, 1); + // Left open, and the recorder retired under it. + meta.tracing = None; + + assert_eq!( + meta.find_biggest_function(), + Some((0, 0xa11)), + "the closed frame's size survives a missing recorder" + ); } #[test] diff --git a/pyre/bench/synth/binary_int_overflow_local_resume.cranelift.jitstats b/pyre/bench/synth/binary_int_overflow_local_resume.cranelift.jitstats index 9df977d4495..2a8ae763685 100644 --- a/pyre/bench/synth/binary_int_overflow_local_resume.cranelift.jitstats +++ b/pyre/bench/synth/binary_int_overflow_local_resume.cranelift.jitstats @@ -2,7 +2,10 @@ bridges_compiled=5 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 guard_failures=647 diff --git a/pyre/bench/synth/binary_int_overflow_local_resume.dynasm.jitstats b/pyre/bench/synth/binary_int_overflow_local_resume.dynasm.jitstats index 9df977d4495..2a8ae763685 100644 --- a/pyre/bench/synth/binary_int_overflow_local_resume.dynasm.jitstats +++ b/pyre/bench/synth/binary_int_overflow_local_resume.dynasm.jitstats @@ -2,7 +2,10 @@ bridges_compiled=5 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 guard_failures=647 diff --git a/pyre/bench/synth/binary_int_overflow_local_resume.wasm.jitstats b/pyre/bench/synth/binary_int_overflow_local_resume.wasm.jitstats index 9df977d4495..2a8ae763685 100644 --- a/pyre/bench/synth/binary_int_overflow_local_resume.wasm.jitstats +++ b/pyre/bench/synth/binary_int_overflow_local_resume.wasm.jitstats @@ -2,7 +2,10 @@ bridges_compiled=5 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 guard_failures=647 diff --git a/pyre/bench/synth/blackhole_inlined_callee_local_after_escape_declined.py b/pyre/bench/synth/blackhole_inlined_callee_local_after_escape_declined.py index 0e5096d0a1a..917737bcd8d 100644 --- a/pyre/bench/synth/blackhole_inlined_callee_local_after_escape_declined.py +++ b/pyre/bench/synth/blackhole_inlined_callee_local_after_escape_declined.py @@ -16,9 +16,10 @@ # # Guard for what an adopted multi-frame blackhole chain owes its inner levels. # -# An inlined callee assigns a local, the frame then escapes through a residual -# `sys._getframe()`, and an attribute read POSITIONED AFTER that escape reads the -# local back. The read is executed by the blackhole, not by the walk, so the +# An inlined callee assigns a local, the frame then escapes through the +# `.f_locals` read added above -- `sys._getframe()` itself folds here -- and an +# attribute read POSITIONED AFTER that escape reads the local back. The read is +# executed by the blackhole, not by the walk, so the # shape holds the adopt to two separate obligations and fails differently on # each: # diff --git a/pyre/bench/synth/exc_bridge_entry_guard_not_removed.cranelift.jitstats b/pyre/bench/synth/exc_bridge_entry_guard_not_removed.cranelift.jitstats index 32fba6ef986..127a61f9c86 100644 --- a/pyre/bench/synth/exc_bridge_entry_guard_not_removed.cranelift.jitstats +++ b/pyre/bench/synth/exc_bridge_entry_guard_not_removed.cranelift.jitstats @@ -2,7 +2,10 @@ bridges_compiled=4 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 guard_failures=809 diff --git a/pyre/bench/synth/exc_bridge_entry_guard_not_removed.dynasm.jitstats b/pyre/bench/synth/exc_bridge_entry_guard_not_removed.dynasm.jitstats index 32fba6ef986..127a61f9c86 100644 --- a/pyre/bench/synth/exc_bridge_entry_guard_not_removed.dynasm.jitstats +++ b/pyre/bench/synth/exc_bridge_entry_guard_not_removed.dynasm.jitstats @@ -2,7 +2,10 @@ bridges_compiled=4 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 guard_failures=809 diff --git a/pyre/bench/synth/exc_bridge_entry_guard_not_removed.wasm.jitstats b/pyre/bench/synth/exc_bridge_entry_guard_not_removed.wasm.jitstats index 32fba6ef986..127a61f9c86 100644 --- a/pyre/bench/synth/exc_bridge_entry_guard_not_removed.wasm.jitstats +++ b/pyre/bench/synth/exc_bridge_entry_guard_not_removed.wasm.jitstats @@ -2,7 +2,10 @@ bridges_compiled=4 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 guard_failures=809 diff --git a/pyre/bench/synth/getframe_caller_resume_coord_two_call_sites.cranelift.jitstats b/pyre/bench/synth/getframe_caller_resume_coord_two_call_sites.cranelift.jitstats new file mode 100644 index 00000000000..8555152ba84 --- /dev/null +++ b/pyre/bench/synth/getframe_caller_resume_coord_two_call_sites.cranelift.jitstats @@ -0,0 +1,14 @@ +bridges_compiled=0 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=5 +fbw_blackhole_adopted_single_frame=5 +fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 +field_pos_attached_misplaced=0 +field_pos_spec_misplaced=0 +guard_failures=0 +internal_compile_panics=0 +loops_aborted=10 +loops_compiled=0 diff --git a/pyre/bench/synth/getframe_caller_resume_coord_two_call_sites.dynasm.jitstats b/pyre/bench/synth/getframe_caller_resume_coord_two_call_sites.dynasm.jitstats new file mode 100644 index 00000000000..8555152ba84 --- /dev/null +++ b/pyre/bench/synth/getframe_caller_resume_coord_two_call_sites.dynasm.jitstats @@ -0,0 +1,14 @@ +bridges_compiled=0 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=5 +fbw_blackhole_adopted_single_frame=5 +fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 +field_pos_attached_misplaced=0 +field_pos_spec_misplaced=0 +guard_failures=0 +internal_compile_panics=0 +loops_aborted=10 +loops_compiled=0 diff --git a/pyre/bench/synth/getframe_caller_resume_coord_two_call_sites.py b/pyre/bench/synth/getframe_caller_resume_coord_two_call_sites.py new file mode 100644 index 00000000000..79265acc8d8 --- /dev/null +++ b/pyre/bench/synth/getframe_caller_resume_coord_two_call_sites.py @@ -0,0 +1,66 @@ +# A callee reads its CALLER's resume coordinate -- `f_lineno` and `f_lasti` -- +# from two DIFFERENT call sites in the same hot loop. +# +# `f_lineno` and `f_lasti` both resolve off the frame's `last_instr`, which +# compiled code does not store per opcode, so the value only reaches the frame +# if the force publishes it. A single call site makes that unobservable: the +# caller's coordinate is then constant by construction and a frozen read is +# indistinguishable from a live one. Calling from two lines makes the correct +# answer ALTERNATE, so a frame left holding one leg's coordinate collapses the +# two rows into one. +# +# The staleness this pins runs in the other direction too: a publish that is +# withdrawn too early -- put back at the residual's return instead of at walk +# end -- leaves the caller answering for the coordinate it held BEFORE the +# call, and the shape set gains an extra row rather than losing one. Surveying +# every iteration into a set is what catches both, because the pre-compile +# iterations are correct and a miss appears as a changed row count. +# +# Measured, by putting that defect back in: dropping the `flushed` test from +# `LiveLastInstrGuard::drop`, so the guard always restores at the residual's +# return, prints +# ([(0, 3), (0, 8), (1, 3), (1, 6)], [0, 0, 1, 1], 3) +# against this fixture's +# ([(0, 8), (1, 6)], [0, 1], 2) +# -- the pre-call coordinate appears alongside the call-site one on BOTH legs, +# and the offsets' discrimination count rises with it. Nothing else in +# bench/synth reads `f_lasti` at all, and only the traceback-frame fixture +# reads an `f_lineno`. +# +# `f_lasti` is a bytecode offset, so its absolute value is a property of the +# compiler and cannot be compared against the pypy oracle. Only its +# DISCRIMINATION is portable: the two call sites must report two distinct +# offsets, one per leg. `f_lineno` is a source coordinate and is compared +# directly, relative to `co_firstlineno` so edits above these functions do not +# move the expected values. +# +# Neither read goes through `.f_locals`: that getset forces the frame on its +# own, and a forcing read would mask exactly the staleness under test. +import sys + +N = 30000 + + +def inner(): + f = sys._getframe(1) + return (f.f_lineno - f.f_code.co_firstlineno, f.f_lasti) + + +def outer(n): + lines = set() + offsets = set() + i = 0 + while i < n: + if i & 1: + line, off = inner() + else: + line, off = inner() + lines.add((i & 1, line)) + offsets.add((i & 1, off)) + i = i + 1 + legs = sorted(leg for leg, _ in offsets) + distinct = len({off for _, off in offsets}) + return (sorted(lines), legs, distinct) + + +print(outer(N)) diff --git a/pyre/bench/synth/getframe_caller_resume_coord_two_call_sites.wasm.jitstats b/pyre/bench/synth/getframe_caller_resume_coord_two_call_sites.wasm.jitstats new file mode 100644 index 00000000000..8555152ba84 --- /dev/null +++ b/pyre/bench/synth/getframe_caller_resume_coord_two_call_sites.wasm.jitstats @@ -0,0 +1,14 @@ +bridges_compiled=0 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=5 +fbw_blackhole_adopted_single_frame=5 +fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 +field_pos_attached_misplaced=0 +field_pos_spec_misplaced=0 +guard_failures=0 +internal_compile_panics=0 +loops_aborted=10 +loops_compiled=0 diff --git a/pyre/bench/synth/handler_tb_frame_locals_after_declined_flush.cranelift.jitstats b/pyre/bench/synth/handler_tb_frame_locals_after_declined_flush.cranelift.jitstats new file mode 100644 index 00000000000..77817899ecb --- /dev/null +++ b/pyre/bench/synth/handler_tb_frame_locals_after_declined_flush.cranelift.jitstats @@ -0,0 +1,14 @@ +bridges_compiled=0 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=5 +fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 +field_pos_attached_misplaced=0 +field_pos_spec_misplaced=0 +guard_failures=0 +internal_compile_panics=0 +loops_aborted=5 +loops_compiled=0 diff --git a/pyre/bench/synth/handler_tb_frame_locals_after_declined_flush.dynasm.jitstats b/pyre/bench/synth/handler_tb_frame_locals_after_declined_flush.dynasm.jitstats new file mode 100644 index 00000000000..77817899ecb --- /dev/null +++ b/pyre/bench/synth/handler_tb_frame_locals_after_declined_flush.dynasm.jitstats @@ -0,0 +1,14 @@ +bridges_compiled=0 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=5 +fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 +field_pos_attached_misplaced=0 +field_pos_spec_misplaced=0 +guard_failures=0 +internal_compile_panics=0 +loops_aborted=5 +loops_compiled=0 diff --git a/pyre/bench/synth/handler_tb_frame_locals_after_declined_flush.py b/pyre/bench/synth/handler_tb_frame_locals_after_declined_flush.py new file mode 100644 index 00000000000..f17c10a118a --- /dev/null +++ b/pyre/bench/synth/handler_tb_frame_locals_after_declined_flush.py @@ -0,0 +1,40 @@ +# A forced frame read from INSIDE a Python expression, so the merge-point +# escape flush declines and only the locals region is written. +# +# `'i' in tb.tb_frame.f_locals` forces the frame while the operand stack holds +# `[seen, add, ]`. A mid-expression stack slot reads NULL +# from the virtualizable shadow, so `flush_walk_end_state_to_frame_inner` +# declines the whole write and `flush_locals_region_to_frame` writes slots +# `0..nlocals` on their own -- an unforced array would render `f_locals` as an +# EMPTY mapping, a wrong answer rather than a stale one. +# +# That leg claims no resume pc, so nothing sets `COMMITTED_FRAME_ESCAPE_PC` and +# the walk-end block gated on it never runs. Its deferred undo restore has to +# be armed at the residual instead: `LiveLastInstrGuard` declines to put +# `last_instr` back while an undo capture is live, so without the arming the +# frame keeps the EXECUTING pc over an operand stack no flush ever wrote, and +# the replay re-enters one opcode late on an empty stack -- `value-stack +# underflow: depth=N base=N`, a JIT-only panic with no output at all. +# +# The handler is what puts the force inside an expression whose stack is deep +# enough to notice: the `seen.add(...)` receiver and its bound method are both +# live below the value being computed. +import sys + +N = 30000 + + +def run(): + seen = set() + i = 0 + while i < N: + try: + raise ValueError(i) + except ValueError: + tb = sys.exc_info()[2] + seen.add('i' in tb.tb_frame.f_locals) + i = i + 1 + return sorted(seen) + + +print(run()) diff --git a/pyre/bench/synth/handler_tb_frame_locals_after_declined_flush.wasm.jitstats b/pyre/bench/synth/handler_tb_frame_locals_after_declined_flush.wasm.jitstats new file mode 100644 index 00000000000..77817899ecb --- /dev/null +++ b/pyre/bench/synth/handler_tb_frame_locals_after_declined_flush.wasm.jitstats @@ -0,0 +1,14 @@ +bridges_compiled=0 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=5 +fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 +field_pos_attached_misplaced=0 +field_pos_spec_misplaced=0 +guard_failures=0 +internal_compile_panics=0 +loops_aborted=5 +loops_compiled=0 diff --git a/pyre/bench/synth/list_append_write_barrier_gc.cranelift.jitstats b/pyre/bench/synth/list_append_write_barrier_gc.cranelift.jitstats index b0b125c96eb..144b9884a47 100644 --- a/pyre/bench/synth/list_append_write_barrier_gc.cranelift.jitstats +++ b/pyre/bench/synth/list_append_write_barrier_gc.cranelift.jitstats @@ -2,7 +2,10 @@ bridges_compiled=5 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 guard_failures=1345 diff --git a/pyre/bench/synth/list_append_write_barrier_gc.dynasm.jitstats b/pyre/bench/synth/list_append_write_barrier_gc.dynasm.jitstats index b0b125c96eb..144b9884a47 100644 --- a/pyre/bench/synth/list_append_write_barrier_gc.dynasm.jitstats +++ b/pyre/bench/synth/list_append_write_barrier_gc.dynasm.jitstats @@ -2,7 +2,10 @@ bridges_compiled=5 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 guard_failures=1345 diff --git a/pyre/bench/synth/list_append_write_barrier_gc.wasm.jitstats b/pyre/bench/synth/list_append_write_barrier_gc.wasm.jitstats index b0b125c96eb..144b9884a47 100644 --- a/pyre/bench/synth/list_append_write_barrier_gc.wasm.jitstats +++ b/pyre/bench/synth/list_append_write_barrier_gc.wasm.jitstats @@ -2,7 +2,10 @@ bridges_compiled=5 descr_set_absent=0 descr_set_ambiguous=0 descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 guard_failures=1345 diff --git a/pyre/check.py b/pyre/check.py index b32a5051815..71a0c99f503 100644 --- a/pyre/check.py +++ b/pyre/check.py @@ -1660,6 +1660,22 @@ def _apply_snapshot_gate( # ── build ── def build_backend(self, backend): + # `pyre-jit-trace/build.rs` compares each `build/llbc/*.ullbc` against + # what its crate's sources hash to now, and by default reports a + # mismatch as a `cargo::warning` — which cargo replays only when it + # re-runs the build script, so a run whose crates were cached prints + # nothing at all. Every number this script produces is read out of a + # binary whose field offsets come from those artefacts, so a stale one + # does not fail: it answers, wrongly and quietly. Four measurement runs + # on this tree carried the mismatch and none of their logs named it. + # + # `PYRE_LLBC_STRICT=1` is the promotion build.rs already documents "for + # callers that want a gate" — the same finding as `cargo::error`. The + # cost is that a rebase which moves the LLBC crates makes the next + # check.py stop and ask for a multi-minute re-extraction; the + # alternative is a green run that measured the wrong bytes. Set here + # rather than per-command so the wasm build gets it too. + os.environ["PYRE_LLBC_STRICT"] = "1" cfg = CARGO_CONFIG[backend] if cfg.get("wasm"): return self.build_wasm_backend() @@ -1682,6 +1698,17 @@ def build_backend(self, backend): print(proc.stderr.rstrip()) print("────────────────────") cargo_output = (proc.stderr or "") + (proc.stdout or "") + if "LLBC STALE" in cargo_output: + # `PYRE_LLBC_STRICT=1` above turned build.rs's staleness + # warning into the build failure that got us here. It already + # printed the exact `extract-llbc.py` line naming the crates + # that moved, so repeat the reason rather than the command. + print(red("LLBC artefacts under build/llbc/ are STALE.")) + print("Field offsets come from them, so a run on this tree " + "would measure the wrong bytes.") + print("Re-extract with the command build.rs printed above, " + "then re-run this script.") + sys.exit(1) llbc_missing = ( # translator runtime panic (majit-translate/src/lib.rs) "no LLBC source resolved" in cargo_output diff --git a/pyre/pyre-interpreter/src/module/sys/vm.rs b/pyre/pyre-interpreter/src/module/sys/vm.rs index 057af531fd7..ed3394c9238 100644 --- a/pyre/pyre-interpreter/src/module/sys/vm.rs +++ b/pyre/pyre-interpreter/src/module/sys/vm.rs @@ -700,26 +700,6 @@ fn sys_getframe(args: &[PyObjectRef]) -> crate::PyResult { getframe(depth) } -/// `vm.py:54 f.mark_as_escaped()` as one non-forcing call, for the walker's -/// constant-depth [`getframe`] arm. -/// -/// Upstream's traced-through `getframe` emits it as `setfield_gc(p0, 1, -/// inst_escaped)`; `escaped` is a plain field, not one of the six -/// `interp_jit.py:25-30` declares, so writing it neither reads nor materialises -/// the virtualizable. The frame reaches this helper only to address the flag -/// byte — nothing under it can call -/// [`crate::executioncontext::force_frame`], which is what keeps the arm's -/// whole point (no residual force) intact. -/// -/// Emitted as a void `CallN`, matching the upstream `setfield_gc`'s lack of a -/// result: the store is the whole point, so nothing may drop it as dead. -pub extern "C" fn jit_frame_mark_as_escaped(frame: i64) { - let f = frame as *mut crate::PyFrame; - if !f.is_null() { - unsafe { (*f).mark_as_escaped() }; - } -} - /// True iff `callable` is the canonical `sys._getframe` builtin. /// /// `sys` is an ordinary mutable module, so the JIT walker has to key on 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 27beb1d35d5..d43f2a970b2 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs @@ -1351,7 +1351,23 @@ pub fn flush_active_frame_escape(ctx: &TraceCtx, frame: *mut pyre_interpreter::P if let Some(py_pc) = portal_py_pc { COMMITTED_FRAME_ESCAPE_PC.with(|committed| committed.set(Some((py_pc, kind)))); } - } else if !crate::state::flush_locals_region_to_frame(ctx, expected) { + } else if crate::state::flush_locals_region_to_frame(ctx, expected) { + // The locals write landed but claimed no resume pc, so nothing + // will consume `COMMITTED_FRAME_ESCAPE_PC` and the walk-end + // block gated on it is skipped entirely -- including its own + // restore. Arm the deferred one here, or the undo stays armed + // for a leg that never runs and the frame keeps the EXECUTING + // pc `LiveLastInstrGuard` published (it declines to restore + // while a capture is live) over an operand stack no flush ever + // wrote. The legacy replay then re-enters one opcode late on + // an empty stack: `value-stack underflow`. + // + // Where the walk goes on to adopt a blackhole image the + // deferred restore is skipped by its own guard and this only + // consumes the request: the adoption claims the flushed frame, + // and rolling it back underneath would be the opposite defect. + mark_escape_flush_undo_pending(); + } else { // All-or-nothing decline: nothing was written, nothing to undo. discard_escape_flush_undo(); } diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs index 5221aec5ee8..b6bd9725232 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs @@ -7688,16 +7688,26 @@ pub(crate) fn try_walker_specialize_sys_getframe( // `f.mark_as_escaped()` — vm.py:54. `escaped` is not one of the six fields // `interp_jit.py:25-30` declares, so the store cannot force; it is // load-bearing at `executioncontext.py:99-106 leave`, which forces the - // leaving frame's own vref only for a frame that escaped. - ctx.trace_ctx.call_void_typed_with_effect( - pyre_interpreter::module::sys::vm::jit_frame_mark_as_escaped as *const (), - &[vable_op], - &[majit_ir::Type::Ref], - majit_ir::EffectInfo::new( - majit_ir::ExtraEffect::CannotRaise, - majit_ir::OopSpecIndex::None, - ), + // leaving frame's own vref only for a frame that escaped. Upstream traces + // it as the ordinary `setfield_gc` on the flag, so it is emitted as the + // read/or/store the `tb_frame` fold above already uses — an opaque call + // would hide the update from the optimizer and its heap cache. + let flags_descr = crate::descr::pyframe_flags_descr(); + let live_flags = + crate::state::opimpl_getfield_gc_i(ctx.trace_ctx, vable_op, flags_descr.clone()); + let escaped_bit = ctx + .trace_ctx + .const_int(i64::from(pyre_interpreter::PyFrame::FLAG_ESCAPED)); + let new_flags = ctx + .trace_ctx + .record_op(OpCode::IntOr, &[live_flags, escaped_bit]); + ctx.trace_ctx.record_op_with_descr( + OpCode::SetfieldGc, + &[vable_op, new_flags], + flags_descr.clone(), ); + ctx.trace_ctx + .heapcache_setfield_cached(vable_op, flags_descr.index(), new_flags); // The walk IS the interpreter running, so the recorded store has to take // effect here too — the residual would have applied it before returning. unsafe { (*frame).mark_as_escaped() }; diff --git a/pyre/pyre-jit-trace/src/trace.rs b/pyre/pyre-jit-trace/src/trace.rs index fd7cf96d08b..9f8b56cdbba 100644 --- a/pyre/pyre-jit-trace/src/trace.rs +++ b/pyre/pyre-jit-trace/src/trace.rs @@ -4112,6 +4112,13 @@ fn run_perfn_walk( { crate::jitcode_dispatch::restore_escape_flush_undo(); } + // Measured, so that the next reader does not re-derive it: an "is the + // capture still armed here" invariant does NOT gate this leg. On + // `getframe_caller_resume_coord_two_call_sites` every walk that + // reaches this point reports `armed=false fb=true` — the blackhole + // adoption above claims the flushed frame, and the capture is already + // consumed. A counter conditioned on the three flags being false would + // read 0 both with and without the force arm's deferred restore. // The third field marks an abort that may only consume the `Entry` // carrier: the `MidBody` carrier is latched exclusively by the first two // variants (`inline_call.rs`), so a kept-stack abort matching a MidBody @@ -6160,6 +6167,9 @@ pub mod fbw_diag { /// slot, little-endian) followed by one slot of packed counters. A `u64` /// export cannot carry a string, and the outcome set is far too large to /// spend a tally slot per variant. + /// + /// `pyre-wasm-runner` decodes the ring through its OWN copy of this + /// constant (`main.rs`); the two have to move together. pub const RING_BASE: usize = 14; pub const RING_ENTRIES: usize = 24; pub const RING_STRIDE: usize = 5;