diff --git a/majit/majit-backend-cranelift/src/compiler.rs b/majit/majit-backend-cranelift/src/compiler.rs index 36889d142c5..f39191fc8b7 100644 --- a/majit/majit-backend-cranelift/src/compiler.rs +++ b/majit/majit-backend-cranelift/src/compiler.rs @@ -17301,7 +17301,7 @@ mod tests { /// GC_STORE against callee jitframes. Mirrors the dynasm test layout /// (runner.rs install_call_assembler_test_layout); the offsets match /// the production cranelift mapping in pyre-jit call_jit.rs - /// arena_jitframe_descrs. Reads jitframe_gc_type_id() after + /// jitframe_layout_descrs. Reads jitframe_gc_type_id() after /// set_gc_allocator has lazily registered JITFRAME. fn install_call_assembler_test_layout() { register_jitframe_layout(JitFrameLayoutInfo { diff --git a/pyre/bench/synth/comprehension_object_append_hot.py b/pyre/bench/synth/comprehension_object_append_hot.py index 552ba6dffa2..e05d3b4b0a5 100644 --- a/pyre/bench/synth/comprehension_object_append_hot.py +++ b/pyre/bench/synth/comprehension_object_append_hot.py @@ -1,10 +1,4 @@ -# No `max-pypy-ratio` gate: pypy runs this in ~0.019s, two ticks of the 10ms -# user-CPU resolution, so the ratio's own granularity is ~30-50%. A run where -# pyre got FASTER (1.22s -> 1.09s) still reddened a ratio=40 gate, because pypy -# happened to measure 0.03s instead of 0.04s. Raising the bound far enough to -# absorb one tick leaves it too loose to catch anything, so the bench keeps only -# its output check — which is what it was written for. The comprehension's flat -# constant factor against pypy is tracked separately. +# pyre-check: max-pypy-ratio=32 # An inlined list comprehension whose LIST_APPEND element lands in a list # Object-strategy (tuple / None / str / dict / f-string) folds through the #171 # orthodox append. Its Object arm stores a GC ref and runs list_write_barrier, diff --git a/pyre/pyre-interpreter/src/display.rs b/pyre/pyre-interpreter/src/display.rs index c8fc14adb63..13bcb3f6866 100644 --- a/pyre/pyre-interpreter/src/display.rs +++ b/pyre/pyre-interpreter/src/display.rs @@ -362,6 +362,24 @@ pub(crate) unsafe fn builtin_subclass_dunder_obj( if w_class.is_null() || !pyre_object::is_type(w_class) { return Ok(None); } + // Only a subclass can redirect the dunder: it keeps the builtin + // `ob_type` and retags `w_class` (`typedef::subclass_to_tag`), which + // is exactly what `is_exact_builtin_instance` tests. An exact + // instance resolves the dunder to the builtin the caller is about to + // run natively, and the builtin types are immutable, so the two MRO + // walks and the descriptor call below can only reproduce it. + // + // `long` is the one leaf where the descriptor does more than the leaf + // formatter: `longobject.py descr_repr` also enforces + // `sys.set_int_max_str_digits`, and that check sits in the descriptor + // rather than in the conversion, so an exact `long` keeps going + // through it. A machine `int` cannot reach any settable limit — 19 + // digits against a floor of 640. + if !std::ptr::eq(tp, &LONG_TYPE as *const PyType) + && pyre_object::is_exact_builtin_instance(obj) + { + return Ok(None); + } let Some((src, found)) = crate::baseobjspace::lookup_where_pair(w_class, name) else { return Ok(None); }; diff --git a/pyre/pyre-interpreter/src/executioncontext.rs b/pyre/pyre-interpreter/src/executioncontext.rs index 5c0c20711bb..adf7ee999cb 100644 --- a/pyre/pyre-interpreter/src/executioncontext.rs +++ b/pyre/pyre-interpreter/src/executioncontext.rs @@ -351,6 +351,8 @@ impl ExecutionContext { // builtins_module, matching a fresh PyPy ExecutionContext. ec.builtin_dict_cache = std::cell::Cell::new(pyre_object::PY_NULL); ec.sys_exc_value = pyre_object::PY_NULL; + // executioncontext.py:53 — a fresh ExecutionContext starts at 0. + ec.coroutine_origin_tracking_depth = 0; ec.current_gen_or_coroutine = pyre_object::PY_NULL; ec.w_asyncgen_firstiter_fn = pyre_object::PY_NULL; ec.w_asyncgen_finalizer_fn = pyre_object::PY_NULL; diff --git a/pyre/pyre-interpreter/src/importing.rs b/pyre/pyre-interpreter/src/importing.rs index 74f38ddade7..adac1f7183c 100644 --- a/pyre/pyre-interpreter/src/importing.rs +++ b/pyre/pyre-interpreter/src/importing.rs @@ -357,8 +357,8 @@ pub fn mount_embedded_stdlib(mount: &Path) { // PyPy equivalent: `space.sys.get('modules')`, `space.sys.path`, and // `space.builtin_modules` are object-space/process state, shared by every // ExecutionContext. Raw GC references use the established process-global -// `usize` representation; the GIL serializes semantic access while the mutex -// also makes foreign STW root walks well-defined. +// `usize` representation; the mutex serializes semantic access and keeps +// foreign STW root walks well-defined. static SYS_MODULES: LazyLock>> = LazyLock::new(|| Mutex::new(HashMap::new())); static SYS_MODULES_DICT: AtomicUsize = AtomicUsize::new(0); diff --git a/pyre/pyre-interpreter/src/module/thread/mod.rs b/pyre/pyre-interpreter/src/module/thread/mod.rs index adf721f8158..eaaeb1ff6b1 100644 --- a/pyre/pyre-interpreter/src/module/thread/mod.rs +++ b/pyre/pyre-interpreter/src/module/thread/mod.rs @@ -707,11 +707,20 @@ mod handle_class { if is_none(timeout) { None } else if is_float(timeout) { - Some(Duration::from_secs_f64( - floatobject::w_float_get_value(timeout).max(0.0), - )) + // os_lock.py:33-39 parse_acquire_args — a timeout past the + // microsecond clock's range is an OverflowError, never a + // native abort. The negated comparison rejects NaN too. + let secs = floatobject::w_float_get_value(timeout); + if !(secs <= TIMEOUT_MAX) { + return Err(crate::PyError::overflow_error("timeout value is too large")); + } + Some(Duration::from_secs_f64(secs.max(0.0))) } else if is_int(timeout) { - Some(Duration::from_secs(w_int_get_value(timeout).max(0) as u64)) + let secs = w_int_get_value(timeout); + if secs as f64 > TIMEOUT_MAX { + return Err(crate::PyError::overflow_error("timeout value is too large")); + } + Some(Duration::from_secs(secs.max(0) as u64)) } else { return Err(crate::PyError::type_error( "timeout must be a number or None", @@ -857,6 +866,18 @@ mod local_class { #[staticmethod] fn __new__(cls: PyObjectRef, args: &[PyObjectRef]) -> Result { crate::typedef::check_user_subclass(type_object(), cls)?; + // os_local.py:81 rejects construction arguments before + // `allocate_instance`, and does so for every subtype that inherits + // `object.__init__` — not just for the exact type — so a refused + // construction never reaches `_register_in_ec` (os_local.py:40). + if args.len() > 1 + && unsafe { crate::baseobjspace::lookup_where_class_uncached(cls, "__init__") } + == Some(crate::typedef::w_object()) + { + return Err(crate::PyError::type_error( + "Initialization arguments are not supported", + )); + } // os_local.py installs the first dictionary before app-level // __init__ is entered, preventing recursive initialization. let dicts = pyre_object::w_dict_new(); @@ -872,15 +893,6 @@ mod local_class { }); unsafe { (*obj).w_class = cls }; register_local_in_current_ec(obj); - - // The base `_local` has no app-level initializer accepting arguments. - // Subclass initialization is dispatched by the ordinary type call - // after this allocator returns, exactly as for PyPy's TypeDef. - if args.len() > 1 && cls == type_object() { - return Err(crate::PyError::type_error( - "Initialization arguments are not supported", - )); - } Ok(obj) } @@ -1260,7 +1272,10 @@ fn stack_size(args: &[PyObjectRef]) -> Result { } let old = STACK_SIZE.load(Ordering::Relaxed); if let Some(&arg) = args.first() { - let size = unsafe { w_int_get_value(arg) }; + // `@unwrap_spec(size=int)` (os_thread.py:216) unwraps through + // `space.int_w`, which rejects a non-integer instead of reading its + // payload word as one. + let size = crate::baseobjspace::int_w(arg)?; if size < 0 || (size != 0 && size < 32_768) { return Err(crate::PyError::value_error(format!( "size not valid: {size} bytes" diff --git a/pyre/pyre-interpreter/src/module/time/interp_time.rs b/pyre/pyre-interpreter/src/module/time/interp_time.rs index dfb776ad0a4..c1b3fc6e1f6 100644 --- a/pyre/pyre-interpreter/src/module/time/interp_time.rs +++ b/pyre/pyre-interpreter/src/module/time/interp_time.rs @@ -169,12 +169,17 @@ pub fn sleep(args: &[PyObjectRef]) -> Result { return Ok(w_none()); } let dur = std::time::Duration::from_nanos(timeout_ns as u64); - // interp_time.py's `time_sleep` is an `@rffi` external call. In pyre the - // blocking mutator leaves the free-threaded GC's STW RUNNING census. - let _blocking = crate::module::thread::before_external_block(); + // `nanosleep` is a `releasegil=True` external (interp_time.py:504-506), so + // only the blocking call itself runs outside the free-threaded GC's STW + // RUNNING census. `checksignals` runs with the GIL re-acquired + // (interp_time.py:707), i.e. with the mutator back in the census — so the + // guard is scoped to each call, not to the whole retry loop. A Python + // signal handler running outside the census would trip the running-mutator + // assertions on its first allocation or blocking call. #[cfg(feature = "sandbox")] { // The controller services the sleep; signal handling is its concern. + let _blocking = crate::module::thread::before_external_block(); crate::host_seam::ops::sleep(dur.as_secs_f64()) .map_err(|e| crate::host_seam::seam_os_err(e, ""))?; Ok(w_none()) @@ -187,7 +192,11 @@ pub fn sleep(args: &[PyObjectRef]) -> Result { let deadline = std::time::Instant::now() + dur; let mut remaining = dur; loop { - match host_time::nanosleep(remaining) { + let slept = { + let _blocking = crate::module::thread::before_external_block(); + host_time::nanosleep(remaining) + }; + match slept { Ok(()) => return Ok(w_none()), Err(e) if e.raw_os_error() == Some(libc::EINTR) => { crate::module::signal::interp_signal::checksignals_now()?; @@ -208,6 +217,7 @@ pub fn sleep(args: &[PyObjectRef]) -> Result { } #[cfg(not(any(all(unix, feature = "host_env"), feature = "sandbox")))] { + let _blocking = crate::module::thread::before_external_block(); std::thread::sleep(dur); Ok(w_none()) } diff --git a/pyre/pyre-interpreter/src/objspace/std/mapdict.rs b/pyre/pyre-interpreter/src/objspace/std/mapdict.rs index 17f7af19dc9..a8affcb2c5b 100644 --- a/pyre/pyre-interpreter/src/objspace/std/mapdict.rs +++ b/pyre/pyre-interpreter/src/objspace/std/mapdict.rs @@ -43,12 +43,33 @@ static INSTANCE_LOCKS: LazyLock> = static CODE_CACHE_LOCKS: LazyLock> = LazyLock::new(|| (0..256).map(|_| ForkReentrantLock::new()).collect()); -fn instance_lock_for(obj: PyObjectRef) -> &'static ReentrantMutex<()> { - INSTANCE_LOCKS[(obj as usize >> 4) & (INSTANCE_LOCKS.len() - 1)].get() +type MapDictGuard = parking_lot::lock_api::ReentrantMutexGuard< + 'static, + parking_lot::RawMutex, + parking_lot::RawThreadId, + (), +>; + +/// A contended stripe must not be waited on from inside the running-mutator +/// census: the owner can allocate under the stripe and request a collection, +/// which then waits for this thread while this thread waits for the stripe. +/// Leave the census around the blocking acquire, as `w_list_lock` does. +fn lock_stripe(lock: &'static ReentrantMutex<()>) -> MapDictGuard { + if let Some(guard) = lock.try_lock() { + return guard; + } + let blocked = crate::module::thread::before_external_block(); + let guard = lock.lock(); + drop(blocked); + guard } -fn code_cache_lock_for(code: PyObjectRef) -> &'static ReentrantMutex<()> { - CODE_CACHE_LOCKS[(code as usize >> 4) & (CODE_CACHE_LOCKS.len() - 1)].get() +fn instance_lock(obj: PyObjectRef) -> MapDictGuard { + lock_stripe(INSTANCE_LOCKS[(obj as usize >> 4) & (INSTANCE_LOCKS.len() - 1)].get()) +} + +fn code_cache_lock(code: PyObjectRef) -> MapDictGuard { + lock_stripe(CODE_CACHE_LOCKS[(code as usize >> 4) & (CODE_CACHE_LOCKS.len() - 1)].get()) } pub fn after_fork_child() { @@ -384,7 +405,7 @@ pub unsafe fn instance_node_setdictvalue( name: &Wtf8, value: PyObjectRef, ) -> bool { - let _instance_guard = instance_lock_for(obj).lock(); + let _instance_guard = instance_lock(obj); ensure_mapdict_initialized(obj); let inst = &mut *(obj as *mut pyre_object::W_ObjectObject); let map = inst._get_mapdict_map(); @@ -409,7 +430,7 @@ pub unsafe fn instance_node_setdictvalue( /// `obj` must be a live `W_ObjectObject` (caller guards with `is_instance`). #[majit_macros::dont_look_inside] pub unsafe fn instance_node_getdictvalue(obj: PyObjectRef, name: &Wtf8) -> Option { - let _instance_guard = instance_lock_for(obj).lock(); + let _instance_guard = instance_lock(obj); ensure_mapdict_initialized(obj); let inst = &mut *(obj as *mut pyre_object::W_ObjectObject); let map = inst._get_mapdict_map(); @@ -434,7 +455,7 @@ pub unsafe fn instance_node_getdictvalue(obj: PyObjectRef, name: &Wtf8) -> Optio /// `obj` must be a live `W_ObjectObject` (caller guards with `is_instance`). #[majit_macros::dont_look_inside] pub unsafe fn instance_node_deldictvalue(obj: PyObjectRef, name: &Wtf8) -> bool { - let _instance_guard = instance_lock_for(obj).lock(); + let _instance_guard = instance_lock(obj); ensure_mapdict_initialized(obj); let inst = &mut *(obj as *mut pyre_object::W_ObjectObject); let map = inst._get_mapdict_map(); @@ -460,7 +481,7 @@ pub unsafe fn instance_node_deldictvalue(obj: PyObjectRef, name: &Wtf8) -> bool /// `obj` must be a live `W_ObjectObject` (caller guards with `is_instance`). #[majit_macros::dont_look_inside] pub unsafe fn instance_get_dict_slot(obj: PyObjectRef) -> Option { - let _instance_guard = instance_lock_for(obj).lock(); + let _instance_guard = instance_lock(obj); ensure_mapdict_initialized(obj); let inst = &*(obj as *const pyre_object::W_ObjectObject); let map = inst._get_mapdict_map(); @@ -480,7 +501,7 @@ pub unsafe fn instance_get_dict_slot(obj: PyObjectRef) -> Option { /// `obj` must be a live `W_ObjectObject` (caller guards with `is_instance`). #[majit_macros::dont_look_inside] pub unsafe fn instance_set_dict_slot(obj: PyObjectRef, w_dict: PyObjectRef) -> bool { - let _instance_guard = instance_lock_for(obj).lock(); + let _instance_guard = instance_lock(obj); ensure_mapdict_initialized(obj); let inst = &mut *(obj as *mut pyre_object::W_ObjectObject); let map = inst._get_mapdict_map(); @@ -494,7 +515,7 @@ pub unsafe fn instance_set_dict_slot(obj: PyObjectRef, w_dict: PyObjectRef) -> b /// `obj` must be a live `W_ObjectObject`. #[majit_macros::dont_look_inside] pub unsafe fn instance_get_weakref_slot(obj: PyObjectRef) -> Option { - let _instance_guard = instance_lock_for(obj).lock(); + let _instance_guard = instance_lock(obj); ensure_mapdict_initialized(obj); let inst = &*(obj as *const pyre_object::W_ObjectObject); let map = inst._get_mapdict_map(); @@ -511,7 +532,7 @@ pub unsafe fn instance_get_weakref_slot(obj: PyObjectRef) -> Option /// `obj` must be a live `W_ObjectObject`. #[majit_macros::dont_look_inside] pub unsafe fn instance_set_weakref_slot(obj: PyObjectRef, lifeline: PyObjectRef) -> bool { - let _instance_guard = instance_lock_for(obj).lock(); + let _instance_guard = instance_lock(obj); ensure_mapdict_initialized(obj); let inst = &mut *(obj as *mut pyre_object::W_ObjectObject); let map = inst._get_mapdict_map(); @@ -525,7 +546,7 @@ pub unsafe fn instance_set_weakref_slot(obj: PyObjectRef, lifeline: PyObjectRef) /// `obj` must be a live `W_ObjectObject`. #[majit_macros::dont_look_inside] pub unsafe fn instance_del_weakref_slot(obj: PyObjectRef) { - let _instance_guard = instance_lock_for(obj).lock(); + let _instance_guard = instance_lock(obj); ensure_mapdict_initialized(obj); let inst = &mut *(obj as *mut pyre_object::W_ObjectObject); let map = inst._get_mapdict_map(); @@ -1193,8 +1214,8 @@ pub unsafe fn load_attr_caching( // `_mapdict_caches` entry one atomic observation. Preserve those exact // owners under free threading with narrow synchronization around the // upstream cache operation. - let _instance_guard = instance_lock_for(w_obj).lock(); - let _code_cache_guard = code_cache_lock_for(pycode).lock(); + let _instance_guard = instance_lock(w_obj); + let _code_cache_guard = code_cache_lock(pycode); let entry = unsafe { crate::pycode::w_code_mapdict_caches_get(pycode, nameindex) }; // mapdict.py:1482 `map = w_obj._get_mapdict_map()`. let map = unsafe { mapdict_map_or_null(w_obj) }; @@ -1597,8 +1618,8 @@ pub unsafe fn store_attr_caching( name: &str, w_value: PyObjectRef, ) -> Result<(), PyError> { - let _instance_guard = instance_lock_for(w_obj).lock(); - let _code_cache_guard = code_cache_lock_for(pycode).lock(); + let _instance_guard = instance_lock(w_obj); + let _code_cache_guard = code_cache_lock(pycode); let entry = unsafe { crate::pycode::w_code_mapdict_caches_get(pycode, nameindex) }; // mapdict.py:1577 `map = w_obj._get_mapdict_map()`. let map = unsafe { mapdict_map_or_null(w_obj) }; @@ -2943,6 +2964,7 @@ static MAPDICT_ROOT_AREA: MapdictRootArea = MapdictRootArea; /// `obj` must be a live `W_ObjectObject` backing a hasdict instance. #[majit_macros::dont_look_inside] pub unsafe fn instance_node_dict_length(obj: PyObjectRef) -> usize { + let _instance_guard = instance_lock(obj); ensure_mapdict_initialized(obj); let inst = &*(obj as *const pyre_object::W_ObjectObject); let mut res: usize = 0; @@ -2964,6 +2986,7 @@ pub unsafe fn instance_node_dict_length(obj: PyObjectRef) -> usize { /// `obj` must be a live `W_ObjectObject` backing a hasdict instance. #[majit_macros::dont_look_inside] pub unsafe fn instance_node_dict_clear(obj: PyObjectRef) { + let _instance_guard = instance_lock(obj); ensure_mapdict_initialized(obj); let inst = &mut *(obj as *mut pyre_object::W_ObjectObject); let map = inst._get_mapdict_map(); @@ -3002,6 +3025,7 @@ unsafe fn dict_nodes_in_order(inst: &pyre_object::W_ObjectObject) -> Vec /// `obj` must be a live `W_ObjectObject` backing a hasdict instance. #[majit_macros::dont_look_inside] pub unsafe fn instance_node_dict_keys(obj: PyObjectRef) -> Vec { + let _instance_guard = instance_lock(obj); ensure_mapdict_initialized(obj); let inst = &*(obj as *const pyre_object::W_ObjectObject); let nodes = dict_nodes_in_order(inst); @@ -3034,6 +3058,7 @@ pub unsafe fn instance_node_dict_keys(obj: PyObjectRef) -> Vec { /// `obj` must be a live `W_ObjectObject` backing a hasdict instance. #[majit_macros::dont_look_inside] pub unsafe fn instance_node_dict_values(obj: PyObjectRef) -> Vec { + let _instance_guard = instance_lock(obj); ensure_mapdict_initialized(obj); let inst = &*(obj as *const pyre_object::W_ObjectObject); let nodes = dict_nodes_in_order(inst); @@ -3057,6 +3082,7 @@ pub unsafe fn instance_node_dict_values(obj: PyObjectRef) -> Vec { /// `obj` must be a live `W_ObjectObject` backing a hasdict instance. #[majit_macros::dont_look_inside] pub unsafe fn instance_node_dict_items(obj: PyObjectRef) -> Vec<(PyObjectRef, PyObjectRef)> { + let _instance_guard = instance_lock(obj); ensure_mapdict_initialized(obj); let inst = &*(obj as *const pyre_object::W_ObjectObject); let nodes = dict_nodes_in_order(inst); diff --git a/pyre/pyre-interpreter/src/pyframe.rs b/pyre/pyre-interpreter/src/pyframe.rs index 6d95fc84b06..e5bd59c4b07 100644 --- a/pyre/pyre-interpreter/src/pyframe.rs +++ b/pyre/pyre-interpreter/src/pyframe.rs @@ -515,32 +515,8 @@ pub fn remember_frame_locals_array(array: *mut FixedObjectArray) { } } -/// Allocate a `FixedObjectArray` pre-populated from `values`. The -/// resulting array has `values.len()` slots; allocation layout matches -/// [`alloc_fixed_array_with_header`]. -pub unsafe fn alloc_fixed_array_from_vec( - values: Vec, -) -> *mut FixedObjectArray { - unsafe { - let len = values.len(); - let layout = fixed_array_layout(len); - let raw = std::alloc::alloc_zeroed(layout); - if raw.is_null() { - std::alloc::handle_alloc_error(layout); - } - let arr = raw.add(GC_HEADER_SIZE) as *mut FixedObjectArray; - (*arr).len = len; - let items = (arr as *mut u8).add(pyre_object::FIXED_ARRAY_ITEMS_OFFSET) - as *mut pyre_object::PyObjectRef; - for (i, v) in values.into_iter().enumerate() { - items.add(i).write(v); - } - arr - } -} - /// Deallocate a `FixedObjectArray` allocated with -/// [`alloc_fixed_array_with_header`] or [`alloc_fixed_array_from_vec`]. +/// [`alloc_fixed_array_with_header`]. pub unsafe fn dealloc_array_with_gc_header(ptr: *mut FixedObjectArray) { if ptr.is_null() { return; @@ -2184,46 +2160,6 @@ impl PyFrame { Ok(self.get_w_locals()) } - /// Create a minimal frame stub for passing to call dispatch. - /// Used by MIFrame Box tracking when concrete_frame is unavailable. - pub fn new_minimal(code: *const (), execution_context: *const PyExecutionContext) -> Self { - let raw = - unsafe { crate::w_code_get_ptr(code as pyre_object::PyObjectRef) as *const CodeObject }; - let nlocals = unsafe { (&*raw).varnames.len() }; - let ncells = unsafe { ncells(&*raw) }; - let size = nlocals + ncells + 16; // small stack - // `pyframe.py:98 __init__(self, space, code, w_globals, ...)` - // stores `w_globals` as the canonical W_DictObject directly. - // This storage-only builder carries no globals object. - let w_globals = PY_NULL; - let w_builtin = crate::baseobjspace::frame_builtin_obj(w_globals, execution_context); - // pyframe.py:103 — stamp `pycode.w_globals`; side effect only (the - // gated debugdata snapshot retired in favour of `w_globals`). - unsafe { - crate::w_code_frame_stores_global(code as PyObjectRef, w_globals); - } - let mut frame = PyFrame { - ob_header: frame_ob_header(), - execution_context, - pycode: code, - locals_cells_stack_w: unsafe { - alloc_fixed_array_from_vec(vec![pyre_object::PY_NULL; size]) - }, - valuestackdepth: nlocals + ncells, - last_instr: -1, - flags: 0, - debugdata: std::ptr::null_mut(), - lastblock: std::ptr::null_mut(), - vable_token: 0, - f_generator_nowref: PY_NULL, - w_yielding_from: PY_NULL, - f_backref: std::ptr::null_mut(), - w_builtin, - w_globals, - }; - frame - } - /// Test-helper constructor — creates a frame with a fresh execution /// context. /// @@ -2299,60 +2235,6 @@ impl PyFrame { crate::createframe_obj(w_code as *const (), w_globals, ctx_ptr, None) } - /// PyFrame constructor body called from `createframe` (PyPy - /// `baseobjspace.py:796`) when `outer_func` is `None` — sets up the - /// fixed-array stack, debug data, w_globals binding, and module-level - /// `w_locals = w_globals` semantics. Crate-private helper kept as the - /// namespace constructor shape even when call sites allocate directly. - pub(crate) fn new_with_namespace( - code: *const (), - execution_context: *const PyExecutionContext, - ) -> Self { - let raw = - unsafe { crate::w_code_get_ptr(code as pyre_object::PyObjectRef) as *const CodeObject }; - let code_ref = unsafe { &*raw }; - let num_locals = code_ref.varnames.len(); - let num_cells = ncells(code_ref); - let max_stack = code_ref.max_stackdepth as usize; - - // This storage-only builder carries no globals object. - let w_globals = PY_NULL; - let w_builtin = crate::baseobjspace::frame_builtin_obj(w_globals, execution_context); - // pyframe.py:103 — stamp `pycode.w_globals`; side effect only (the - // gated debugdata snapshot retired in favour of `w_globals`). - unsafe { - crate::w_code_frame_stores_global(code as PyObjectRef, w_globals); - } - let mut frame = PyFrame { - ob_header: frame_ob_header(), - execution_context, - pycode: code, - locals_cells_stack_w: unsafe { - alloc_fixed_array_with_header(num_locals + num_cells + max_stack, PY_NULL) - }, - valuestackdepth: num_locals + num_cells, - last_instr: -1, - flags: 0, - debugdata: std::ptr::null_mut(), - lastblock: std::ptr::null_mut(), - vable_token: 0, - f_generator_nowref: PY_NULL, - w_yielding_from: PY_NULL, - f_backref: std::ptr::null_mut(), - w_builtin, - w_globals, - }; - // Module-level w_locals = w_globals binding flows naturally - // through `createframe → initialize_frame_scopes` since RustPython - // codegen emits empty flags for the module seed CodeInfo - // (pyframe.py:216-218). This constructor bypasses - // initialize_frame_scopes, so still bind w_locals to w_globals - // explicitly to match what `createframe` would observe — in the - // object form (the canonical W_DictObject), not the raw storage. - frame.getorcreate_debug_data(-1).w_locals = w_globals; - frame - } - /// RPython MetaInterp traces against its own MIFrame stack instead of /// mutating the live interpreter frame in place. pyre still executes /// bytecodes concretely during tracing, so use an owned snapshot when @@ -2941,8 +2823,7 @@ impl PyFrame { /// pyframe.py:216-220 `get_builtin` — returns `self.builtin` (the /// per-frame picked builtin Module, set at frame creation by /// `pick_builtin(w_globals)`). Falls back to the EC's default - /// builtin when the frame was constructed without globals (e.g. - /// `PyFrame::new_minimal` stub frames). + /// builtin when the frame was constructed without globals. #[inline] pub fn get_builtin(&self) -> PyObjectRef { if !self.w_builtin.is_null() { diff --git a/pyre/pyre-jit-trace/src/helpers.rs b/pyre/pyre-jit-trace/src/helpers.rs index 7d42be971cb..18693ea2410 100644 --- a/pyre/pyre-jit-trace/src/helpers.rs +++ b/pyre/pyre-jit-trace/src/helpers.rs @@ -948,8 +948,8 @@ pub fn emit_box_float_inline( /// self-recursive single-int-argument fast path. /// /// Replaces the opaque `jit_create_self_recursive_callee_frame_1_raw_int` -/// CallR that today (`call_jit.rs:2814`) wraps `arena.take()` + reuse -/// check + locals zero-fill + raw_int boxing in an opaque helper. The +/// CallR, which wraps the frame allocation, locals fill and raw_int +/// boxing in an opaque helper. The /// helper is `#[dont_look_inside]` so the optimizer cannot virtualize /// the new frame nor fold the boxing — every fib(35) iteration pays /// the full helper trampoline (~336k calls/run, observed in 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 8096a42851f..f4b94133501 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/inline_call.rs @@ -2072,6 +2072,11 @@ pub(crate) fn try_walker_inline_resolved_user_call( // below met). For a strict callee this gates routing its guards through the // multi-frame snapshot vs. falling back to collapse. let mut callee_frame_seeded = false; + // The concrete callee frame the seed block materializes, retained so the + // sub-walk can put it on the interpreter frame chain: the walk executes + // the callee's residuals for real, and a residual that reads the chain + // (`sys._getframe`, a traceback) must see the callee it is running in. + let mut concrete_callee_frame = std::ptr::null_mut::(); if try_multiframe || strict_seed { 'seed: { // Branch-A frame shape only (mirror REC_CA): no cells. @@ -2211,6 +2216,7 @@ pub(crate) fn try_walker_inline_resolved_user_call( ); drop(arg_roots); let concrete_frame_ptr = frame.as_mut_ptr(); + concrete_callee_frame = concrete_frame_ptr; callee_concrete_r[frame_reg as usize] = ConcreteValue::Ref(concrete_frame_ptr as pyre_object::PyObjectRef); ctx.trace_ctx.set_opref_concrete( @@ -2497,6 +2503,9 @@ pub(crate) fn try_walker_inline_resolved_user_call( let _inline_frame = InlineFrameGuard::enter(ctx.session, callee_code_key, parent_frame); let _foriter_deferred = ForiterDeferredInlineGuard::enter(callee_code_key, foriter_deferred_admit); + // Name the frame this sub-walk executes concretely, so each residual + // it runs can `enter`/`leave` it on the interpreter frame chain. + let _inline_concrete_frame = InlineConcreteFrameGuard::enter(concrete_callee_frame); if let Some(frame) = ActiveResumeFrame::current(ctx.session, ctx.fbw_mode.snapshot_sym) { if frame.body_matches(&body) { seed_callee_vstack_mirror(&mut sub_wc, &frame); 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 d3ddbeaef62..c8c51eeccb6 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/residual_call.rs @@ -297,6 +297,110 @@ pub(crate) fn escape_flush_undo_cell_ptr() -> *const std::cell::RefCell = + const { std::cell::Cell::new(std::ptr::null_mut()) }; + + /// The concrete frame currently published on the interpreter chain by a + /// live [`ResidualFrameChainGuard`], or null. Distinct from + /// [`INLINE_CONCRETE_FRAME`], which stays set across the whole sub-walk. + static PUBLISHED_INLINE_FRAME: std::cell::Cell<*mut pyre_interpreter::PyFrame> = + const { std::cell::Cell::new(std::ptr::null_mut()) }; +} + +/// Names the frame an inline sub-walk executes concretely for the duration of +/// that sub-walk. Publishing it on the interpreter chain is left to +/// [`ResidualFrameChainGuard`], which brackets only the residual calls. +pub(crate) struct InlineConcreteFrameGuard(*mut pyre_interpreter::PyFrame); + +impl InlineConcreteFrameGuard { + pub(crate) fn enter(frame: *mut pyre_interpreter::PyFrame) -> Self { + // Set unconditionally, null included: a nested sub-walk whose seed + // block bailed has no frame of its own, and inheriting the enclosing + // callee's frame would publish it for the inner callee's residuals — + // resolving one level too shallow, the error this guard exists to + // stop. A null slot makes `ResidualFrameChainGuard::enter` publish + // nothing. + let previous = INLINE_CONCRETE_FRAME.with(|slot| slot.replace(frame)); + Self(previous) + } +} + +impl Drop for InlineConcreteFrameGuard { + fn drop(&mut self) { + INLINE_CONCRETE_FRAME.with(|slot| slot.set(self.0)); + } +} + +/// `executioncontext.py:85 enter` / `:91 leave` around one concretely executed +/// residual call of an inline sub-walk. +/// +/// Inlining a call elides the callee's real call sequence, so nothing +/// publishes the callee frame. PyPy can leave the chain alone — its +/// metainterp never runs the callee for real while tracing — but the walker +/// does, and a residual that reads the chain (`sys._getframe`, a traceback) +/// would otherwise observe the caller as the running frame and resolve one +/// level too shallow. Scoped to the call itself so the walk's own frame +/// bookkeeping outside it is untouched. +struct ResidualFrameChainGuard { + ec: *mut pyre_interpreter::PyExecutionContext, + frame: *mut pyre_interpreter::PyFrame, + saved_topframeref: *mut pyre_interpreter::PyFrame, + previous_published: *mut pyre_interpreter::PyFrame, +} + +impl ResidualFrameChainGuard { + fn enter() -> Option { + let frame = INLINE_CONCRETE_FRAME.with(|slot| slot.get()); + if frame.is_null() { + return None; + } + let ec = unsafe { (*frame).execution_context } as *mut pyre_interpreter::PyExecutionContext; + if ec.is_null() { + return None; + } + let saved_topframeref = unsafe { (*ec).topframeref }; + // Re-entering the same frame would make it its own caller. + if std::ptr::eq(saved_topframeref, frame) { + return None; + } + unsafe { + (*frame).f_backref = saved_topframeref; + (*ec).topframeref = frame; + } + let previous_published = PUBLISHED_INLINE_FRAME.with(|slot| slot.replace(frame)); + Some(Self { + ec, + frame, + saved_topframeref, + previous_published, + }) + } +} + +impl Drop for ResidualFrameChainGuard { + fn drop(&mut self) { + unsafe { + // `executioncontext.py:91-109 leave`: move the raw caller vref + // back without forcing it, then, when the frame escaped, force + // the caller and mark it escaped too. A frame handed to + // application code keeps a reference to its caller, so the caller + // must stay materialised; dropping that propagation would leave + // the escape recorded only on a frame the walk owns privately. + PUBLISHED_INLINE_FRAME.with(|slot| slot.set(self.previous_published)); + (*self.ec).topframeref = self.saved_topframeref; + if (*self.frame).escaped() { + let f_back = (*self.frame).get_f_back(); + if !f_back.is_null() { + (*f_back).mark_as_escaped(); + } + } + } + } +} + struct ActiveFrameEscapeGuard { prev: Option<(usize, usize)>, prev_stack: Option>, @@ -325,15 +429,34 @@ impl Drop for ActiveFrameEscapeGuard { /// Returns whether `frame` is the one recorded as escaping this residual call /// (`expected == frame`) — i.e. the traced virtualizable itself was handed to -/// Python, so its tracing token must be forced. A residual callee inspecting -/// its own frame passes a different `frame` and returns false. Independently, -/// the walk-end resume pc is committed only when the state flush succeeds (a -/// merge point with cached depth); a matched frame that cannot flush still -/// escaped and must be forced, so the two signals are decoupled. +/// Python, so its tracing token must be forced. The concrete frame an inline +/// sub-walk published counts as well: it runs under that virtualizable, so +/// handing it out escapes the virtualizable too. Any other frame a residual +/// callee inspects returns false. Independently, the walk-end resume pc is +/// committed only when the state flush succeeds (a merge point with cached +/// depth); a directly matched frame that cannot flush still escaped and must +/// be forced, so for it the two signals are decoupled. pub fn flush_active_frame_escape(ctx: &TraceCtx, frame: *mut pyre_interpreter::PyFrame) -> bool { + // `executioncontext.py:104-106 leave` — a frame handed to application code + // keeps a reference to its caller, so escaping the concrete frame an inline + // sub-walk published escapes the traced virtualizable it runs under. The + // flush stays keyed on that virtualizable, whose resume pc this residual + // already latched, so the walk resumes forward rather than replaying from + // entry. + let escaped_published = PUBLISHED_INLINE_FRAME.with(|slot| { + let published = slot.get(); + let matched = !published.is_null() && std::ptr::eq(published, frame); + if matched { + let f_back = unsafe { (*published).get_f_back() }; + if !f_back.is_null() { + unsafe { (*f_back).mark_as_escaped() }; + } + } + matched + }); ACTIVE_FRAME_ESCAPE.with(|slot| { if let Some((expected, py_pc)) = slot.get() - && expected == frame as usize + && (expected == frame as usize || escaped_published) { // Force #2+ within this residual (`enter` resets the committed pc // per residual): the live frame is already heap-authoritative @@ -357,7 +480,12 @@ pub fn flush_active_frame_escape(ctx: &TraceCtx, frame: *mut pyre_interpreter::P // All-or-nothing decline: nothing was written, nothing to undo. discard_escape_flush_undo(); } - return true; + // A directly matched frame escaped whether or not the flush + // committed, so the two signals stay decoupled. A redirected one + // is reported only once the resume pc is committed: forcing + // without it would raise an escape the walk can only answer by + // replaying from entry, double-applying this residual's body. + return flushed || expected == frame as usize; } false }) @@ -1616,6 +1744,9 @@ pub(crate) fn try_execute_residual_call_via_executor( .then(|| ctx.vstack_boxes.clone()); let _frame_escape = ActiveFrameEscapeGuard::enter(escape_frame, ctx.vstack_cur_pypc as usize, escape_stack); + // `executioncontext.py:85 enter` for the inlined callee this residual + // runs inside of. + let _frame_chain = ResidualFrameChainGuard::enter(); let _suspend = majit_metainterp::TraceContinuationSuspendGuard::enter(); majit_metainterp::executor::execute_residual_call(call_descr, func_ptr, &args) }; diff --git a/pyre/pyre-jit-trace/src/state.rs b/pyre/pyre-jit-trace/src/state.rs index 581a92c5803..568b5bbe9d5 100644 --- a/pyre/pyre-jit-trace/src/state.rs +++ b/pyre/pyre-jit-trace/src/state.rs @@ -11999,7 +11999,6 @@ mod tests { /// `MetaInterpFrame`. No upstream counterpart. pub struct PendingInlineFrame { pub sym: PyreSym, - pub concrete_frame: pyre_interpreter::pyframe::PyFrame, pub green_key: u64, /// Raw `(code_ptr, target_pc)` greenkey components for element- /// wise recursion-depth comparison. `green_key` above is the u64 @@ -12087,8 +12086,6 @@ pub(crate) fn assemble_bridge_inline_pending( execution_context: *const pyre_interpreter::PyExecutionContext, parent_frames: Vec, ) -> PendingInlineFrame { - use pyre_interpreter::pyframe::PyFrame; - let nlocals = recipe.nlocals; let valuestackdepth = recipe.valuestackdepth; @@ -12109,35 +12106,12 @@ pub(crate) fn assemble_bridge_inline_pending( // object, so this is non-null here. let w_globals = recover_inline_callee_globals(recipe.code_ptr); - // resume.py:1042-1057 newframe + reload: build a fresh concrete frame for - // the callee's own pycode wrapper and seed - // `locals_cells_stack_w[0..valuestackdepth]` from the decoded boxes. The - // callee has no cells/freevars (gated in `reconstruct_inline_recipe`), so - // `closure = PY_NULL` and the array layout is `[locals | stack]` with - // `stack_base() == nlocals`. - let mut concrete_frame = PyFrame::new_for_call_with_closure_and_globals_obj( - w_code, - &[], - w_globals, - execution_context, - pyre_object::PY_NULL, - pyre_interpreter::pyframe::FrameLocalsArrayAllocation::StdAlloc, - ); - { - let arr = concrete_frame.locals_w_mut(); - for k in 0..nlocals { - arr[k] = recipe_slot_to_pyobj(recipe.concrete_r[k]); - } - } - for k in nlocals..valuestackdepth { - concrete_frame.push(recipe_slot_to_pyobj(recipe.concrete_r[k])); - } - // last_instr is one before the recipe's Python pc so next_instr() resumes there. - concrete_frame.set_last_instr_from_next_instr(forward_py_pc_or_backxlat( - recipe.jitcode_index, - recipe.jitcode_pc, - ) as usize); - + // resume.py:1042-1057 newframe + reload: the decoded boxes are reloaded + // into the symbolic frame below. The callee's concrete state lives in + // `sym.concrete_locals` / `sym.concrete_stack` and in the emitted frame + // vable that `setup_reconstructed_callee_frame` binds to `sym.frame`; + // no separate interpreter-side `PyFrame` is materialized here. + // // Symbolic side: mirror the FAST branch field-for-field. let mut sym = PyreSym::new_uninit(OpRef::NONE); sym.nlocals = nlocals; @@ -12189,7 +12163,6 @@ pub(crate) fn assemble_bridge_inline_pending( PendingInlineFrame { sym, - concrete_frame, // The reconstructed frame represents the same inlined call the // forward trace pushed at function entry; match its (code, 0) // greenkey identity for recursion-depth + inline-position tracking. @@ -12199,9 +12172,8 @@ pub(crate) fn assemble_bridge_inline_pending( nargs: recipe.nargs, caller_result_stack_idx: None, caller_result_type: Some(Type::Ref), - // Reconstructed frames carry no CALL-site OpRefs; the inline - // back-edge CALL_ASSEMBLER path requires drop_frame_opref and - // is gated out for them anyway. + // Reconstructed frames carry no CALL-site OpRefs, and the inline + // back-edge CALL_ASSEMBLER path is gated out for them anyway. replay_callable: OpRef::NONE, replay_args: Vec::new(), } diff --git a/pyre/pyre-jit-trace/src/trace.rs b/pyre/pyre-jit-trace/src/trace.rs index 60a17f42b40..16effb9667c 100644 --- a/pyre/pyre-jit-trace/src/trace.rs +++ b/pyre/pyre-jit-trace/src/trace.rs @@ -1146,9 +1146,9 @@ fn drive_bridge_carrier_walk( let pre_pos = ctx.get_trace_position(); // `setup_reconstructed_callee_frame` emits the callee frame vable into the // trace and returns `argboxes_r` seeding the portal reds + in-flight - // operand-stack temps; the `_pending` callee sym/concrete frame is unused on - // the sub-walk path (the sub-walk drives the callee body off `argboxes_r` + - // the emitted frame vable, not a callee MIFrame). + // operand-stack temps; the `_pending` callee sym is unused on the sub-walk + // path (the sub-walk drives the callee body off `argboxes_r` + the emitted + // frame vable, not a callee MIFrame). let Some((_pending, argboxes_r)) = crate::state::setup_reconstructed_callee_frame(ctx, recipe, root_ec, Vec::new()) else { diff --git a/pyre/pyre-jit/src/call_jit.rs b/pyre/pyre-jit/src/call_jit.rs index 8dc0ff83159..5b0e23742d0 100644 --- a/pyre/pyre-jit/src/call_jit.rs +++ b/pyre/pyre-jit/src/call_jit.rs @@ -5,7 +5,6 @@ use std::borrow::Cow; use std::cell::UnsafeCell; -use std::mem::MaybeUninit; use std::sync::Once; /// Whether `PYRE_NBODY_DEBUG` is set, cached at first access. @@ -225,16 +224,9 @@ fn self_recursive_dispatch(green_key: u64) -> Option { // Force cache implementation removed — CallAssemblerI + bridge // handles recursive dispatch natively. -// ── Callee frame arena (RPython nursery bump equivalent) ───────── -// ── Global arena pointers for Cranelift inline access ────────────── -// -// Single-threaded JIT invariant: only one thread executes compiled code -// at a time, so these globals need no synchronization. -static mut ARENA_BUF_BASE: *mut u8 = std::ptr::null_mut(); -static mut ARENA_TOP: usize = 0; -static mut ARENA_INITIALIZED: usize = 0; +// ── JitFrame layout descriptors published to the backends ───────── -fn arena_jitframe_descrs() -> majit_gc::rewrite::JitFrameDescrs { +fn jitframe_layout_descrs() -> majit_gc::rewrite::JitFrameDescrs { use majit_backend::jitframe::*; majit_gc::rewrite::JitFrameDescrs { jitframe_tid: crate::jit::descr::JITFRAME_GC_TYPE_ID, @@ -257,215 +249,104 @@ fn arena_jitframe_descrs() -> majit_gc::rewrite::JitFrameDescrs { #[cfg(test)] mod tests { - use super::arena_jitframe_descrs; + use super::jitframe_layout_descrs; use majit_backend::jitframe::{FIRST_ITEM_OFFSET, JF_FRAME_OFS}; #[test] - fn arena_jitframe_descrs_uses_frame_relative_offsets() { - let descrs = arena_jitframe_descrs(); + fn jitframe_layout_descrs_uses_frame_relative_offsets() { + let descrs = jitframe_layout_descrs(); assert_eq!(descrs.jf_frame_baseitemofs, FIRST_ITEM_OFFSET); assert_eq!(descrs.jf_frame_lengthofs, JF_FRAME_OFS); } } #[cfg(feature = "cranelift")] -pub fn arena_global_info() -> majit_backend_cranelift::JitFrameLayoutInfo { +pub fn jitframe_layout_info() -> majit_backend_cranelift::JitFrameLayoutInfo { majit_backend_cranelift::JitFrameLayoutInfo { - jitframe_descrs: Some(arena_jitframe_descrs()), + jitframe_descrs: Some(jitframe_layout_descrs()), } } #[cfg(feature = "dynasm")] -pub fn arena_global_info_dynasm() -> majit_backend_dynasm::JitFrameLayoutInfo { +pub fn jitframe_layout_info_dynasm() -> majit_backend_dynasm::JitFrameLayoutInfo { majit_backend_dynasm::JitFrameLayoutInfo { - jitframe_descrs: Some(arena_jitframe_descrs()), + jitframe_descrs: Some(jitframe_layout_descrs()), } } -// -// LIFO stack of pre-allocated PyFrame slots. Recursive call/return -// order is naturally LIFO, so arena_take/arena_put are O(1). -// Eliminates heap allocation for recursion depths up to ARENA_CAP. - -const ARENA_CAP: usize = 64; +// ── Callee frame allocation ─────────────────────────────────────── -/// GcStruct layout: [GcHeader (8 bytes)] [struct fields]. -/// Every GC object (including PyFrame / W_Root) is prepended by a -/// zeroed GcHeader. Arena slots and heap fallbacks match this layout. -/// Single source of truth: [`majit_gc::header::GcHeader::SIZE`]. -const GC_HEADER_SIZE: usize = majit_gc::header::GcHeader::SIZE; - -/// Arena slot: leading GcHeader (tid 0, flags 0) then the frame payload. -#[repr(C)] -struct GcFrameSlot { - gc_header: majit_gc::header::GcHeader, - frame: MaybeUninit, -} - -impl GcFrameSlot { - const fn zeroed() -> Self { - GcFrameSlot { - gc_header: majit_gc::header::GcHeader { tid_and_flags: 0 }, - frame: MaybeUninit::uninit(), - } - } -} +thread_local! { + /// Callee frames the JIT created and has not finished running. + /// Their only root between `jit_create_callee_frame_*` and + /// `jit_drop_callee_frame` is this list: compiled code holds the + /// frame in a register / jitframe slot, and the frame sits on no + /// `CURRENT_FRAME` chain. Walked by `walk_jit_callee_frame_roots`. + static LIVE_CALLEE_FRAMES: UnsafeCell> = const { UnsafeCell::new(Vec::new()) }; -/// Heap-allocated frame with prepended GcHeader. -#[repr(C)] -struct GcPyFrame { - gc_header: majit_gc::header::GcHeader, - frame: PyFrame, + static JIT_CALLEE_FRAME_ROOT_AREA: JitCalleeFrameRootArea = JitCalleeFrameRootArea { + live_frames: LIVE_CALLEE_FRAMES.with(|cell| cell as *const _), + }; } -fn heap_alloc_frame(frame: PyFrame) -> *mut PyFrame { - let gc_frame = Box::into_raw(Box::new(GcPyFrame { - gc_header: majit_gc::header::GcHeader { tid_and_flags: 0 }, - frame, - })); - let ptr = unsafe { &mut (*gc_frame).frame as *mut PyFrame }; - HEAP_CALLEE_FRAMES.with(|cell| unsafe { &mut *cell.get() }.push(ptr)); +struct JitCalleeFrameRootArea { + live_frames: *const UnsafeCell>, +} + +/// Create a callee frame for a JIT call, exactly as the interpreter +/// creates every other frame: `baseobjspace.py:799-801 createframe` on a +/// `pyframe.py:52 class PyFrame(W_Root)`, a normal GC object whose +/// lifetime is its reachability. Nothing recycles the block, so a frame +/// the program retains past the call — a traceback node, an `f_back` +/// chain, a `sys._getframe` result — stays valid for as long as Python +/// can reach it. +fn alloc_callee_frame( + code: *const (), + args: &[PyObjectRef], + w_globals: PyObjectRef, + execution_context: *const pyre_interpreter::PyExecutionContext, +) -> *mut PyFrame { + let ptr = pyre_interpreter::pyframe::FrameBox::new( + PyFrame::new_for_call_with_closure_and_globals_obj( + code, + args, + w_globals, + execution_context, + PY_NULL, + pyre_interpreter::pyframe::FrameLocalsArrayAllocation::OldGenGc, + ), + ) + .into_raw(); + LIVE_CALLEE_FRAMES.with(|cell| unsafe { &mut *cell.get() }.push(ptr)); ptr } -fn heap_free_frame(ptr: *mut PyFrame) { - HEAP_CALLEE_FRAMES.with(|cell| { +/// Drop the JIT's root on a callee frame that finished running. The +/// frame itself is not freed — `executioncontext.py:91-107 leave` frees +/// nothing either; the collector reclaims it once nothing reaches it. +fn unroot_callee_frame(ptr: *mut PyFrame) { + // This list is what makes a minor collection scan the frame's slots. + // Once it stops, a young ref stored since the last minor — argument + // boxing, the CALL_ASSEMBLER writeback through `jit_frame_set_slot_*` — + // is reachable only through the old-gen locals array's own items, which + // a minor does not walk. The frame outlives the call whenever the program + // retained it (a traceback node, an `f_back` chain), so re-arm the + // remembered set here, at the moment root-walking stops: the same barrier + // `pyframe.py:52 __init__` arms at creation, for the same reason. + unsafe { + pyre_interpreter::pyframe::remember_frame_locals_array((*ptr).locals_cells_stack_w); + } + if pyre_object::gc_hook::try_gc_owns_object(ptr as *mut u8) { + pyre_object::gc_hook::try_gc_write_barrier(ptr as *mut u8); + } + LIVE_CALLEE_FRAMES.with(|cell| { let frames = unsafe { &mut *cell.get() }; - if let Some(pos) = frames.iter().position(|p| *p == ptr) { + // Calls return in LIFO order, so the match is normally the last + // entry. + if let Some(pos) = frames.iter().rposition(|p| *p == ptr) { frames.swap_remove(pos); } }); - let gc_frame = unsafe { (ptr as *mut u8).sub(GC_HEADER_SIZE) as *mut GcPyFrame }; - unsafe { drop(Box::from_raw(gc_frame)) }; -} - -thread_local! { - /// Live heap-fallback callee frames (arena overflow). Walked by - /// `walk_jit_callee_frame_roots` alongside armed arena slots. - static HEAP_CALLEE_FRAMES: UnsafeCell> = const { UnsafeCell::new(Vec::new()) }; -} - -struct FrameArena { - buf: Box<[GcFrameSlot; ARENA_CAP]>, - /// Number of frames currently in use (LIFO stack pointer). - top: usize, - /// Frames below this index have been initialized at least once. - /// Reuse only needs reinit of changed fields, not full new_for_call. - initialized: usize, - /// Per-slot GC visibility: set once a slot's frame is fully - /// initialized for the current call, cleared on `put`. The extra - /// root walker (`walk_jit_callee_frame_roots`) visits only armed - /// slots — `top` alone cannot be used because a non-LIFO `put` - /// leaves dead slots below `top`, and a slot between `take` and - /// end-of-init holds an uninitialized or stale frame. - armed: [bool; ARENA_CAP], -} - -impl FrameArena { - fn new() -> Self { - let mut arena = Self { - buf: Box::new([const { GcFrameSlot::zeroed() }; ARENA_CAP]), - top: 0, - initialized: 0, - armed: [false; ARENA_CAP], - }; - // Publish stable pointers so Cranelift-generated code can - // inline arena take/put without going through TLS. - unsafe { - ARENA_BUF_BASE = arena.buf.as_mut_ptr() as *mut u8; - ARENA_TOP = 0; - ARENA_INITIALIZED = 0; - } - arena - } - - /// Take the next frame slot. Returns (ptr, was_previously_initialized). - /// The returned pointer points to the PyFrame part (after the GcHeader). - #[inline] - fn take(&mut self) -> Option<(*mut PyFrame, bool)> { - if self.top < ARENA_CAP { - let idx = self.top; - self.top += 1; - unsafe { - ARENA_TOP = self.top; - } - let ptr = self.buf[idx].frame.as_mut_ptr(); - let was_init = idx < self.initialized; - Some((ptr, was_init)) - } else { - None - } - } - - /// Return a frame to the arena. Must be the most recently taken frame (LIFO). - #[inline] - fn put(&mut self, ptr: *mut PyFrame) -> bool { - if let Some(idx) = self.slot_index(ptr) { - self.armed[idx] = false; - } - if self.top > 0 && ptr == self.buf[self.top - 1].frame.as_mut_ptr() { - self.top -= 1; - unsafe { - ARENA_TOP = self.top; - } - return true; - } - // Check if within arena range — don't free, but mark as non-LIFO. - self.slot_index(ptr).is_some() - } - - /// Slot index for a frame pointer inside the arena buffer, if any. - #[inline] - fn slot_index(&self, ptr: *mut PyFrame) -> Option { - let base = self.buf.as_ptr() as usize; - let end = unsafe { (self.buf.as_ptr()).add(ARENA_CAP) } as usize; - let addr = ptr as usize; - if addr >= base && addr < end { - Some((addr - base) / std::mem::size_of::()) - } else { - None - } - } - - /// Mark a fully-initialized in-use slot as visible to the GC root - /// walker. Call only after the frame body and its locals array are - /// completely written for the current call. - #[inline] - fn arm(&mut self, ptr: *mut PyFrame) { - if let Some(idx) = self.slot_index(ptr) { - self.armed[idx] = true; - } - } - - /// Mark that frames up to `top` have been fully initialized. - #[inline] - fn mark_initialized(&mut self) { - if self.top > self.initialized { - self.initialized = self.top; - unsafe { - ARENA_INITIALIZED = self.top; - } - } - } -} - -thread_local! { - static FRAME_ARENA: UnsafeCell = UnsafeCell::new(FrameArena::new()); - - static JIT_CALLEE_FRAME_ROOT_AREA: JitCalleeFrameRootArea = JitCalleeFrameRootArea { - arena: FRAME_ARENA.with(|cell| cell as *const _), - heap_frames: HEAP_CALLEE_FRAMES.with(|cell| cell as *const _), - }; -} - -struct JitCalleeFrameRootArea { - arena: *const UnsafeCell, - heap_frames: *const UnsafeCell>, -} - -#[inline] -fn arena_ref() -> &'static mut FrameArena { - FRAME_ARENA.with(|cell| unsafe { &mut *cell.get() }) } /// Visit the GC-ref slots of one live callee frame: every @@ -485,17 +366,16 @@ unsafe fn visit_callee_frame_roots(frame: *mut PyFrame, visitor: &mut dyn FnMut( visitor(unsafe { &mut *(&mut frame.w_globals as *mut PyObjectRef as *mut GcRef) }); } -/// Extra GC root walker for JIT-created callee frames (frame arena + -/// heap fallbacks). These frames are host-allocated (zeroed GcHeader, -/// outside the GC heap) and sit on no `CURRENT_FRAME`/`f_backref` -/// chain while compiled code runs, so neither the standard tracer nor -/// `walk_pyframe_roots` reaches their locals. Without this walk, a -/// young object stored into a callee frame slot (argument boxing, -/// back-edge CALL_ASSEMBLER writeback) is invisible to a minor -/// collection and the slot is left pointing at evacuated nursery -/// memory. Registered via `register_extra_root_walker`, mirroring the -/// framework.py `root_walker.walk_roots` seam the collector already -/// uses for the other host-side root sources. +/// Extra GC root walker for the callee frames a JIT call is running. +/// They sit on no `CURRENT_FRAME`/`f_backref` chain while compiled code +/// runs, so `walk_pyframe_roots` reaches neither the frames themselves +/// nor their locals: a major collection would sweep the block compiled +/// code is executing on, and a young object stored into a frame slot +/// (argument boxing, back-edge CALL_ASSEMBLER writeback) would be +/// invisible to a minor collection, leaving the slot pointing at +/// evacuated nursery memory. Registered via `register_extra_root_walker`, +/// mirroring the framework.py `root_walker.walk_roots` seam the collector +/// already uses for the other host-side root sources. pub fn walk_jit_callee_frame_roots(visitor: &mut dyn FnMut(&mut GcRef)) { let data = capture_jit_callee_frame_root_area(); unsafe { walk_jit_callee_frame_roots_area(data, visitor) }; @@ -513,15 +393,13 @@ pub unsafe fn walk_jit_callee_frame_roots_area( visitor: &mut dyn FnMut(&mut GcRef), ) { let area = unsafe { &*(data as *const JitCalleeFrameRootArea) }; - let arena = unsafe { &mut *(*area.arena).get() }; - for idx in 0..ARENA_CAP { - if arena.armed[idx] { - unsafe { visit_callee_frame_roots(arena.buf[idx].frame.as_mut_ptr(), visitor) }; - } - } - let heap_frames = unsafe { &*(*area.heap_frames).get() }; - for &ptr in heap_frames.iter() { - unsafe { visit_callee_frame_roots(ptr, visitor) }; + let live_frames = unsafe { &mut *(*area.live_frames).get() }; + for slot in live_frames.iter_mut() { + // Mark the frame object itself — this list is its only root + // until the call returns. The frame is a non-moving stable + // allocation, so the visitor never rewrites the slot. + visitor(unsafe { &mut *(slot as *mut *mut PyFrame as *mut GcRef) }); + unsafe { visit_callee_frame_roots(*slot, visitor) }; } } @@ -777,33 +655,37 @@ pub extern "C" fn jit_force_callee_frame(frame_ptr: i64) -> i64 { // `assembler_call_helper` (warmspot.py:1021-1028) resumes the callee // frame the rewritten CALL_ASSEMBLER passed as arg 0. - portal_runner_from_raw_frame_ptr(frame_ptr) + run_frame_through_portal(frame_ptr) } -/// warmspot.py:941-959 `ll_portal_runner` core — reconstruct a proper -/// interpreter frame from a raw JitFrame-like block pointer and run it -/// through the portal. +/// warmspot.py:941-959 `ll_portal_runner` core — run the frame the JIT handed +/// in through the portal (`maybe_compile_and_run` + interpreter main loop; +/// ContinueRunningNormally re-enters the JIT via the portal, +/// warmspot.py:961-983). /// -/// Nursery-safe force: read code/namespace/exec_ctx via raw offsets (valid -/// for both arena `PyFrame` AND nursery-allocated raw blocks), build a proper -/// `PyFrame`, then hand it to `portal_runner` (`maybe_compile_and_run` + -/// interpreter main loop; ContinueRunningNormally re-enters the JIT via the -/// portal, warmspot.py:961-983). The callee frame may be a nursery-allocated -/// JitFrame-like block, so the fields are recovered from raw offsets. -fn portal_runner_from_raw_frame_ptr(frame_ptr: i64) -> i64 { - let (code, w_globals, exec_ctx) = unsafe { - use pyre_interpreter::pyframe::*; - let p = frame_ptr as *const u8; - let code = *(p.add(PYFRAME_PYCODE_OFFSET) as *const *const ()); - let w_globals = *(p.add(PYFRAME_W_GLOBALS_OFFSET) as *const pyre_object::PyObjectRef); - let ec = *(p.add(std::mem::offset_of!(PyFrame, execution_context)) - as *const *const pyre_interpreter::PyExecutionContext); - (code, w_globals, ec) - }; - let mut func_frame = PyFrame::new_for_call_with_globals_obj(code, &[], w_globals, exec_ctx); - func_frame.fix_array_ptrs(); - - let result = crate::eval::portal_runner(&mut func_frame); +/// The pointer is the callee `PyFrame` the trace itself built — +/// `emit_new_pyframe_inline_with_params` emits `NewWithVtable` + +/// `SetfieldGc` for `pycode` / `w_globals` / `execution_context` and the +/// locals array, and the GC rewriter stores that pointer into the callee +/// jitframe's first slot, which is what every force site reads back. It is +/// the `frame` red of `jd.portal_calldescr`, and `ll_portal_runner` forwards +/// its reds unchanged (`warmspot.py:953-954` `portal_ptr(*args)`). +/// +/// Rebuilding a frame here instead would drop the callee's arguments (its +/// locals array) and its `last_instr`, and would run the interpreter on a +/// frame that dies with the C stack. +fn run_frame_through_portal(frame_ptr: i64) -> i64 { + // `jit_drop_callee_frame` tolerates a tagged word on the same slot because + // it only unroots; there is nothing sensible to run here for one. The + // portal's result is a Ref whose NULL spelling means "exception stored", + // so returning NULL for a non-frame would manufacture an exception-less + // error rather than deopt cleanly. + debug_assert!( + frame_ptr != 0 && frame_ptr & 1 == 0, + "CALL_ASSEMBLER arg 0 must be the callee PyFrame" + ); + let frame = unsafe { &mut *(frame_ptr as *mut PyFrame) }; + let result = crate::eval::portal_runner(frame); // warmspot.py:449 result_type=REF: always boxed Ref result as i64 @@ -834,12 +716,7 @@ pub extern "C" fn ll_portal_runner_shim( // constructed — a proper `PyFrame` with `locals_cells_stack_w` already // populated (NewWithVtable + SetfieldGc). Run it directly; the greens // (`next_instr` / `pycode`) are redundant because the frame carries them. - // (Contrast `jit_force_callee_frame`, whose CA_FORCE_FN deadframe is a raw - // JitFrame-like block that must be reconstructed with fresh fields.) - let frame = unsafe { &mut *(frame_ptr as *mut PyFrame) }; - let result = crate::eval::portal_runner(frame); - // warmspot.py:449 result_type=REF: always boxed Ref - result as i64 + run_frame_through_portal(frame_ptr) } /// warmspot.py:1021-1028 — assembler_call_helper. @@ -1537,7 +1414,7 @@ pub fn install_jit_call_bridge() { majit_backend_cranelift::register_call_assembler_blackhole( jit_blackhole_resume_from_guard, ); - majit_backend_cranelift::register_jitframe_layout(arena_global_info()); + majit_backend_cranelift::register_jitframe_layout(jitframe_layout_info()); majit_backend_cranelift::register_call_assembler_unbox_int(unbox_int_for_force); // resume.py:763-870 VStr/VUni.allocate parity — Cranelift // backend's materialize_virtual_recursive invokes these @@ -1572,7 +1449,7 @@ pub fn install_jit_call_bridge() { majit_backend_dynasm::register_call_assembler_blackhole( jit_blackhole_resume_from_guard, ); - majit_backend_dynasm::register_jitframe_layout(arena_global_info_dynasm()); + majit_backend_dynasm::register_jitframe_layout(jitframe_layout_info_dynasm()); majit_backend_dynasm::register_call_assembler_unbox_int(unbox_int_for_force); // rpython/jit/backend/llsupport/llmodel.py:229-234 insert_stack_check // parity. The backend inlines MOV [endaddr]; SUB rsp; CMP [lengthaddr] @@ -3644,7 +3521,7 @@ fn try_compile_ca_bridge( /// that actually finished (a base case, or a chained-bridge finish the arm's /// static fast-path set did not recognise) short-circuits to its output slot. /// -/// `frame_ptr` is the deopted callee arena frame; `compiled_ptr` is the source +/// `frame_ptr` is the deopted callee frame; `compiled_ptr` is the source /// loop's `CompiledWasmLoop`, both baked into the trace by `compile_bridge`. #[cfg(target_arch = "wasm32")] pub extern "C" fn wasm_ca_resume_deopt(frame_ptr: i64, compiled_ptr: i64) -> i64 { @@ -3828,28 +3705,6 @@ fn fill_positional_defaults_for_jit_call<'a>( Cow::Owned(full) } -#[inline] -fn reset_reused_call_frame(frame: &mut PyFrame, args: &[PyObjectRef]) { - frame.locals_w_mut().as_mut_slice().fill(PY_NULL); - let nargs = args.len().min(frame.nlocals()); - for (idx, value) in args.iter().take(nargs).enumerate() { - frame.locals_w_mut()[idx] = *value; - } - frame.valuestackdepth = frame.stack_base(); - frame.set_last_instr_from_next_instr(0); - frame.vable_token = 0; - frame.set_frame_finished_execution(false); - frame.f_generator_nowref = PY_NULL; - frame.w_yielding_from = PY_NULL; - frame.f_backref = std::ptr::null_mut(); - // pyframe.py:78-86: reused arena frames must look like new frames. - // debugdata and lastblock are GC-managed refs — release references only, - // never manually free (JIT snapshots may still hold these pointers). - frame.debugdata = std::ptr::null_mut(); - frame.set_escaped(false); - frame.set_blocklist(&[]); -} - fn create_callee_frame_impl_1_boxed( caller_frame: i64, callable: PyObjectRef, @@ -3862,60 +3717,7 @@ fn create_callee_frame_impl_1_boxed( let args = fill_positional_defaults_for_jit_call(callable, w_code, &one_arg); let args = args.as_ref(); - let arena = arena_ref(); - if let Some((ptr, was_init)) = arena.take() { - if was_init { - let f = unsafe { &mut *ptr }; - if f.pycode == w_code - && f.w_globals == w_globals - && f.execution_context == caller.execution_context - { - reset_reused_call_frame(f, args); - } else { - unsafe { - // Different function: drop the previous frame before - // overwriting, so PyFrame::drop releases the old - // locals_cells_stack_w (pyframe.rs:150). - std::ptr::drop_in_place(ptr); - std::ptr::write( - ptr, - PyFrame::new_for_call_with_globals_obj( - w_code, - args, - w_globals, - caller.execution_context, - ), - ); - (&mut *ptr).fix_array_ptrs(); - } - } - } else { - unsafe { - std::ptr::write( - ptr, - PyFrame::new_for_call_with_globals_obj( - w_code, - args, - w_globals, - caller.execution_context, - ), - ); - (&mut *ptr).fix_array_ptrs(); - } - arena.mark_initialized(); - } - arena.arm(ptr); - return ptr as i64; - } - - let frame_ptr = heap_alloc_frame(PyFrame::new_for_call_with_globals_obj( - w_code, - args, - w_globals, - caller.execution_context, - )); - unsafe { &mut *frame_ptr }.fix_array_ptrs(); - frame_ptr as i64 + alloc_callee_frame(w_code, args, w_globals, caller.execution_context) as i64 } fn create_self_recursive_callee_frame_impl_1_boxed( @@ -3927,70 +3729,11 @@ fn create_self_recursive_callee_frame_impl_1_boxed( let w_globals = caller.w_globals; let execution_context = caller.execution_context; - let arena = arena_ref(); - if let Some((ptr, was_init)) = arena.take() { - if was_init { - let f = unsafe { &mut *ptr }; - if f.pycode == func_code - && f.w_globals == w_globals - && f.execution_context == execution_context - { - // Reuse: same code/globals/ec — full reset matching - // new_for_call_with_closure() semantics. No partial - // shortcuts: blackhole/force paths must see a clean frame. - reset_reused_call_frame(f, &[boxed_arg]); - } else { - unsafe { - std::ptr::drop_in_place(ptr); - std::ptr::write( - ptr, - PyFrame::new_for_call_with_globals_obj( - func_code, - &[boxed_arg], - w_globals, - execution_context, - ), - ); - (&mut *ptr).fix_array_ptrs(); - } - } - } else { - unsafe { - std::ptr::write( - ptr, - PyFrame::new_for_call_with_globals_obj( - func_code, - &[boxed_arg], - w_globals, - execution_context, - ), - ); - (&mut *ptr).fix_array_ptrs(); - } - arena.mark_initialized(); - } - arena.arm(ptr); - if majit_metainterp::majit_log_enabled() { - let f = unsafe { &*ptr }; - eprintln!( - "[jit][ca-frame] ptr={ptr:p} locals=0x{:x} vsd={} reused={} boxed_arg=0x{:x}", - f.locals_cells_stack_w as usize, f.valuestackdepth, was_init, boxed_arg as usize, - ); - } - return ptr as i64; - } - - let frame_ptr = heap_alloc_frame(PyFrame::new_for_call_with_globals_obj( - func_code, - &[boxed_arg], - w_globals, - execution_context, - )); - unsafe { &mut *frame_ptr }.fix_array_ptrs(); + let frame_ptr = alloc_callee_frame(func_code, &[boxed_arg], w_globals, execution_context); if majit_metainterp::majit_log_enabled() { let f = unsafe { &*frame_ptr }; eprintln!( - "[jit][ca-frame] ptr={frame_ptr:p} locals=0x{:x} vsd={} reused=false boxed_arg=0x{:x}", + "[jit][ca-frame] ptr={frame_ptr:p} locals=0x{:x} vsd={} boxed_arg=0x{:x}", f.locals_cells_stack_w as usize, f.valuestackdepth, boxed_arg as usize, ); } @@ -4005,63 +3748,7 @@ fn create_callee_frame_impl(caller_frame: i64, callable: i64, args: &[PyObjectRe let args = fill_positional_defaults_for_jit_call(callable, w_code, args); let args = args.as_ref(); - let arena = arena_ref(); - if let Some((ptr, was_init)) = arena.take() { - if was_init { - // Fast reinit: only update fields that change between calls. - // code, execution_context, namespace, locals_cells_stack_w.ptr - // are stable for self-recursion (same function, same module). - let f = unsafe { &mut *ptr }; - if f.pycode == w_code - && f.w_globals == w_globals - && f.execution_context == caller.execution_context - { - reset_reused_call_frame(f, args); - } else { - // Different function: full reinit (rare for fib) - unsafe { - std::ptr::drop_in_place(ptr); - std::ptr::write( - ptr, - PyFrame::new_for_call_with_globals_obj( - w_code, - args, - w_globals, - caller.execution_context, - ), - ); - (&mut *ptr).fix_array_ptrs(); - } - } - } else { - // First-time init for this arena slot - unsafe { - std::ptr::write( - ptr, - PyFrame::new_for_call_with_globals_obj( - w_code, - args, - w_globals, - caller.execution_context, - ), - ); - (&mut *ptr).fix_array_ptrs(); - } - arena.mark_initialized(); - } - arena.arm(ptr); - return ptr as i64; - } - - // Arena full: heap fallback (should not happen for recursion < 64) - let frame_ptr = heap_alloc_frame(PyFrame::new_for_call_with_globals_obj( - w_code, - args, - w_globals, - caller.execution_context, - )); - unsafe { &mut *frame_ptr }.fix_array_ptrs(); - frame_ptr as i64 + alloc_callee_frame(w_code, args, w_globals, caller.execution_context) as i64 } #[majit_macros::dont_look_inside] @@ -4114,62 +3801,11 @@ pub extern "C" fn jit_create_self_recursive_callee_frame_1_raw_int( let boxed = pyre_object::intobject::w_int_new(raw_int_arg); - let arena = arena_ref(); - if let Some((ptr, was_init)) = arena.take() { - let f = unsafe { &mut *ptr }; - if was_init - && f.pycode == func_code - && f.w_globals == w_globals - && f.execution_context == execution_context - { - // Reuse: full reset matching new_for_call semantics. - reset_reused_call_frame(f, &[boxed]); - } else { - unsafe { - if was_init { - std::ptr::drop_in_place(ptr); - } - std::ptr::write( - ptr, - PyFrame::new_for_call_with_globals_obj( - func_code, - &[boxed], - w_globals, - execution_context, - ), - ); - (&mut *ptr).fix_array_ptrs(); - } - if !was_init { - arena.mark_initialized(); - } - } - arena.arm(ptr); - if majit_metainterp::majit_log_enabled() { - let f = unsafe { &*ptr }; - eprintln!( - "[jit][ca-frame-raw] ptr={ptr:p} locals=0x{:x} local0=0x{:x} vsd={} reused={} raw_arg={}", - f.locals_cells_stack_w as usize, - f.locals_w()[0] as usize, - f.valuestackdepth, - was_init, - raw_int_arg, - ); - } - return ptr as i64; - } - - let frame_ptr = heap_alloc_frame(PyFrame::new_for_call_with_globals_obj( - func_code, - &[boxed], - w_globals, - execution_context, - )); - unsafe { &mut *frame_ptr }.fix_array_ptrs(); + let frame_ptr = alloc_callee_frame(func_code, &[boxed], w_globals, execution_context); if majit_metainterp::majit_log_enabled() { let f = unsafe { &*frame_ptr }; eprintln!( - "[jit][ca-frame-raw] ptr={frame_ptr:p} locals=0x{:x} local0=0x{:x} vsd={} reused=false raw_arg={}", + "[jit][ca-frame-raw] ptr={frame_ptr:p} locals=0x{:x} local0=0x{:x} vsd={} raw_arg={}", f.locals_cells_stack_w as usize, f.locals_w()[0] as usize, f.valuestackdepth, @@ -4273,15 +3909,7 @@ pub extern "C" fn jit_drop_callee_frame(frame_ptr: i64) { if majit_metainterp::majit_log_enabled() { eprintln!("[jit][ca-drop] ptr={ptr:p}"); } - let arena = arena_ref(); - let reused = arena.put(ptr); - if majit_metainterp::majit_log_enabled() { - eprintln!("[jit][ca-drop] ptr={ptr:p} arena_reused={reused}"); - } - if !reused { - // Not an arena frame (heap fallback) — free GcPyFrame allocation. - heap_free_frame(ptr); - } + unroot_callee_frame(ptr); } /// Store a W_Root into a callee frame's `locals_cells_stack_w[idx]`. @@ -6605,8 +6233,8 @@ mod tests_bh_normalize_raise { use pyre_interpreter::{PyErrorKind, compile_exec}; #[test] - fn arena_jitframe_descrs_uses_frame_relative_offsets() { - let descrs = arena_jitframe_descrs(); + fn jitframe_layout_descrs_uses_frame_relative_offsets() { + let descrs = jitframe_layout_descrs(); assert_eq!(descrs.jf_frame_baseitemofs, FIRST_ITEM_OFFSET); assert_eq!(descrs.jf_frame_lengthofs, JF_FRAME_OFS); } diff --git a/pyre/pyre-jit/src/eval.rs b/pyre/pyre-jit/src/eval.rs index 97584c60059..f928d1de287 100644 --- a/pyre/pyre-jit/src/eval.rs +++ b/pyre/pyre-jit/src/eval.rs @@ -498,9 +498,9 @@ unsafe fn generator_object_custom_trace(obj_addr: usize, f: &mut dyn FnMut(*mut /// `pyframe_object_custom_trace` recurses into locals/cells/ /// valuestack and the `f_backref` chain, so a frame reachable only /// through a live traceback (the whole point of `tb_frame`) is not -/// reclaimed. A non-Gc frame (Box tracer snapshot / arena callee, -/// already freed by the time the traceback escapes) is left -/// dangling exactly as before — never dereferenced. +/// reclaimed. A non-Gc frame (a `FrameBox::new_boxed` tracer +/// snapshot, freed at the end of its walk) is left dangling exactly +/// as before — never dereferenced. unsafe fn pytraceback_object_custom_trace( obj_addr: usize, f: &mut dyn FnMut(*mut majit_ir::GcRef), @@ -1952,12 +1952,12 @@ fn build_gc() -> Box { // Frames stamped with this type id: JIT-built inline frames // (`emit_new_pyframe_inline_self_recursive`, whose locals array is a // GC-managed `PY_OBJECT_ARRAY_GC_TYPE_ID` block) AND executing / - // generator `FrameBox` frames (`FrameBox::new` via + // generator / JIT-callee `FrameBox` frames (`FrameBox::new` via // `try_gc_alloc_stable`, whose locals array is a stationary // `std::alloc` block). The custom trace's regime split - // (`try_gc_owns_object`) handles both. Callee-arena JIT frames - // remain `type_id = 0` off-GC blocks reached only as roots via - // `walk_jit_callee_frame_roots` (S2c). + // (`try_gc_owns_object`) handles both. A callee frame the JIT is + // still running is additionally kept reachable by + // `walk_jit_callee_frame_roots`, since it sits on no frame chain. // // Frame-owned locals arrays, debug data, and block-stack nodes are all // GC-managed. The collector reclaims them with the frame once it is @@ -3745,16 +3745,16 @@ unsafe extern "C" fn force_pyframe(frame: *mut pyre_interpreter::PyFrame) { driver.meta_interp_mut().force_virtualizable_token(token); }); }; - // Force the traced frame only when the frame handed to Python IS the - // traced virtualizable (it was the one recorded as escaping). Clearing - // TOKEN_TRACING_RESCALL is what `tracing_after_residual_call` reads as - // "the callee forced the virtualizable", raising - // `VableEscapedDuringResidualCall` so the walk resumes forward. A - // residual callee inspecting its own frame escapes a DIFFERENT frame; - // clearing the token there raises a spurious escape with no committed - // resume pc, so the walk replays from entry — double-applying the - // residual's non-journaled body effects. Skipping the force there - // leaves the callee frame (which is not the traced shadow) untouched. + // Force the traced frame only when the frame handed to Python belongs + // to the traced virtualizable — either it IS the virtualizable, or it + // is the concrete frame an inline sub-walk published and therefore + // runs under it. Clearing TOKEN_TRACING_RESCALL is what + // `tracing_after_residual_call` reads as "the callee forced the + // virtualizable", raising `VableEscapedDuringResidualCall` so the walk + // resumes forward. Any other frame a residual callee inspects is not + // the traced shadow; clearing the token there raises a spurious escape + // with no committed resume pc, so the walk replays from entry — + // double-applying the residual's non-journaled body effects. if traced_frame_escaped && let Some(ptr) = tracing_frame { force(ptr); } diff --git a/pyre/pyre-object/src/listobject.rs b/pyre/pyre-object/src/listobject.rs index 748aff8cb81..e7bf89480b6 100644 --- a/pyre/pyre-object/src/listobject.rs +++ b/pyre/pyre-object/src/listobject.rs @@ -1355,6 +1355,7 @@ pub unsafe fn w_list_pop_end(obj: PyObjectRef) -> Option { /// state, exactly like PyPy. The next append will pick a fresh typed /// strategy via switch_to_correct_strategy. pub unsafe fn w_list_clear(obj: PyObjectRef) { + let _list_guard = w_list_lock(obj); let list = &mut *(obj as *mut W_ListObject); list.drop_object_items(); list.int_items = IntArray::from_vec(Vec::new()); diff --git a/pyre/pyrex/src/lib.rs b/pyre/pyrex/src/lib.rs index 5513006402f..6e26c9536ef 100644 --- a/pyre/pyrex/src/lib.rs +++ b/pyre/pyrex/src/lib.rs @@ -977,8 +977,13 @@ fn run_module(module: &str, no_site: bool) { if e.kind == PyErrorKind::SystemExit { finalize_system_exit(e, canonical, ec_ptr); } - maybe_print_jit_stats(); + // targetpypystandalone.py:88 `finally: space.finish()` — finalize on + // every exit path, not only on SystemExit. Print first: the raw + // `PyObjectRef` fields of `e` are not GC-visible and `finalize_runtime` + // collects. pyre_interpreter::eprint_exception(&e, true); + finalize_runtime(canonical, ec_ptr); + maybe_print_jit_stats(); std::process::exit(1); } finalize_runtime(canonical, ec_ptr); @@ -1103,8 +1108,13 @@ fn run_source(source: &str, mode: Mode, filename: &str, no_site: bool) { if e.kind == PyErrorKind::SystemExit { finalize_system_exit(e, canonical, ec_ptr); } - maybe_print_jit_stats(); + // targetpypystandalone.py:88 `finally: space.finish()` — finalize + // on every exit path, not only on SystemExit. Print first: the raw + // `PyObjectRef` fields of `e` are not GC-visible and + // `finalize_runtime` collects. pyre_interpreter::eprint_exception(&e, true); + finalize_runtime(canonical, ec_ptr); + maybe_print_jit_stats(); std::process::exit(1); } }