diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs index 29626d104da..e37f3c3c4dc 100644 --- a/crates/vm/src/frame.rs +++ b/crates/vm/src/frame.rs @@ -581,8 +581,6 @@ pub struct Frame { /// tracer function for this frame (usually is None) pub trace: PyMutex, - /// Cell and free variable references (cellvars + freevars). - cells_frees: FrameUnsafeCell>, /// Previous line number for LINE event suppression. prev_line: FrameUnsafeCell, @@ -627,7 +625,6 @@ unsafe impl Traverse for Frame { // SAFETY: GC traversal does not run concurrently with frame execution. unsafe { (*self.localsplus.get()).traverse(tracer_fn); - (*self.cells_frees.get()).traverse(tracer_fn); } self.locals.traverse(tracer_fn); self.globals.traverse(tracer_fn); @@ -660,12 +657,6 @@ impl Frame { let num_cells = code.cellvars.len(); let nfrees = closure.len(); - let cells_frees: Box<[PyCellRef]> = - core::iter::repeat_with(|| PyCell::default().into_ref(&vm.ctx)) - .take(num_cells) - .chain(closure.iter().cloned()) - .collect(); - let nlocalsplus = nlocals .checked_add(num_cells) .and_then(|v| v.checked_add(nfrees)) @@ -677,9 +668,13 @@ impl Frame { LocalsPlus::new(nlocalsplus, max_stackdepth) }; - // Store cell objects at cellvars and freevars positions - for (i, cell) in cells_frees.iter().enumerate() { - localsplus.fastlocals_mut()[nlocals + i] = Some(cell.clone().into()); + // Store cell/free variable objects directly in localsplus + let fastlocals = localsplus.fastlocals_mut(); + for i in 0..num_cells { + fastlocals[nlocals + i] = Some(PyCell::default().into_ref(&vm.ctx).into()); + } + for (i, cell) in closure.iter().enumerate() { + fastlocals[nlocals + num_cells + i] = Some(cell.clone().into()); } Self { @@ -696,7 +691,6 @@ impl Frame { code, func_obj, lasti: Radium::new(0), - cells_frees: FrameUnsafeCell::new(cells_frees), prev_line: FrameUnsafeCell::new(0), trace: PyMutex::new(vm.ctx.none()), trace_lines: PyMutex::new(true), @@ -746,9 +740,9 @@ impl Frame { /// For full local/cell cleanup, call `clear_locals_and_stack()`. pub(crate) fn clear_stack_and_cells(&self) { // SAFETY: Called when frame is not executing (generator closed). + // Cell refs in fastlocals[nlocals..] are cleared by clear_locals_and_stack(). unsafe { (*self.localsplus.get()).stack_clear(); - let _old = core::mem::take(&mut *self.cells_frees.get()); } } @@ -777,8 +771,14 @@ impl Frame { /// Set cell contents by cell index. Only safe to call before frame execution starts. pub(crate) fn set_cell_contents(&self, cell_idx: usize, value: Option) { + let nlocals = self.code.varnames.len(); // SAFETY: Called before frame execution starts. - unsafe { (*self.cells_frees.get())[cell_idx].set(value) }; + let fastlocals = unsafe { (*self.localsplus.get()).fastlocals() }; + fastlocals[nlocals + cell_idx] + .as_ref() + .and_then(|obj| obj.downcast_ref::()) + .expect("cell slot empty or not a PyCell") + .set(value); } /// Store a borrowed back-reference to the owning generator/coroutine. @@ -917,7 +917,6 @@ impl Py { }, lasti: &self.lasti, object: self, - cells_frees: unsafe { &mut *self.cells_frees.get() }, prev_line: unsafe { &mut *self.prev_line.get() }, monitoring_mask: 0, }; @@ -969,7 +968,6 @@ impl Py { builtins_dict: None, lasti: &self.lasti, object: self, - cells_frees: unsafe { &mut *self.cells_frees.get() }, prev_line: unsafe { &mut *self.prev_line.get() }, monitoring_mask: 0, }; @@ -1010,7 +1008,6 @@ struct ExecutingFrame<'a> { builtins_dict: Option<&'a PyExact>, object: &'a Py, lasti: &'a PyAtomic, - cells_frees: &'a mut Box<[PyCellRef]>, prev_line: &'a mut u32, /// Cached monitoring events mask. Reloaded at Resume instruction only, monitoring_mask: u32, @@ -1038,6 +1035,18 @@ impl ExecutingFrame<'_> { self.lasti.load(Relaxed) } + /// Access the PyCellRef at the given cell/free variable index. + /// `cell_idx` is 0-based: 0..ncells for cellvars, ncells.. for freevars. + #[inline(always)] + fn cell_ref(&self, cell_idx: usize) -> &PyCell { + let nlocals = self.code.varnames.len(); + self.localsplus.fastlocals()[nlocals + cell_idx] + .as_ref() + .expect("cell slot empty") + .downcast_ref::() + .expect("cell slot is not a PyCell") + } + /// Perform deferred stack unwinding after set_f_lineno. /// /// set_f_lineno cannot pop the value stack directly because the execution @@ -1837,7 +1846,7 @@ impl ExecutingFrame<'_> { } Instruction::DeleteAttr { namei: idx } => self.delete_attr(vm, idx.get(arg)), Instruction::DeleteDeref { i } => { - self.cells_frees[i.get(arg) as usize].set(None); + self.cell_ref(i.get(arg) as usize).set(None); Ok(None) } Instruction::DeleteFast { var_num: idx } => { @@ -2272,7 +2281,8 @@ impl ExecutingFrame<'_> { }; self.push_value(match value { Some(v) => v, - None => self.cells_frees[i] + None => self + .cell_ref(i) .get() .ok_or_else(|| self.unbound_cell_exception(i, vm))?, }); @@ -2329,7 +2339,8 @@ impl ExecutingFrame<'_> { } Instruction::LoadDeref { i } => { let idx = i.get(arg) as usize; - let x = self.cells_frees[idx] + let x = self + .cell_ref(idx) .get() .ok_or_else(|| self.unbound_cell_exception(idx, vm))?; self.push_value(x); @@ -2942,7 +2953,7 @@ impl ExecutingFrame<'_> { } Instruction::StoreDeref { i } => { let value = self.pop_value(); - self.cells_frees[i.get(arg) as usize].set(Some(value)); + self.cell_ref(i.get(arg) as usize).set(Some(value)); Ok(None) } Instruction::StoreFast { var_num: idx } => {