diff --git a/crates/host_env/src/ctypes.rs b/crates/host_env/src/ctypes.rs index 25f921853..972afe533 100644 --- a/crates/host_env/src/ctypes.rs +++ b/crates/host_env/src/ctypes.rs @@ -1860,6 +1860,12 @@ impl CallbackThunk { } } + /// Borrow the userdata owned by this callback closure. + pub fn userdata(&self) -> &U { + // SAFETY: new stores Box::into_raw here; Drop owns its only reclamation. + unsafe { &*self.userdata_ptr } + } + pub fn code_ptr(&self) -> CodePtr { self.code_ptr } diff --git a/crates/vm/src/builtins/type.rs b/crates/vm/src/builtins/type.rs index b450fe7d0..8b0ceea13 100644 --- a/crates/vm/src/builtins/type.rs +++ b/crates/vm/src/builtins/type.rs @@ -306,6 +306,7 @@ unsafe impl crate::object::Traverse for PyType { self.attributes.traverse(tracer_fn); if let Some(ext) = self.heaptype_ext.as_ref() { ext.specialization_cache.traverse(tracer_fn); + ext.type_data.traverse(tracer_fn); } } @@ -336,6 +337,11 @@ unsafe impl crate::object::Traverse for PyType { self.attributes.drain_into(out); if let Some(ext) = self.heaptype_ext.as_ref() { ext.specialization_cache.clear_into(out); + if let Some(mut data) = ext.type_data.try_write() + && let Some(data) = data.as_mut() + { + data.clear(out); + } } } } @@ -1700,6 +1706,18 @@ impl PyType { /// Initialize type data for this type. Can only be called once. /// Returns an error if the type is not a heap type or if data is already initialized. pub fn init_type_data(&self, data: T) -> Result<(), String> { + self.init_type_data_slot(TypeDataSlot::new(data)) + } + + /// Initialize extension storage with GC traversal and clearing callbacks. + pub fn init_traced_type_data( + &self, + data: T, + ) -> Result<(), String> { + self.init_type_data_slot(TypeDataSlot::new_traced(data)) + } + + fn init_type_data_slot(&self, data: TypeDataSlot) -> Result<(), String> { let ext = self .heaptype_ext .as_ref() @@ -1709,7 +1727,7 @@ impl PyType { if type_data.is_some() { return Err("Type data already initialized".to_string()); } - *type_data = Some(TypeDataSlot::new(data)); + *type_data = Some(data); Ok(()) } diff --git a/crates/vm/src/stdlib/_ctypes.rs b/crates/vm/src/stdlib/_ctypes.rs index 4345b7fbf..152b85bc5 100644 --- a/crates/vm/src/stdlib/_ctypes.rs +++ b/crates/vm/src/stdlib/_ctypes.rs @@ -46,8 +46,11 @@ impl Py { .filter(|info| info.initialized) } - /// Get _type_ attribute as String (type code like "i", "d", etc.) + /// Get the scalar converter selected when the storage type was initialized. fn type_code(&self, vm: &VirtualMachine) -> Option { + if let Some(info) = self.stg_info_opt() { + return info.scalar_code.map(|code| code.to_string()); + } self.as_object() .get_attr("_type_", vm) .ok() @@ -62,7 +65,7 @@ impl Py { } else { let mut stg = StgInfo::default(); stg.flags |= StgInfoFlags::DICTFLAG_FINAL; - let _ = base.init_type_data(stg); + let _ = base.init_traced_type_data(stg); } } } diff --git a/crates/vm/src/stdlib/_ctypes/array.rs b/crates/vm/src/stdlib/_ctypes/array.rs index 386975719..b192fb871 100644 --- a/crates/vm/src/stdlib/_ctypes/array.rs +++ b/crates/vm/src/stdlib/_ctypes/array.rs @@ -381,12 +381,23 @@ impl AsSequence for PyCArrayType { name = "Array", base = PyCData, metaclass = "PyCArrayType", - module = "_ctypes" + module = "_ctypes", + traverse = "manual" )] #[derive(Debug)] #[repr(transparent)] pub(crate) struct PyCArray(pub PyCData); +// SAFETY: this transparent wrapper owns exactly the references in its base. +unsafe impl crate::object::Traverse for PyCArray { + fn traverse(&self, tracer: &mut crate::object::TraverseFn<'_>) { + self.0.traverse(tracer); + } + fn clear(&mut self, out: &mut Vec) { + self.0.clear(out); + } +} + impl PyCArray { /// Get the type code of array element type (e.g., "c" for c_char, "u" for c_wchar) fn get_element_type_code(zelf: &Py, vm: &VirtualMachine) -> Option { diff --git a/crates/vm/src/stdlib/_ctypes/base.rs b/crates/vm/src/stdlib/_ctypes/base.rs index c729c510a..065920e9b 100644 --- a/crates/vm/src/stdlib/_ctypes/base.rs +++ b/crates/vm/src/stdlib/_ctypes/base.rs @@ -7,6 +7,7 @@ use crate::class::StaticType; use crate::convert::ToPyObject; use crate::dict_inner::DictKey; use crate::function::{ArgBytesLike, ArgStrictInt, OptionalArg, PySetterValue}; +use crate::object::{Traverse, TraverseFn}; use crate::protocol::{BufferMethods, PyBuffer}; use crate::types::{Constructor, GetDescriptor, Representable}; use crate::{ @@ -159,6 +160,21 @@ impl Default for StgInfo { } } +// SAFETY: these are the only Python references in the type's storage metadata. +unsafe impl Traverse for StgInfo { + fn traverse(&self, tracer: &mut TraverseFn<'_>) { + self.proto.traverse(tracer); + self.element_type.traverse(tracer); + self.pointer_type.traverse(tracer); + } + + fn clear(&mut self, out: &mut Vec) { + out.extend(self.proto.take().map(Into::into)); + out.extend(self.element_type.take().map(Into::into)); + out.extend(self.pointer_type.take()); + } +} + impl StgInfo { pub(crate) fn new(size: usize, align: usize) -> Self { Self { @@ -247,10 +263,10 @@ impl StgInfo { /// Build the host_env call layout for a ctypes type from its already-borrowed /// `StgInfo`. Aggregate layouts come straight from the type's `field_layouts` /// (built incrementally from the base class, so struct inheritance is -/// reflected); array elements recurse into the element type; simple types read -/// their `_type_` code. The caller passes the borrowed `stg` so this never +/// reflected); array elements recurse into the element type; simple types use +/// their initialized scalar code. The caller passes the borrowed `stg` so this never /// re-locks `ty`'s own type data. -pub(super) fn type_layout(ty: &Py, stg: &StgInfo, vm: &VirtualMachine) -> CTypeLayout { +pub(super) fn type_layout(stg: &StgInfo) -> CTypeLayout { match stg.paramfunc { ParamFunc::Structure => CTypeLayout::Struct { fields: stg.field_layouts.clone(), @@ -264,7 +280,7 @@ pub(super) fn type_layout(ty: &Py, stg: &StgInfo, vm: &VirtualMachine) - let element = stg .element_type .as_ref() - .and_then(|et| et.stg_info_opt().map(|et_stg| type_layout(et, &et_stg, vm))) + .and_then(|et| et.stg_info_opt().map(|et_stg| type_layout(&et_stg))) .unwrap_or(CTypeLayout::Opaque { size: stg.element_size, }); @@ -275,9 +291,8 @@ pub(super) fn type_layout(ty: &Py, stg: &StgInfo, vm: &VirtualMachine) - } } ParamFunc::Pointer => CTypeLayout::Pointer, - ParamFunc::Simple | ParamFunc::None => ty - .type_code(vm) - .and_then(|code| code.chars().next()) + ParamFunc::Simple | ParamFunc::None => stg + .scalar_code .map_or(CTypeLayout::Opaque { size: stg.size }, CTypeLayout::Simple), } } @@ -303,7 +318,9 @@ pub(super) fn pointer_type_set( vm: &VirtualMachine, ) -> PyResult<()> { if let Some(mut info) = zelf.get_type_data_mut::() { - info.pointer_type = Some(value); + let old = info.pointer_type.replace(value); + drop(info); + drop(old); Ok(()) } else { Err(vm.new_attribute_error(format!("cannot set __pointer_type__ on {}", zelf.name()))) @@ -418,7 +435,7 @@ pub(super) fn str_to_wchar_bytes(s: &Wtf8, vm: &VirtualMachine) -> (PyObjectRef, } // PyCData - base type for all ctypes data types -#[pyclass(name = "_CData", module = "_ctypes")] +#[pyclass(name = "_CData", module = "_ctypes", traverse = "manual")] #[derive(Debug, PyPayload)] pub struct PyCData { /// Memory buffer - Owned (self-owned) or Borrowed (external reference) @@ -446,12 +463,63 @@ pub struct PyCData { kept_refs: PyRwLock>>, } +// SAFETY: visit each owned reference once; slot IDs are not additional owners. +unsafe impl Traverse for PyCData { + fn traverse(&self, tracer: &mut TraverseFn<'_>) { + self.base.traverse(tracer); + self.objects.traverse(tracer); + if let Some(refs) = self.kept_refs.try_read_recursive() + && let Some(refs) = refs.as_ref() + { + refs.traverse(tracer); + } + } + + fn clear(&mut self, out: &mut Vec) { + // Disconnect a view before its backing owners can be released. Owned + // storage keeps its allocation until deallocation. + if let Some(mut buffer) = self.buffer.try_write() { + match &mut *buffer { + Cow::Owned(_) => {} + Cow::Borrowed(_) => *buffer = Cow::Owned(Vec::new()), + } + } else { + return; + } + if let Some(mut base) = self.base.try_write() { + out.extend(base.take()); + } + if let Some(mut objects) = self.objects.try_write() { + out.extend(objects.take()); + } + if let Some(mut refs) = self.kept_refs.try_write() + && let Some(mut refs) = refs.take() + { + refs.clear(out); + } + } +} + #[derive(Debug, Default)] struct PrivateRefs { slots: std::collections::HashMap, rapidhash::quality::RandomState>, owners: std::collections::HashMap, } +// SAFETY: the owner map stores one strong reference per identity; slots hold IDs. +unsafe impl Traverse for PrivateRefs { + #[expect(clippy::iter_over_hash_type, reason = "GC edge order is irrelevant")] + fn traverse(&self, tracer: &mut TraverseFn<'_>) { + for (owner, _) in self.owners.values() { + owner.traverse(tracer); + } + } + fn clear(&mut self, out: &mut Vec) { + self.slots.clear(); + out.extend(self.owners.drain().map(|(_, (owner, _))| owner)); + } +} + impl PrivateRefs { fn replace(&mut self, key: String, owners: Vec) -> Vec { let old = if owners.is_empty() { @@ -1243,11 +1311,10 @@ impl PyCData { vm: &VirtualMachine, ) -> PyResult<()> { // Check if this is a c_char or c_wchar array field - let is_char_array = PyCField::is_char_array(proto, vm); - let is_wchar_array = PyCField::is_wchar_array(proto, vm); + let array_code = PyCField::array_type_code(proto); // For c_char arrays with bytes input, copy only up to first null - if is_char_array { + if array_code == Some('c') { if let Some(bytes_val) = value.downcast_ref::() { let src = bytes_val.as_bytes(); let to_copy = char_array_assignment_bytes(src); @@ -1260,7 +1327,7 @@ impl PyCData { } // For c_wchar arrays with str input, convert to wchar_t - if is_wchar_array { + if array_code == Some('u') { if let Some(str_val) = value.downcast_ref::() { let wchar_bytes = rustpython_host_env::ctypes::encode_wtf8_to_wchar_padded( str_val.as_wtf8(), @@ -1315,7 +1382,7 @@ impl PyCData { && stg.element_type.is_some() { // c_char array → return bytes - if PyCField::is_char_array(proto, vm) { + if PyCField::array_type_code(proto) == Some('c') { let data = &buffer[offset..offset + size]; return Ok(vm .ctx @@ -1324,7 +1391,7 @@ impl PyCData { } // c_wchar array → return str - if PyCField::is_wchar_array(proto, vm) { + if PyCField::array_type_code(proto) == Some('u') { let data = &buffer[offset..offset + size]; return Ok(vm.ctx.new_str(wchar_array_field_value(data)).into()); } @@ -1534,7 +1601,7 @@ impl Py { // PyCField - Field descriptor for Structure/Union types -#[pyclass(name = "CField", module = "ctypes")] +#[pyclass(name = "CField", module = "ctypes", traverse = "manual")] #[derive(Debug, PyPayload)] pub struct PyCField { /// Field name @@ -1555,6 +1622,13 @@ pub struct PyCField { pub(crate) bit_offset_val: u16, } +// SAFETY: proto is the descriptor's only owned Python reference. +unsafe impl Traverse for PyCField { + fn traverse(&self, tracer: &mut TraverseFn<'_>) { + self.proto.traverse(tracer); + } +} + impl PyCField { /// Create a new CField descriptor (non-bitfield) pub fn new( @@ -1598,6 +1672,30 @@ impl PyCField { } } + pub(super) fn checked_bitfield_size( + name: &str, + proto: &Py, + value: &PyObject, + vm: &VirtualMachine, + ) -> PyResult { + let bits = value.try_int(vm)?.as_bigint().to_u16(); + let info = proto.stg_info(vm)?; + let bits = bits + .filter(|&bits| bits > 0 && usize::from(bits) <= info.size * 8) + .ok_or_else(|| { + vm.new_value_error(format!("number of bits invalid for bit field '{name}'")) + })?; + if !matches!( + info.scalar_code, + Some('b' | 'B' | 'h' | 'H' | 'i' | 'I' | 'l' | 'L' | 'q' | 'Q' | '?') + ) { + return Err( + vm.new_type_error(format!("bit fields not allowed for type {}", proto.name())) + ); + } + Ok(bits) + } + /// Get the byte size of the field's underlying type pub fn get_byte_size(&self) -> usize { self.byte_size_val as usize @@ -1694,7 +1792,16 @@ impl Constructor for PyCField { .transpose()?; if let Some(bs) = bit_size_val { - if bs < 0 { + if !matches!( + field_type.stg_info(vm)?.scalar_code, + Some('b' | 'B' | 'h' | 'H' | 'i' | 'I' | 'l' | 'L' | 'q' | 'Q' | '?') + ) { + return Err(vm.new_type_error(format!( + "bit fields not allowed for type {}", + field_type.name() + ))); + } + if bs <= 0 { return Err(vm.new_value_error("number of bits invalid for bit field")); } let bo = bit_offset_val.unwrap_or(0); @@ -1702,7 +1809,7 @@ impl Constructor for PyCField { return Err(vm.new_value_error("bit_offset must be >= 0")); } let type_bits = byte_size * 8; - if bo + bs > type_bits { + if bs > type_bits || bo > type_bits - bs { return Err(vm.new_value_error(format!( "bit field '{name}' overflows its type ({bo} + {bs} > {type_bits})" ))); @@ -1768,6 +1875,28 @@ impl GetDescriptor for PyCField { // Get PyCData from obj (works for both Structure and Union) let cdata = Self::get_cdata_from_obj(&obj, vm)?; + if zelf.bitfield_size > 0 && zelf.proto.stg_info(vm)?.scalar_code != Some('?') { + let buffer = cdata.buffer.read(); + let data = buffer + .get(offset..offset.saturating_add(size)) + .ok_or_else(|| vm.new_value_error("field outside the buffer"))?; + let value = zelf.bitfield_word(data); + let bits = zelf.bitfield_size as u32; + let value = (value >> zelf.bit_offset_val) & (u64::MAX >> (64 - bits)); + let signed = matches!( + zelf.proto.stg_info(vm)?.scalar_code, + Some('b' | 'h' | 'i' | 'l' | 'q') + ); + drop(buffer); + return Ok(if signed { + vm.ctx + .new_int(((value << (64 - bits)) as i64) >> (64 - bits)) + .into() + } else { + vm.ctx.new_int(value).into() + }); + } + // PyCData_get cdata.get_field( zelf.proto.as_object(), @@ -1781,38 +1910,25 @@ impl GetDescriptor for PyCField { } impl PyCField { - /// Check if the field type is a c_char array (element type has _type_ == 'c') - fn is_char_array(proto: &PyObject, vm: &VirtualMachine) -> bool { - // Get element_type from StgInfo (for array types) - if let Some(proto_type) = proto.downcast_ref::() - && let Some(stg) = proto_type.stg_info_opt() - && let Some(element_type) = &stg.element_type + fn bitfield_word(&self, data: &[u8]) -> u64 { + let mut bytes = [0; 8]; + if self + .proto + .stg_info_opt() + .is_some_and(|info| info.big_endian) { - // Check if element type has _type_ == "c" - if let Ok(type_code) = element_type.as_object().get_attr("_type_", vm) - && let Some(s) = type_code.downcast_ref::() - { - return s.as_bytes() == b"c"; - } + bytes[8 - data.len()..].copy_from_slice(data); + u64::from_be_bytes(bytes) + } else { + bytes[..data.len()].copy_from_slice(data); + u64::from_le_bytes(bytes) } - false } - /// Check if the field type is a c_wchar array (element type has _type_ == 'u') - fn is_wchar_array(proto: &PyObject, vm: &VirtualMachine) -> bool { - // Get element_type from StgInfo (for array types) - if let Some(proto_type) = proto.downcast_ref::() - && let Some(stg) = proto_type.stg_info_opt() - && let Some(element_type) = &stg.element_type - { - // Check if element type has _type_ == "u" - if let Ok(type_code) = element_type.as_object().get_attr("_type_", vm) - && let Some(s) = type_code.downcast_ref::() - { - return s.as_bytes() == b"u"; - } - } - false + // Character-array fields use the element's initialized scalar converter. + fn array_type_code(proto: &PyObject) -> Option { + let info = proto.downcast_ref::()?.stg_info_opt()?; + info.element_type.as_ref()?.stg_info_opt()?.scalar_code } } @@ -1857,6 +1973,42 @@ impl Py { // Check if needs byte swapping let needs_swap = super::simple::is_swapped(&zelf.proto) && size > 1; + let code = zelf.proto.stg_info(vm)?.scalar_code; + if zelf.bitfield_size > 0 && code != Some('?') { + let code = code.ok_or_else(|| vm.new_type_error("invalid bitfield type"))?; + let mut encoded = [0; 4]; + // __index__ may reenter and change adjacent fields. Convert + // before locking, then merge into the current storage. + let (bytes, _, _) = super::simple::assignment_value( + code.encode_utf8(&mut encoded), + &value, + needs_swap, + vm, + )?; + let value = zelf.bitfield_word(&bytes); + let bits = zelf.bitfield_size as u32; + let mask = (u64::MAX >> (64 - bits)) << zelf.bit_offset_val; + let mut buffer = cdata.buffer.write(); + let data = buffer + .get(offset..offset.saturating_add(size)) + .ok_or_else(|| vm.new_value_error("field outside the buffer"))?; + let word = (zelf.bitfield_word(data) & !mask) + | ((value << zelf.bit_offset_val) & mask); + let big_endian = zelf.proto.stg_info(vm)?.big_endian; + let bytes = if big_endian { + word.to_be_bytes() + } else { + word.to_le_bytes() + }; + let data = if big_endian { + &bytes[8 - size..] + } else { + &bytes[..size] + }; + write_cow_bytes_at_offset(&mut buffer, offset, data); + return Ok(()); + } + // PyCData_set cdata.set_field( zelf.proto.as_object(), @@ -2125,11 +2277,7 @@ pub(super) fn bytes_to_pyobject( bytes: &[u8], vm: &VirtualMachine, ) -> PyObjectRef { - // Try to get _type_ attribute - if let Ok(type_attr) = cls.as_object().get_attr("_type_", vm) - && let Ok(s) = type_attr.str(vm) - { - let ty = s.to_string(); + if let Some(ty) = cls.type_code(vm) { return match rustpython_host_env::ctypes::decode_type_code(ty.as_str(), bytes) { rustpython_host_env::ctypes::DecodedValue::Bytes(value) => { vm.ctx.new_bytes(value).into() @@ -2195,15 +2343,16 @@ pub(super) fn is_simple_instance(typ: &Py) -> bool { /// Set or initialize StgInfo on a type pub(super) fn set_or_init_stginfo(type_ref: &PyType, stg_info: StgInfo) { - if type_ref.init_type_data(stg_info.clone()).is_err() + if type_ref.init_traced_type_data(stg_info.clone()).is_err() && let Some(mut existing) = type_ref.get_type_data_mut::() { // Preserve pointer_type cache across StgInfo replacement - let old_pointer_type = existing.pointer_type.take(); - *existing = stg_info; + let mut old = core::mem::replace(&mut *existing, stg_info); if existing.pointer_type.is_none() { - existing.pointer_type = old_pointer_type; + existing.pointer_type = old.pointer_type.take(); } + drop(existing); + drop(old); } } diff --git a/crates/vm/src/stdlib/_ctypes/function.rs b/crates/vm/src/stdlib/_ctypes/function.rs index 7137f22d5..6194820e6 100644 --- a/crates/vm/src/stdlib/_ctypes/function.rs +++ b/crates/vm/src/stdlib/_ctypes/function.rs @@ -1,6 +1,8 @@ // spell-checker:disable #![allow(unreachable_pub)] +use crate::object::{Traverse, TraverseFn}; + use super::{ _ctypes::CArgObject, PyCArray, PyCData, PyCPointer, PyCStructure, PyCUnion, StgInfo, @@ -320,7 +322,7 @@ impl ArgumentType for PyTypeRef { let bytes = cdata.buffer.read().to_vec(); let layout = self.stg_info_opt().map_or_else( || CTypeLayout::Opaque { size: bytes.len() }, - |stg| super::base::type_layout(self, &stg, vm), + |stg| super::base::type_layout(&stg), ); // Keep the converted instance alive through the call: the // snapshot may embed pointers into buffers its keep-alive set @@ -374,7 +376,7 @@ impl Initializer for PyCFuncPtrType { stg_info.flags |= StgInfoFlags::TYPEFLAG_ISPOINTER; stg_info.paramfunc = ParamFunc::Pointer; // CFuncPtr is passed as a pointer - let _ = new_type.init_type_data(stg_info); + let _ = new_type.init_traced_type_data(stg_info); Ok(()) } } @@ -416,7 +418,8 @@ impl PyCFuncPtrType { module = "_ctypes", name = "CFuncPtr", base = PyCData, - metaclass = "PyCFuncPtrType" + metaclass = "PyCFuncPtrType", + traverse = "manual" )] #[repr(C)] pub(super) struct PyCFuncPtr { @@ -447,6 +450,56 @@ pub(super) struct PyCFuncPtr { pub paramflags: PyRwLock>, } +// SAFETY: each stored reference is an independent strong edge, including +// thunk when it is also retained by _base.objects. +unsafe impl Traverse for PyCFuncPtr { + fn traverse(&self, tracer: &mut TraverseFn<'_>) { + self._base.traverse(tracer); + self.thunk.traverse(tracer); + self.callable.traverse(tracer); + self.converters.traverse(tracer); + self.argtypes.traverse(tracer); + self.restype.traverse(tracer); + self.checker.traverse(tracer); + self.errcheck.traverse(tracer); + self.paramflags.traverse(tracer); + #[cfg(windows)] + self.iid.traverse(tracer); + } + + fn clear(&mut self, out: &mut Vec) { + self._base.clear(out); + if let Some(mut value) = self.thunk.try_write() { + out.extend(value.take().map(Into::into)); + } + if let Some(mut value) = self.callable.try_write() { + out.extend(value.take()); + } + if let Some(mut value) = self.converters.try_write() { + out.extend(value.take()); + } + if let Some(mut value) = self.argtypes.try_write() { + out.extend(value.take()); + } + if let Some(mut value) = self.restype.try_write() { + out.extend(value.take()); + } + if let Some(mut value) = self.checker.try_write() { + out.extend(value.take()); + } + if let Some(mut value) = self.errcheck.try_write() { + out.extend(value.take()); + } + if let Some(mut value) = self.paramflags.try_write() { + out.extend(value.take()); + } + #[cfg(windows)] + if let Some(mut value) = self.iid.try_write() { + out.extend(value.take()); + } + } +} + impl Debug for PyCFuncPtr { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { f.debug_struct("PyCFuncPtr") @@ -1086,7 +1139,7 @@ fn compute_ret_spec( if let Some(stg_info) = restype_type.stg_info_opt() { return match stg_info.paramfunc { ParamFunc::Structure | ParamFunc::Union => { - RetSpec::Aggregate(super::base::type_layout(&restype_type, &stg_info, vm)) + RetSpec::Aggregate(super::base::type_layout(&stg_info)) } // Any other aggregate-ish StgInfo without a code: size-approximated // register return, as before. @@ -2070,7 +2123,12 @@ unsafe extern "C" fn thunk_callback( } // CThunkObject wraps a Python callable to make it callable from C code. -#[pyclass(name = "CThunkObject", module = "_ctypes")] +#[pyclass( + name = "CThunkObject", + module = "_ctypes", + traverse = "manual", + clear = false +)] #[derive(PyPayload)] pub(super) struct PyCThunk { callable: PyObjectRef, @@ -2079,6 +2137,22 @@ pub(super) struct PyCThunk { code_ptr: FfiCodePtr, } +// SAFETY: the thunk owns both callable references, and all type references in +// its userdata. No references are inferred from executable or native pointers. +unsafe impl Traverse for PyCThunk { + fn traverse(&self, tracer: &mut TraverseFn<'_>) { + self.callable.traverse(tracer); + if let Some(data) = self.thunk_data.try_read_recursive() + && let Some(data) = data.as_ref() + { + let data = data.userdata(); + data.callable.traverse(tracer); + data.arg_types.traverse(tracer); + data.res_type.traverse(tracer); + } + } +} + impl Debug for PyCThunk { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { f.debug_struct("PyCThunk") diff --git a/crates/vm/src/stdlib/_ctypes/pointer.rs b/crates/vm/src/stdlib/_ctypes/pointer.rs index badc2811b..d15b67f2a 100644 --- a/crates/vm/src/stdlib/_ctypes/pointer.rs +++ b/crates/vm/src/stdlib/_ctypes/pointer.rs @@ -7,14 +7,13 @@ use crate::protocol::{ use crate::types::{AsBuffer, AsMapping, AsNumber, AsSequence, Constructor, Initializer}; use crate::{ AsObject, Py, PyObject, PyObjectRef, PyPayload, PyResult, VirtualMachine, - builtins::{PyInt, PyList, PySlice, PyStr, PyType, PyTypeRef}, + builtins::{PyInt, PyList, PySlice, PyType, PyTypeRef}, function::{FuncArgs, OptionalArg}, }; use alloc::borrow::Cow; use num_traits::ToPrimitive; use rustpython_host_env::ctypes::{ - AddressValue, IntegerValue, pointer_item_address, read_pointer_char_slice, - read_pointer_wchar_slice, + pointer_item_address, read_pointer_char_slice, read_pointer_wchar_slice, }; #[pyclass(name = "PyCPointerType", base = PyType, module = "_ctypes")] @@ -73,7 +72,7 @@ impl Initializer for PyCPointerType { stg_info.format = Some(format!("&{shape_str}{current_format}")); } - let _ = new_type.init_type_data(stg_info); + let _ = new_type.init_traced_type_data(stg_info); // Cache: set target_type.__pointer_type__ = self (via StgInfo, not as inheritable attr) if let Ok(type_attr) = new_type.as_object().get_attr("_type_", vm) @@ -81,7 +80,9 @@ impl Initializer for PyCPointerType { && let Some(mut target_info) = target_type.get_type_data_mut::() { let zelf_obj: PyObjectRef = zelf.to_owned().into(); - target_info.pointer_type = Some(zelf_obj); + let old = target_info.pointer_type.replace(zelf_obj); + drop(target_info); + drop(old); } Ok(()) @@ -213,7 +214,9 @@ impl PyCPointerType { // 5. Cache: set target_type.__pointer_type__ = self (via StgInfo) if let Some(mut target_info) = typ_type.get_type_data_mut::() { - target_info.pointer_type = Some(zelf.into()); + let old = target_info.pointer_type.replace(zelf.into()); + drop(target_info); + drop(old); } Ok(()) @@ -231,12 +234,23 @@ impl AsSequence for PyCPointerType { name = "_Pointer", base = PyCData, metaclass = "PyCPointerType", - module = "_ctypes" + module = "_ctypes", + traverse = "manual" )] #[derive(Debug)] #[repr(transparent)] pub(crate) struct PyCPointer(pub PyCData); +// SAFETY: this transparent wrapper owns exactly the references in its base. +unsafe impl crate::object::Traverse for PyCPointer { + fn traverse(&self, tracer: &mut crate::object::TraverseFn<'_>) { + self.0.traverse(tracer); + } + fn clear(&mut self, out: &mut Vec) { + self.0.clear(out); + } +} + impl Constructor for PyCPointer { type Args = FuncArgs; @@ -396,34 +410,26 @@ impl PyCPointer { // Direct simple types and their generated endian variants return a // scalar; user subclasses and aggregates return a view. - if super::simple::returns_scalar(proto_type) - && let Ok(type_attr) = proto_type.as_object().get_attr("_type_", vm) - && let Some(type_str) = type_attr.downcast_ref::() - { - let type_code = type_str.to_string(); - if type_code == "O" || super::simple::is_swapped(proto_type) { - // SAFETY: as for ordinary pointer indexing below, the caller - // must supply a valid address covering the element's storage. - let bytes = unsafe { - rustpython_host_env::ctypes::borrow_memory(addr as *const u8, element_size) - }; - if type_code == "O" { - return zelf.0.object_at( - rustpython_host_env::ctypes::read_pointer_from_buffer(bytes), - Some(index as usize), - vm, - ); - } + if super::simple::returns_scalar(proto_type) { + // SAFETY: pointer indexing requires valid native storage covering + // the element. Ownership of arbitrary addresses remains the caller's + // responsibility, as for the view returned below. + let bytes = unsafe { + rustpython_host_env::ctypes::borrow_memory(addr as *const u8, element_size) + }; + if proto_type.stg_info(vm)?.scalar_code == Some('O') { + return zelf.0.object_at( + rustpython_host_env::ctypes::read_pointer_from_buffer(bytes), + Some(index as usize), + vm, + ); + } + if super::simple::is_swapped(proto_type) { let mut bytes = bytes.to_vec(); bytes.reverse(); return Ok(super::base::bytes_to_pyobject(proto_type, &bytes, vm)); } - return Ok(Self::read_value_at_address( - addr, - element_size, - Some(&type_code), - vm, - )); + return Ok(super::base::bytes_to_pyobject(proto_type, bytes, vm)); } // Complex type: create instance that references the memory directly (not a copy) @@ -507,12 +513,7 @@ impl PyCPointer { } else { rustpython_host_env::ctypes::pointer_size() }; - let type_code = stg_info - .proto - .as_ref() - .and_then(|p| p.as_object().get_attr("_type_", vm).ok()) - .and_then(|t| t.str(vm).ok()) - .map(|s| s.to_string()); + let type_code = stg_info.proto.as_ref().and_then(|p| p.type_code(vm)); let ptr_value = zelf.get_ptr_value(); @@ -593,23 +594,6 @@ impl PyCPointer { } prepared.keep(&zelf.0, index as usize, vm) } - - /// Read a value from memory address - fn read_value_at_address( - addr: usize, - size: usize, - type_code: Option<&str>, - vm: &VirtualMachine, - ) -> PyObjectRef { - match unsafe { rustpython_host_env::ctypes::read_value_at_address(addr, size, type_code) } { - AddressValue::ByteString(byte) => vm.ctx.new_bytes(vec![byte]).into(), - AddressValue::Integer(IntegerValue::Signed(value)) => vm.ctx.new_int(value).into(), - AddressValue::Integer(IntegerValue::Unsigned(value)) => vm.ctx.new_int(value).into(), - AddressValue::Float(value) => vm.ctx.new_float(value).into(), - AddressValue::Pointer(value) => vm.ctx.new_int(value).into(), - AddressValue::Bytes(bytes) => vm.ctx.new_bytes(bytes).into(), - } - } } impl AsNumber for PyCPointer { diff --git a/crates/vm/src/stdlib/_ctypes/simple.rs b/crates/vm/src/stdlib/_ctypes/simple.rs index c78fb81d4..0e08e5bdf 100644 --- a/crates/vm/src/stdlib/_ctypes/simple.rs +++ b/crates/vm/src/stdlib/_ctypes/simple.rs @@ -49,6 +49,15 @@ fn new_simple_type( Either::B(typ) => typ.as_object(), }; + if let Some(typ) = cls.downcast_ref::() + && let Some(info) = typ.stg_info_opt() + { + return Ok(PyCSimple(PyCData::from_bytes( + zeroed_bytes(info.size), + None, + ))); + } + let _type_ = cls .get_attr("_type_", vm) .map_err(|_| vm.new_attribute_error("class must define a '_type_' attribute"))?; @@ -690,11 +699,22 @@ fn create_swapped_types( module = "_ctypes", name = "_SimpleCData", base = PyCData, - metaclass = "PyCSimpleType" + metaclass = "PyCSimpleType", + traverse = "manual" )] #[repr(transparent)] pub(crate) struct PyCSimple(pub PyCData); +// SAFETY: this transparent wrapper owns exactly the references in its base. +unsafe impl crate::object::Traverse for PyCSimple { + fn traverse(&self, tracer: &mut crate::object::TraverseFn<'_>) { + self.0.traverse(tracer); + } + fn clear(&mut self, out: &mut Vec) { + self.0.clear(out); + } +} + impl Debug for PyCSimple { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { f.debug_struct("PyCSimple") @@ -1204,13 +1224,10 @@ impl Py { .downcast_ref() .ok_or_else(|| vm.new_type_error("cannot get value of instance"))?; - // Get _type_ from class let cls = zelf.class(); - let type_attr = cls - .as_object() - .get_attr("_type_", vm) - .map_err(|_| vm.new_type_error("no _type_ attribute"))?; - let type_code = type_attr.str(vm)?.to_string(); + let type_code = cls + .type_code(vm) + .ok_or_else(|| vm.new_type_error("abstract class"))?; // Special handling for c_char_p (z) and c_wchar_p (Z) // z_get, Z_get - dereference pointer to get string @@ -1266,13 +1283,10 @@ impl Py { .downcast() .map_err(|_| vm.new_type_error("cannot set value of instance"))?; - // Get _type_ from class let cls = zelf.class(); - let type_attr = cls - .as_object() - .get_attr("_type_", vm) - .map_err(|_| vm.new_type_error("no _type_ attribute"))?; - let type_code = type_attr.str(vm)?.to_string(); + let type_code = cls + .type_code(vm) + .ok_or_else(|| vm.new_type_error("abstract class"))?; // Handle z/Z types with PyBytes/PyStr separately to avoid memory leak if type_code == "z" { diff --git a/crates/vm/src/stdlib/_ctypes/structure.rs b/crates/vm/src/stdlib/_ctypes/structure.rs index 34d1ef584..d2f8cb943 100644 --- a/crates/vm/src/stdlib/_ctypes/structure.rs +++ b/crates/vm/src/stdlib/_ctypes/structure.rs @@ -8,7 +8,6 @@ use crate::types::{AsBuffer, AsSequence, Constructor, Initializer, SetAttr}; use crate::{AsObject, Py, PyObject, PyObjectRef, PyPayload, PyResult, VirtualMachine}; use alloc::borrow::Cow; use core::fmt::Debug; -use num_traits::ToPrimitive; /// Calculate Structure type size from _fields_ (sum of field sizes) pub(super) fn calculate_struct_size(cls: &Py, vm: &VirtualMachine) -> PyResult { @@ -52,7 +51,7 @@ impl Constructor for PyCStructType { // 4. Initialize StgInfo for the new type (initialized=false, to be set in init) let stg_info = StgInfo::default(); - let _ = new_type.init_type_data(stg_info); + let _ = new_type.init_traced_type_data(stg_info); // Note: _fields_ processing moved to Initializer::init() Ok(new_class) @@ -375,7 +374,7 @@ impl PyCStructType { has_bitfield = true; } // Collect the call layout for this field - field_layouts.push(super::base::type_layout(type_obj, &field_stg, vm)); + field_layouts.push(super::base::type_layout(&field_stg)); } // Mark field type as finalized (using type as field finalizes it) @@ -386,7 +385,7 @@ impl PyCStructType { // Create StgInfo with FINAL flag if it doesn't exist let mut stg_info = StgInfo::new(size, field_align); stg_info.flags |= StgInfoFlags::DICTFLAG_FINAL; - let _ = type_obj.init_type_data(stg_info); + let _ = type_obj.init_traced_type_data(stg_info); } } @@ -431,11 +430,8 @@ impl PyCStructType { // Check for bitfield size (optional 3rd element in tuple) let (c_field, field_advances_offset) = if field_tuple.as_slice().len() > 2 { let bit_size_obj = field_tuple.as_slice().get(2).expect("len checked"); - let bit_size = bit_size_obj - .try_int(vm)? - .as_bigint() - .to_u16() - .ok_or_else(|| vm.new_value_error("number of bits invalid for bit field"))?; + let bit_size = + PyCField::checked_bitfield_size(&name, &field_type_ref, bit_size_obj, vm)?; has_bitfield = true; let type_bits = (size * 8) as u16; @@ -470,6 +466,12 @@ impl PyCStructType { bitfield_bit_offset += bit_size; } + let bit_offset = if big_endian { + type_bits - bit_offset - bit_size + } else { + bit_offset + }; + // For packed bitfields that share offset, use the same offset as previous let field_offset = if !advances { offset - size // Reuse the previous field's offset @@ -645,11 +647,22 @@ impl SetAttr for PyCStructType { module = "_ctypes", name = "Structure", base = PyCData, - metaclass = "PyCStructType" + metaclass = "PyCStructType", + traverse = "manual" )] #[repr(transparent)] pub(crate) struct PyCStructure(pub PyCData); +// SAFETY: this transparent wrapper owns exactly the references in its base. +unsafe impl crate::object::Traverse for PyCStructure { + fn traverse(&self, tracer: &mut crate::object::TraverseFn<'_>) { + self.0.traverse(tracer); + } + fn clear(&mut self, out: &mut Vec) { + self.0.clear(out); + } +} + impl Debug for PyCStructure { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { f.debug_struct("PyCStructure") diff --git a/crates/vm/src/stdlib/_ctypes/union.rs b/crates/vm/src/stdlib/_ctypes/union.rs index a8817e48b..bbf65742f 100644 --- a/crates/vm/src/stdlib/_ctypes/union.rs +++ b/crates/vm/src/stdlib/_ctypes/union.rs @@ -8,7 +8,6 @@ use crate::stdlib::_warnings; use crate::types::{AsBuffer, AsSequence, Constructor, Initializer, SetAttr}; use crate::{AsObject, Py, PyObject, PyObjectRef, PyPayload, PyResult, VirtualMachine}; use alloc::borrow::Cow; -use num_traits::ToPrimitive; /// Calculate Union type size from _fields_ (max field size) pub(super) fn calculate_union_size(cls: &Py, vm: &VirtualMachine) -> PyResult { @@ -53,7 +52,7 @@ impl Constructor for PyCUnionType { // 4. Initialize StgInfo for the new type (initialized=false, to be set in init) let stg_info = StgInfo::default(); - let _ = new_type.init_type_data(stg_info); + let _ = new_type.init_traced_type_data(stg_info); // Note: _fields_ processing moved to Initializer::init() Ok(new_class) @@ -258,7 +257,7 @@ impl PyCUnionType { has_bitfield = true; } // Collect the call layout for this field - field_layouts.push(super::base::type_layout(type_obj, &field_stg, vm)); + field_layouts.push(super::base::type_layout(&field_stg)); } // Mark field type as finalized (using type as field finalizes it) @@ -269,7 +268,7 @@ impl PyCUnionType { // Create StgInfo with FINAL flag if it doesn't exist let mut stg_info = StgInfo::new(size, field_align); stg_info.flags |= StgInfoFlags::DICTFLAG_FINAL; - let _ = type_obj.init_type_data(stg_info); + let _ = type_obj.init_traced_type_data(stg_info); } } @@ -282,11 +281,8 @@ impl PyCUnionType { // For unions, each field starts fresh (CPython: _layout.py) let c_field = if field_tuple.as_slice().len() > 2 { let bit_size_obj = field_tuple.as_slice().get(2).expect("len checked"); - let bit_size = bit_size_obj - .try_int(vm)? - .as_bigint() - .to_u16() - .ok_or_else(|| vm.new_value_error("number of bits invalid for bit field"))?; + let bit_size = + PyCField::checked_bitfield_size(&name, &field_type_ref, bit_size_obj, vm)?; has_bitfield = true; // Union fields all start at offset 0, so bit_offset = 0 @@ -534,10 +530,26 @@ impl SetAttr for PyCUnionType { } } -#[pyclass(module = "_ctypes", name = "Union", base = PyCData, metaclass = "PyCUnionType")] +#[pyclass( + module = "_ctypes", + name = "Union", + base = PyCData, + metaclass = "PyCUnionType", + traverse = "manual" +)] #[repr(transparent)] pub(crate) struct PyCUnion(pub PyCData); +// SAFETY: this transparent wrapper owns exactly the references in its base. +unsafe impl crate::object::Traverse for PyCUnion { + fn traverse(&self, tracer: &mut crate::object::TraverseFn<'_>) { + self.0.traverse(tracer); + } + fn clear(&mut self, out: &mut Vec) { + self.0.clear(out); + } +} + impl core::fmt::Debug for PyCUnion { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { f.debug_struct("PyCUnion") diff --git a/crates/vm/src/types/slot.rs b/crates/vm/src/types/slot.rs index 6852cb658..1a1693b21 100644 --- a/crates/vm/src/types/slot.rs +++ b/crates/vm/src/types/slot.rs @@ -30,6 +30,8 @@ pub struct TypeDataSlot { // PyObject_GetTypeData type_id: TypeId, data: Box, + traverse: Option)>, + clear: Option)>, } impl TypeDataSlot { @@ -38,9 +40,19 @@ impl TypeDataSlot { Self { type_id: TypeId::of::(), data: Box::new(data), + traverse: None, + clear: None, } } + /// Create extension storage whose Python references participate in GC. + pub fn new_traced(data: T) -> Self { + let mut slot = Self::new(data); + slot.traverse = Some(|data, tracer| data.downcast_ref::().unwrap().traverse(tracer)); + slot.clear = Some(|data, out| data.downcast_mut::().unwrap().clear(out)); + slot + } + /// Get a reference to the data if the type matches. #[must_use] pub fn get(&self) -> Option<&T> { @@ -61,6 +73,20 @@ impl TypeDataSlot { } } +// SAFETY: callbacks are installed together with the matching concrete type. +unsafe impl crate::object::Traverse for TypeDataSlot { + fn traverse(&self, tracer: &mut crate::object::TraverseFn<'_>) { + if let Some(traverse) = self.traverse { + traverse(&*self.data, tracer); + } + } + fn clear(&mut self, out: &mut Vec) { + if let Some(clear) = self.clear { + clear(&mut *self.data, out); + } + } +} + /// Read guard for type data access, using mapped guard for zero-cost deref. pub struct TypeDataRef<'a, T: 'static> { guard: PyMappedRwLockReadGuard<'a, T>, diff --git a/extra_tests/snippets/stdlib_ctypes.py b/extra_tests/snippets/stdlib_ctypes.py index d99f47e3d..26630bc57 100644 --- a/extra_tests/snippets/stdlib_ctypes.py +++ b/extra_tests/snippets/stdlib_ctypes.py @@ -969,6 +969,22 @@ for _code in ("h", "q", "f"): MutableInt._type_ = _code _array[0] = -2 assert bytes(_array) == bytes(ctypes.c_int(-2)) + bytes(ctypes.c_int(0x1234)) + assert _array[0].value == -2 + assert MutableInt(-3).value == -3 + +for _scalar, _text_value in ((ctypes.c_char, b"ab"), (ctypes.c_wchar, "ab")): + + class MutableChar(_scalar): + pass + + class TextField(ctypes.Structure): + _fields_ = [("text", MutableChar * 4)] + + _text = TextField(_text_value) + MutableChar._type_ = "i" + assert _text.text == _text_value + _text.text = _text_value + assert _text.text == _text_value if _sys.platform == "win32": @@ -978,3 +994,16 @@ if _sys.platform == "win32": _bstr = (BSTR * 1)() _bstr[0] = None assert bytes(_bstr) == bytes(ctypes.sizeof(ctypes.c_void_p)) + + +# The py_object payload, not only the instance dictionary, closes this cycle. +class ObjectOwner: + pass + + +_owner = ObjectOwner() +_owner.value = ctypes.py_object(_owner) +_ref = weakref.ref(_owner) +del _owner +gc.collect() +assert _ref() is None