diff --git a/Lib/test/test_pyrepl/test_windows_console.py b/Lib/test/test_pyrepl/test_windows_console.py index 698dd75c2..32c4255aa 100644 --- a/Lib/test/test_pyrepl/test_windows_console.py +++ b/Lib/test/test_pyrepl/test_windows_console.py @@ -480,7 +480,6 @@ class WindowsConsoleGetEventTests(TestCase): self.assertEqual(self.get_event([ir]), None) self.assertEqual(self.mock.call_count, 1) - @unittest.expectedFailure # TODO: RUSTPYTHON def test_enter(self): ir = self.get_input_record("\r", self.VK_RETURN) self.assertEqual(self.get_event([ir]), Event("key", "\n")) @@ -492,13 +491,11 @@ class WindowsConsoleGetEventTests(TestCase): self.get_event([ir]), Event("key", "backspace")) self.assertEqual(self.mock.call_count, 1) - @unittest.expectedFailure # TODO: RUSTPYTHON def test_m(self): ir = self.get_input_record("m", self.VK_M) self.assertEqual(self.get_event([ir]), Event("key", "m")) self.assertEqual(self.mock.call_count, 1) - @unittest.expectedFailure # TODO: RUSTPYTHON def test_M(self): ir = self.get_input_record("M", self.VK_M, self.SHIFT_PRESSED) self.assertEqual(self.get_event([ir]), Event("key", "M")) @@ -554,7 +551,6 @@ class WindowsConsoleGetEventTests(TestCase): self.assertEqual(self.get_event([ir]), None) self.assertEqual(self.mock.call_count, 1) - @unittest.expectedFailure # TODO: RUSTPYTHON def test_m_LEFT_ALT_PRESSED(self): ir = self.get_input_record( "m", vcode=self.VK_M, control=self.LEFT_ALT_PRESSED) @@ -562,7 +558,6 @@ class WindowsConsoleGetEventTests(TestCase): self.assertEqual(self.console.get_event(), Event("key", "m")) self.assertEqual(self.mock.call_count, 1) - @unittest.expectedFailure # TODO: RUSTPYTHON def test_m_RIGHT_ALT_PRESSED(self): ir = self.get_input_record( "m", vcode=self.VK_M, control=self.RIGHT_ALT_PRESSED) @@ -570,7 +565,6 @@ class WindowsConsoleGetEventTests(TestCase): self.assertEqual(self.console.get_event(), Event("key", "m")) self.assertEqual(self.mock.call_count, 1) - @unittest.expectedFailure # TODO: RUSTPYTHON def test_AltGr_7(self): # E.g. on a German keyboard layout, '{' is entered via # AltGr + 7, where AltGr is the right Alt key on the keyboard. @@ -587,7 +581,6 @@ class WindowsConsoleGetEventTests(TestCase): self.assertEqual(self.get_event([ir]), Event("key", "{")) self.assertEqual(self.mock.call_count, 1) - @unittest.expectedFailure # TODO: RUSTPYTHON def test_AltGr_m(self): # E.g. on a German keyboard layout, this yields 'µ' # Let's use LEFT_ALT_PRESSED and RIGHT_CTRL_PRESSED this @@ -597,7 +590,6 @@ class WindowsConsoleGetEventTests(TestCase): self.assertEqual(self.get_event([ir]), Event("key", "µ")) self.assertEqual(self.mock.call_count, 1) - @unittest.expectedFailure # TODO: RUSTPYTHON def test_umlaut_a_german(self): ir = self.get_input_record("ä", self.VK_OEM_7) self.assertEqual(self.get_event([ir]), Event("key", "ä")) @@ -612,21 +604,18 @@ class WindowsConsoleGetEventTests(TestCase): # are sufficient to test here, the real tests happen in test_eventqueue # and test_keymap. - @unittest.expectedFailure # TODO: RUSTPYTHON def test_enter_vt(self): ir = self.get_input_record("\r") self.assertEqual(self.get_event([ir], vt_support=True), Event("key", "\n")) self.assertEqual(self.mock.call_count, 1) - @unittest.expectedFailure # TODO: RUSTPYTHON def test_backspace_vt(self): ir = self.get_input_record("\x7f") self.assertEqual(self.get_event([ir], vt_support=True), Event("key", "backspace", b"\x7f")) self.assertEqual(self.mock.call_count, 1) - @unittest.expectedFailure # TODO: RUSTPYTHON def test_up_vt(self): irs = [self.get_input_record(x) for x in "\x1b[A"] self.assertEqual(self.get_event(irs, vt_support=True), diff --git a/crates/host_env/src/ctypes.rs b/crates/host_env/src/ctypes.rs index 440d2078e..5c1a3f70b 100644 --- a/crates/host_env/src/ctypes.rs +++ b/crates/host_env/src/ctypes.rs @@ -1083,7 +1083,7 @@ pub fn simple_storage_value_to_bytes_endian( } _ => vec![0; 2], }, - "P" | "z" | "Z" => match value { + "P" | "z" | "Z" | "X" => match value { SimpleStorageValue::Pointer(value) => { uint_to_sized_bytes_endian(value as u64, pointer_size(), swapped) } @@ -2994,6 +2994,20 @@ pub fn dlsym_checked(_handle: usize, symbol_name: &CStr) -> Result<*mut c_void, mod tests { use super::*; + #[test] + fn bstr_storage_preserves_pointer_width() { + for address in [0, usize::MAX] { + assert_eq!( + simple_storage_value_to_bytes_endian( + "X", + SimpleStorageValue::Pointer(address), + false, + ), + address.to_ne_bytes(), + ); + } + } + #[test] fn simple_type_is_pointer_classifies_codes() { assert!(simple_type_is_pointer("z")); diff --git a/crates/vm/src/stdlib/_ctypes/array.rs b/crates/vm/src/stdlib/_ctypes/array.rs index 124020bd6..386975719 100644 --- a/crates/vm/src/stdlib/_ctypes/array.rs +++ b/crates/vm/src/stdlib/_ctypes/array.rs @@ -17,10 +17,9 @@ use alloc::borrow::Cow; use num_traits::{Signed, ToPrimitive}; use rustpython_host_env::ctypes::{ - ArrayElementWriteValue, DecodedValue, WCHAR_SIZE, WCharArrayWriteError, char_array_field_value, - int_to_sized_bytes, read_array_element, simple_type_size, uint_to_sized_bytes, - wchar_from_bytes, write_array_element, write_char_array_raw, write_char_array_value, - write_wchar_array_value, wstring_from_bytes, zeroed_bytes, + DecodedValue, WCHAR_SIZE, WCharArrayWriteError, char_array_field_value, read_array_element, + simple_type_size, wchar_from_bytes, write_char_array_raw, write_char_array_value, + write_wchar_array_value, wstring_from_bytes, }; fn cow_bytes_as_mut<'buf>(buffer: &'buf mut Cow<'_, [u8]>) -> &'buf mut [u8] { @@ -508,33 +507,6 @@ impl PyCArray { PyGenericAlias::from_args(cls, object, vm) } - fn int_to_bytes(i: &malachite_bigint::BigInt, size: usize) -> Vec { - // Try unsigned first (handles values like 0xFFFFFFFF that overflow signed) - // then fall back to signed (handles negative values) - match size { - 1 => { - if let Some(v) = i.to_u8() { - vec![v] - } else { - vec![i.to_i8().unwrap_or(0) as u8] - } - } - 2 => i.to_u16().map_or_else( - || int_to_sized_bytes(i.to_i16().unwrap_or(0).into(), 2), - |v| uint_to_sized_bytes(v.into(), 2), - ), - 4 => i.to_u32().map_or_else( - || int_to_sized_bytes(i.to_i32().unwrap_or(0).into(), 4), - |v| uint_to_sized_bytes(v.into(), 4), - ), - 8 => i.to_u64().map_or_else( - || int_to_sized_bytes(i.to_i64().unwrap_or(0), 8), - |v| uint_to_sized_bytes(v, 8), - ), - _ => zeroed_bytes(size), - } - } - fn getitem_by_index(zelf: &Py, i: isize, vm: &VirtualMachine) -> PyResult { let stg = zelf.class().stg_info_opt(); let length = stg.as_ref().map_or(0, |i| i.length) as isize; @@ -616,231 +588,39 @@ impl PyCArray { } } - /// Helper to write an element value to a buffer at given offset - fn write_element_to_buffer( - offset: usize, - element_size: usize, - type_code: Option<&str>, - value: &PyObject, - zelf: &Py, - index: usize, - vm: &VirtualMachine, - ) -> PyResult<()> { - // Conversions and reference replacement may call Python. Only the raw - // storage write runs under the buffer lock. - let write = |value| { - let mut buffer = zelf.0.buffer.write(); - write_array_element(cow_bytes_as_mut(&mut buffer), offset, value); - }; - match type_code { - Some("c") => { - if let Some(b) = value.downcast_ref::() { - write(ArrayElementWriteValue::Byte( - b.as_bytes().first().copied().unwrap_or(0), - )); - } else if let Ok(int_val) = value.try_int(vm) { - write(ArrayElementWriteValue::Byte( - int_val.as_bigint().to_u8().unwrap_or(0), - )); - } else { - return Err(vm.new_type_error("an integer or bytes of length 1 is required")); - } - } - Some("u") => { - if let Some(s) = value.downcast_ref::() { - let code = s.as_wtf8().code_points().next().map_or(0, |c| c.to_u32()); - write(ArrayElementWriteValue::Wchar(code)); - } else { - return Err(vm.new_type_error("unicode string expected")); - } - } - Some("z") => { - let mut hidden = None; - let (ptr_val, converted) = if value.is(&vm.ctx.none) { - (0usize, None) - } else if let Some(bytes) = value.downcast_ref::() { - let (kept_alive, ptr) = super::base::ensure_z_null_terminated(bytes, vm); - hidden = Some(kept_alive); - (ptr, Some(value.to_owned())) - } else if let Some(simple) = value.downcast_ref::() - && value.class().type_code(vm).as_deref() == Some("z") - { - let buffer = simple.0.buffer.read(); - ( - rustpython_host_env::ctypes::read_pointer_from_buffer(&buffer), - None, - ) - } else if let Ok(int_val) = value.try_index(vm) { - (int_val.as_bigint().to_usize().unwrap_or(0), None) - } else { - return Err(vm.new_type_error(format!( - "bytes or integer address expected instead of {} instance", - value.class().name() - ))); - }; - write(ArrayElementWriteValue::Pointer { - value: ptr_val, - size: element_size, - }); - if let Some(c) = converted { - return zelf.0.keep_ref_with_hidden(index, c, hidden, vm); - } - } - Some("Z") => { - let (ptr_val, converted) = if value.is(&vm.ctx.none) { - (0usize, None) - } else if let Some(s) = value.downcast_ref::() { - let (holder, ptr) = super::base::str_to_wchar_bytes(s.as_wtf8(), vm); - (ptr, Some(holder)) - } else if let Some(simple) = value.downcast_ref::() - && value.class().type_code(vm).as_deref() == Some("Z") - { - let buffer = simple.0.buffer.read(); - ( - rustpython_host_env::ctypes::read_pointer_from_buffer(&buffer), - None, - ) - } else if let Ok(int_val) = value.try_index(vm) { - (int_val.as_bigint().to_usize().unwrap_or(0), None) - } else { - return Err(vm.new_type_error("unicode string or integer address expected")); - }; - write(ArrayElementWriteValue::Pointer { - value: ptr_val, - size: element_size, - }); - if let Some(c) = converted { - return zelf.0.keep_ref(index, c, vm); - } - } - Some("O") => { - // A py_object instance contributes its stored reference, while - // other Python objects are the value itself. - if value.downcast_ref::().is_none() { - write(ArrayElementWriteValue::Pointer { - value: value.get_id(), - size: element_size, - }); - return zelf.0.keep_ref(index, value.to_owned(), vm); - } - let stg = zelf.class().stg_info(vm)?; - let element_type = stg.element_type.as_ref().unwrap(); - if !value.fast_isinstance(element_type) { - return Err(vm.new_type_error(format!( - "expected {} instance, not {}", - element_type.name(), - value.class().name() - ))); - } - let cdata = value.downcast_ref::().unwrap(); - let bytes = cdata.buffer.read().to_vec(); - write(ArrayElementWriteValue::Bytes { - bytes: &bytes, - size: element_size, - }); - } - Some("f") => { - // c_float: convert int/float to f32 bytes - let f32_val = if let Ok(float_val) = value.try_float(vm) { - float_val.to_f64() as f32 - } else if let Ok(int_val) = value.try_int(vm) { - int_val.as_bigint().to_f64().unwrap_or(0.0) as f32 - } else { - return Err(vm.new_type_error("a float is required")); - }; - write(ArrayElementWriteValue::Float { - value: f32_val.into(), - size: 4, - }); - } - Some("d" | "g") => { - // c_double / c_longdouble: convert int/float to f64 bytes - let f64_val = if let Ok(float_val) = value.try_float(vm) { - float_val.to_f64() - } else if let Ok(int_val) = value.try_int(vm) { - int_val.as_bigint().to_f64().unwrap_or(0.0) - } else { - return Err(vm.new_type_error("a float is required")); - }; - write(ArrayElementWriteValue::Float { - value: f64_val, - size: 8, - }); - // For "g" type, remaining bytes stay zero - } - _ => { - // Handle ctypes instances (copy their buffer) - if let Some(cdata) = value.downcast_ref::() { - let src_buffer = cdata.buffer.read().to_vec(); - write(ArrayElementWriteValue::Bytes { - bytes: &src_buffer, - size: element_size, - }); - // Other types: use int_to_bytes - } else if let Ok(int_val) = value.try_int(vm) { - let bytes = Self::int_to_bytes(int_val.as_bigint(), element_size); - write(ArrayElementWriteValue::Bytes { - bytes: &bytes, - size: element_size, - }); - } else { - return Err(vm.new_type_error(format!( - "expected {} instance, not {}", - type_code.unwrap_or("value"), - value.class().name() - ))); - } - } - } - - // KeepRef - if super::base::PyCData::should_keep_ref(value) { - zelf.0.keep_cdata(index, value, vm)?; - } - - Ok(()) - } - fn setitem_by_index( zelf: &Py, i: isize, value: &PyObject, vm: &VirtualMachine, ) -> PyResult<()> { - let stg = zelf.class().stg_info_opt(); - let length = stg.as_ref().map_or(0, |i| i.length) as isize; + let (length, element_size, element_type) = { + let stg = zelf.class().stg_info(vm)?; + ( + stg.length as isize, + stg.element_size, + stg.element_type + .clone() + .ok_or_else(|| vm.new_type_error("no element type"))?, + ) + }; let index = if i < 0 { length + i } else { i }; if index < 0 || index >= length { return Err(vm.new_index_error("invalid index")); } let index = index as usize; - let element_size = stg.as_ref().map_or(0, |i| i.element_size); - let offset = index * element_size; - let type_code = Self::get_element_type_code(zelf, vm); - - if let Some(element_type) = stg.as_ref().and_then(|info| info.element_type.as_ref()) - && super::simple::is_swapped(element_type) - && value.downcast_ref::().is_none() - { - let type_code = type_code - .as_deref() - .ok_or_else(|| vm.new_type_error("no _type_ attribute"))?; - let bytes = super::simple::swapped_value_bytes(type_code, value, vm)?; - zelf.0.write_bytes_at_offset(offset, &bytes); - return Ok(()); - } - + let prepared = super::base::CDataValue::new( + &element_type, + value, + element_size, + super::simple::is_swapped(&element_type), + vm, + )?; // A view's buffer already points to its element. Its base can be a // pointer object, whose own buffer stores an address rather than data. - Self::write_element_to_buffer( - offset, - element_size, - type_code.as_deref(), - value, - zelf, - index, - vm, - ) + zelf.0 + .write_bytes_at_offset(index * element_size, &prepared.bytes); + prepared.keep(&zelf.0, index, vm) } // Array_subscript diff --git a/crates/vm/src/stdlib/_ctypes/base.rs b/crates/vm/src/stdlib/_ctypes/base.rs index 6375705c6..09a44dc34 100644 --- a/crates/vm/src/stdlib/_ctypes/base.rs +++ b/crates/vm/src/stdlib/_ctypes/base.rs @@ -85,6 +85,8 @@ pub struct StgInfo { pub length: usize, // number of fields (for arrays/structures) pub proto: Option, // Only for Pointer/ArrayObject pub flags: StgInfoFlags, // type flags (TYPEFLAG_*, DICTFLAG_*) + // Scalar assignment uses the converter selected at type initialization. + pub(super) scalar_code: Option, // Array-specific fields pub element_type: Option, // _type_ for arrays @@ -123,6 +125,7 @@ impl core::fmt::Debug for StgInfo { .field("length", &self.length) .field("proto", &self.proto) .field("flags", &self.flags) + .field("scalar_code", &self.scalar_code) .field("element_type", &self.element_type) .field("element_size", &self.element_size) .field("format", &self.format) @@ -143,6 +146,7 @@ impl Default for StgInfo { length: 0, proto: None, flags: StgInfoFlags::empty(), + scalar_code: None, element_type: None, element_size: 0, format: None, @@ -164,6 +168,7 @@ impl StgInfo { length: 0, proto: None, flags: StgInfoFlags::empty(), + scalar_code: None, element_type: None, element_size: 0, format: None, @@ -216,6 +221,7 @@ impl StgInfo { length, proto: None, flags, + scalar_code: None, element_type: Some(element_type), element_size, format, @@ -437,7 +443,43 @@ pub struct PyCData { /// Used for null-terminated c_char_p buffer copies, since /// RustPython's PyBytes lacks CPython's internal trailing null. /// Keyed by unique_key (hierarchical) so nested fields don't collide. - pub(super) kept_refs: PyRwLock>, + kept_refs: PyRwLock>>, +} + +#[derive(Debug, Default)] +struct PrivateRefs { + slots: std::collections::HashMap, rapidhash::quality::RandomState>, + owners: std::collections::HashMap, +} + +impl PrivateRefs { + fn replace(&mut self, key: String, owners: Vec) -> Vec { + let old = if owners.is_empty() { + self.slots.remove(&key) + } else { + self.slots + .insert(key, owners.iter().map(|obj| obj.get_id()).collect()) + }; + // Count each slot's references, but retain each Python owner only once. + // Add first so replacing an owner with itself cannot release it. + for obj in owners { + self.owners.entry(obj.get_id()).or_insert((obj, 0)).1 += 1; + } + let mut displaced = Vec::new(); + for id in old.into_iter().flatten() { + let owner = self.owners.get_mut(&id).expect("registered private owner"); + owner.1 -= 1; + if owner.1 == 0 { + displaced.push(self.owners.remove(&id).unwrap().0); + } + } + // The caller releases these after unlocking and publishing _objects. + displaced + } + + fn snapshot(&self) -> Vec { + self.owners.values().map(|(obj, _)| obj.clone()).collect() + } } // Retention metadata is user-mutable. Its fast lookup must not call Python @@ -470,6 +512,178 @@ impl DictKey for ObjectKey<'_> { } } +// Prepare conversions and ownership before writing. Python conversion hooks +// and old-owner finalizers must never run with a storage or metadata lock held. +pub(super) enum CDataBytes { + Integer { bytes: [u8; 8], size: usize }, + Owned(Vec), +} + +impl From> for CDataBytes { + fn from(bytes: Vec) -> Self { + Self::Owned(bytes) + } +} + +impl core::ops::Deref for CDataBytes { + type Target = [u8]; + + fn deref(&self) -> &[u8] { + match self { + Self::Integer { bytes, size } => &bytes[..*size], + Self::Owned(bytes) => bytes, + } + } +} + +pub(super) struct CDataValue { + pub bytes: CDataBytes, + keep: Option, + hidden: Vec, + temporary: Option, +} + +impl CDataValue { + pub(super) fn new( + proto: &Py, + value: &PyObject, + size: usize, + swapped: bool, + vm: &VirtualMachine, + ) -> PyResult { + if let Some(source) = value.downcast_ref::() { + if value.fast_isinstance(proto) { + // Snapshot before writing: source and destination may overlap. + let bytes = + rustpython_host_env::ctypes::copy_to_sized_bytes(&source.buffer.read(), size); + let hidden = PyCData::hidden_refs(value); + let keep = PyCData::get_kept_objects(value, vm); + return Ok(Self { + bytes: bytes.into(), + keep: (!vm.is_none(&keep)).then_some(keep), + hidden, + temporary: None, + }); + } + if proto + .class() + .fast_issubclass(super::pointer::PyCPointerType::static_type()) + && let Some(array) = value.downcast_ref::() + && { + let target = proto.stg_info(vm)?; + let array_info = array.class().stg_info(vm)?; + target + .proto + .as_deref() + .zip(array_info.element_type.as_deref()) + .is_some_and(|(target, element)| target.is(element)) + } + { + let address = array.0.buffer.read().as_ptr() as usize; + return Ok(Self { + bytes: rustpython_host_env::ctypes::pointer_to_sized_bytes(address, size) + .into(), + keep: Some( + vm.ctx + .new_tuple(vec![PyCData::get_kept_objects(value, vm), value.to_owned()]) + .into(), + ), + hidden: Vec::new(), + temporary: None, + }); + } + return Err(vm.new_type_error(format!( + "incompatible types, {} instance instead of {} instance", + value.class().name(), + proto.name() + ))); + } + let metaclass = proto.class(); + let simple_metaclass = super::simple::PyCSimpleType::static_type(); + if metaclass.is(simple_metaclass) || metaclass.fast_issubclass(simple_metaclass) { + // Reading the mutable _type_ here could select a wider converter + // than the allocated slot, or change conversion during callbacks. + let code = proto + .stg_info(vm)? + .scalar_code + .ok_or_else(|| vm.new_type_error("abstract class"))?; + let mut encoded = [0; 4]; + let type_code = code.encode_utf8(&mut encoded); + let (bytes, keep, hidden) = + super::simple::assignment_value(type_code, value, swapped, vm)?; + return Ok(Self { + bytes, + keep, + hidden: hidden.into_iter().collect(), + temporary: None, + }); + } + if let Some(tuple) = value.downcast_ref::() { + let instance = proto + .as_object() + .call(tuple.as_slice().to_vec(), vm) + .map_err(|e| { + let args = e.args(); + let message = args + .as_slice() + .first() + .and_then(|arg| arg.downcast_ref::()) + .map(|s| s.to_string()) + .unwrap_or_default(); + vm.new_runtime_error(format!( + "({}) {}: {message}", + proto.name(), + e.class().name() + )) + })?; + let mut prepared = Self::new(proto, &instance, size, swapped, vm)?; + prepared.temporary = Some(instance); + return Ok(prepared); + } + if vm.is_none(value) + && proto + .class() + .fast_issubclass(super::pointer::PyCPointerType::static_type()) + { + return Ok(Self { + bytes: vec![0; size].into(), + keep: None, + hidden: Vec::new(), + temporary: None, + }); + } + Err(vm.new_type_error(format!( + "expected {} instance, not {}", + proto.name(), + value.class().name() + ))) + } + + #[inline] + pub(super) fn keep( + self, + destination: &PyCData, + index: usize, + vm: &VirtualMachine, + ) -> PyResult<()> { + // Tuple conversion can create an instance with a finalizer. Keep it + // until the bytes and all owners have been published. + let _temporary = self.temporary; + // Like KeepRef(None), preserve previous owners when assigning a raw + // address: it can still point into their private backing allocation. + if self.keep.is_none() && self.hidden.is_empty() { + return Ok(()); + } + // Publish every owner before dropping the displaced value, whose + // finalizer may assign this same slot again. + let _old_hidden = destination.replace_hidden(index, self.hidden); + if let Some(keep) = self.keep { + destination.keep_ref(index, keep, vm)?; + } + Ok(()) + } +} + impl PyCData { /// Create from StgInfo (PyCData_MallocBuffer pattern) pub(crate) fn from_stg_info(stg_info: &StgInfo) -> Self { @@ -480,7 +694,7 @@ impl PyCData { index: AtomicCell::new(0), objects: PyRwLock::new(None), length: AtomicCell::new(stg_info.length), - kept_refs: PyRwLock::new(std::collections::HashMap::new()), + kept_refs: PyRwLock::new(None), } } @@ -493,7 +707,7 @@ impl PyCData { index: AtomicCell::new(0), objects: PyRwLock::new(objects), length: AtomicCell::new(0), - kept_refs: PyRwLock::new(std::collections::HashMap::new()), + kept_refs: PyRwLock::new(None), } } @@ -510,7 +724,7 @@ impl PyCData { index: AtomicCell::new(0), objects: PyRwLock::new(objects), length: AtomicCell::new(length), - kept_refs: PyRwLock::new(std::collections::HashMap::new()), + kept_refs: PyRwLock::new(None), } } @@ -531,7 +745,7 @@ impl PyCData { index: AtomicCell::new(0), objects: PyRwLock::new(None), length: AtomicCell::new(0), - kept_refs: PyRwLock::new(std::collections::HashMap::new()), + kept_refs: PyRwLock::new(None), } } @@ -560,7 +774,7 @@ impl PyCData { index: AtomicCell::new(idx), objects: PyRwLock::new(None), length: AtomicCell::new(0), - kept_refs: PyRwLock::new(std::collections::HashMap::new()), + kept_refs: PyRwLock::new(None), } } @@ -595,7 +809,7 @@ impl PyCData { index: AtomicCell::new(0), objects: PyRwLock::new(Some(objects_dict.into())), length: AtomicCell::new(length), - kept_refs: PyRwLock::new(std::collections::HashMap::new()), + kept_refs: PyRwLock::new(None), } } @@ -813,14 +1027,12 @@ impl PyCData { } /// Keep a reference alive without exposing it in _objects. - /// Walks up to root object (same as keep_ref) so the reference - /// lives as long as the owning ctypes object. - /// Uses unique_key (hierarchical) so nested fields don't collide. - pub fn keep_alive(&self, index: usize, obj: PyObjectRef) { - drop(self.replace_hidden(index, Some(obj))); + /// Retains a backing allocation used by a scalar's pointer. + pub fn keep_alive(&self, obj: PyObjectRef) { + drop(self.replace_hidden(0, vec![obj])); } - fn replace_hidden(&self, index: usize, obj: Option) -> Option { + fn replace_hidden(&self, index: usize, owners: Vec) -> Vec { let key = self.unique_key(index); let base = self.base.read().clone(); let root = base.as_ref().map(|base| Self::find_root_object(base)); @@ -829,24 +1041,11 @@ impl PyCData { .and_then(|root| root.downcast_ref::()) .map_or(self, |cdata| cdata); let mut refs = cdata.kept_refs.write(); - match obj { - Some(obj) => refs.insert(key, obj), - None => refs.remove(&key), + if refs.is_none() && owners.is_empty() { + return Vec::new(); } - } - - pub(super) fn keep_ref_with_hidden( - &self, - index: usize, - keep: PyObjectRef, - hidden: Option, - vm: &VirtualMachine, - ) -> PyResult<()> { - // Publish all owners before dropping the old visible reference: its - // finalizer may overwrite this same slot. Keep old storage alive until - // then, and do not overwrite the finalizer's owners afterwards. - let _old_hidden = self.replace_hidden(index, hidden); - self.keep_ref(index, keep, vm) + refs.get_or_insert_with(Default::default) + .replace(key, owners) } /// Find the root object (one without a base) by walking up the base chain @@ -906,30 +1105,34 @@ impl PyCData { Ok(()) } - /// Get kept objects from a CData instance - /// Returns the _objects of the root CData, or an empty dict if None. + /// Get kept objects from a CData instance, initializing its root container. pub fn get_kept_objects(value: &PyObject, vm: &VirtualMachine) -> PyObjectRef { - Self::find_root_object(value) - .downcast_ref::() - .and_then(|cdata| cdata.objects.read().clone()) - .unwrap_or_else(|| vm.ctx.new_dict().into()) + let root = Self::find_root_object(value); + let Some(cdata) = root.downcast_ref::() else { + return vm.ctx.none(); + }; + cdata + .objects + .write() + .get_or_insert_with(|| { + if cdata.length.load() == 0 { + vm.ctx.none() + } else { + vm.ctx.new_dict().into() + } + }) + .clone() } // Copying a view must retain its root's references, including private - // backing allocations used for null-terminated strings. - pub(super) fn keep_cdata( - &self, - index: usize, - value: &PyObject, - vm: &VirtualMachine, - ) -> PyResult<()> { + // backing allocations and aliases through other fields or pointers. + // Keep the snapshot flat and deduplicate owners: self-copy must not nest + // a new holder around all previous snapshots. + fn hidden_refs(value: &PyObject) -> Vec { let root = Self::find_root_object(value); - let hidden: Vec<_> = root - .downcast_ref::() - .map(|cdata| cdata.kept_refs.read().values().cloned().collect()) - .unwrap_or_default(); - let hidden = (!hidden.is_empty()).then(|| vm.ctx.new_tuple(hidden).into()); - self.keep_ref_with_hidden(index, Self::get_kept_objects(&root, vm), hidden, vm) + root.downcast_ref::() + .and_then(|cdata| cdata.kept_refs.read().as_ref().map(|refs| refs.snapshot())) + .unwrap_or_default() } // A scalar owns one reference; a scalar view stores it in its root instead. @@ -939,7 +1142,7 @@ impl PyCData { hidden: Option, vm: &VirtualMachine, ) -> PyResult<()> { - let _old_hidden = self.replace_hidden(0, hidden); + let _old_hidden = self.replace_hidden(0, hidden.into_iter().collect()); if self.base.read().is_some() { self.keep_ref(0, value, vm) } else { @@ -998,7 +1201,9 @@ impl PyCData { // through other keys or containers. Search those only on a direct miss. let mut pending: Vec<_> = objects.into_iter().collect(); pending.extend(base); - pending.extend(self.kept_refs.read().values().cloned()); + if let Some(refs) = self.kept_refs.read().as_ref() { + pending.extend(refs.snapshot()); + } let mut visited = std::collections::HashSet::new(); while let Some(obj) = pending.pop() { if obj.get_id() == address { @@ -1010,7 +1215,9 @@ impl PyCData { if let Some(cdata) = obj.downcast_ref::() { pending.extend(cdata.objects.read().iter().cloned()); pending.extend(cdata.base.read().iter().cloned()); - pending.extend(cdata.kept_refs.read().values().cloned()); + if let Some(refs) = cdata.kept_refs.read().as_ref() { + pending.extend(refs.snapshot()); + } } else if let Some(dict) = obj.downcast_ref::() { pending.extend(dict.values_vec()); } else if let Some(tuple) = obj.downcast_ref::() { @@ -1022,12 +1229,6 @@ impl PyCData { Err(vm.new_value_error("PyObject is NULL")) } - /// Check if a value should be stored in _objects - /// Returns true for ctypes objects and bytes (for c_char_p) - pub(crate) fn should_keep_ref(value: &PyObject) -> bool { - value.downcast_ref::().is_some() || value.downcast_ref::().is_some() - } - /// PyCData_set /// Sets a field value at the given offset, handling type conversion and KeepRef #[allow(clippy::too_many_arguments)] @@ -1073,134 +1274,12 @@ impl PyCData { } } - // A compatible ctypes instance already contains the encoded field bytes. - // Snapshot them before writing: source and destination may share storage. - if let Some(proto_type) = proto.downcast_ref::() - && value.class().fast_issubclass(proto_type) - && let Some(source) = value.downcast_ref::() - { - let bytes = { - let buffer = source.buffer.read(); - rustpython_host_env::ctypes::copy_to_sized_bytes(&buffer, size) - }; - self.write_bytes_at_offset(offset, &bytes); - return self.keep_cdata(index, &value, vm); - } - - // Special handling for Pointer fields with Array values - if let Some(proto_type) = proto.downcast_ref::() - && proto_type - .class() - .fast_issubclass(super::pointer::PyCPointerType::static_type()) - && let Some(array) = value.downcast_ref::() - && { - let target_info = proto_type.stg_info(vm)?; - let array_info = array.class().stg_info(vm)?; - target_info - .proto - .as_deref() - .zip(array_info.element_type.as_deref()) - .is_some_and(|(target, element)| target.is(element)) - } - { - let buffer_addr = { - let array_buffer = array.0.buffer.read(); - array_buffer.as_ptr() as usize - }; - let addr_bytes = rustpython_host_env::ctypes::pointer_to_sized_bytes(buffer_addr, size); - self.write_bytes_at_offset(offset, &addr_bytes); - self.keep_ref(index, value, vm)?; - return Ok(()); - } - - if value.downcast_ref::().is_some() { - return Err(vm.new_type_error(format!( - "incompatible types, {} instance instead of {} instance", - value.class().name(), - proto - .downcast_ref::() - .ok_or_else(|| vm.new_type_error("field proto is not a type"))? - .name() - ))); - } - - // For array fields with tuple/list input, instantiate the array type - // and unpack elements as positional args (Array_init expects *args) - if let Some(proto_type) = proto.downcast_ref::() - && let Some(stg) = proto_type.stg_info_opt() - && stg.element_type.is_some() - { - let items: Option> = - if let Some(tuple) = value.downcast_ref::() { - Some(tuple.as_slice().to_vec()) - } else { - value - .downcast_ref::() - .map(|list| list.borrow_vec().to_vec()) - }; - if let Some(items) = items { - let array_obj = proto_type.as_object().call(items, vm).map_err(|e| { - // Wrap errors in RuntimeError with type name prefix - let type_name = proto_type.name().to_string(); - let exc_name = e.class().name().to_string(); - let exc_args = e.args(); - let exc_msg = exc_args - .as_slice() - .first() - .and_then(|a| a.downcast_ref::().map(|s| s.to_string())) - .unwrap_or_default(); - vm.new_runtime_error(format!("({type_name}) {exc_name}: {exc_msg}")) - })?; - if let Some(arr) = array_obj.downcast_ref::() { - let arr_buffer = arr.0.buffer.read(); - let len = core::cmp::min(arr_buffer.len(), size); - self.write_bytes_at_offset(offset, &arr_buffer[..len]); - drop(arr_buffer); - self.keep_ref(index, array_obj, vm)?; - return Ok(()); - } - } - } - - // Get field type code for special handling - let field_type_code = proto - .get_attr("_type_", vm) - .ok() - .and_then(|attr| attr.downcast_ref::().map(|s| s.to_string())); - - // c_char_p (z type) with bytes: store original in _objects, keep - // null-terminated copy alive separately for the pointer. - if field_type_code.as_deref() == Some("z") - && let Some(bytes_val) = value.downcast_ref::() - { - let (kept_alive, ptr) = ensure_z_null_terminated(bytes_val, vm); - let result = - rustpython_host_env::ctypes::pointer_to_sized_bytes_endian(ptr, size, needs_swap); - self.write_bytes_at_offset(offset, &result); - return self.keep_ref_with_hidden(index, value, Some(kept_alive), vm); - } - - let (mut bytes, converted_value) = if let Some(type_code) = &field_type_code { - PyCField::value_to_bytes_for_type(type_code, &value, size, vm)? - } else { - (PyCField::value_to_bytes(&value, size, vm), None) - }; - - // Swap bytes for opposite endianness - if needs_swap { - bytes.reverse(); - } - - self.write_bytes_at_offset(offset, &bytes); - - // KeepRef: for z/Z types use converted value, otherwise use original - if let Some(converted) = converted_value { - self.keep_ref(index, converted, vm)?; - } else if Self::should_keep_ref(&value) { - self.keep_cdata(index, &value, vm)?; - } - - Ok(()) + let proto_type = proto + .downcast_ref::() + .ok_or_else(|| vm.new_type_error("field proto is not a type"))?; + let prepared = CDataValue::new(proto_type, &value, size, needs_swap, vm)?; + self.write_bytes_at_offset(offset, &prepared.bytes); + prepared.keep(self, index, vm) } pub(super) fn into_view(self, cls: PyTypeRef, vm: &VirtualMachine) -> PyResult { @@ -1287,7 +1366,11 @@ impl PyCData { let proto_metaclass = proto_type.class(); // Simple types: return primitive value - if proto_metaclass.fast_issubclass(super::simple::PyCSimpleType::static_type()) { + if super::simple::returns_scalar(&proto_type) { + if proto_type.type_code(vm).as_deref() == Some("O") { + let address = rustpython_host_env::ctypes::read_pointer_from_buffer(&buffer_data); + return self.object_at(address, Some(index), vm); + } // Check for byte swapping let needs_swap = super::simple::is_swapped(&proto_type); @@ -1306,7 +1389,8 @@ impl PyCData { let ptr = self.buffer.read().as_ptr().wrapping_add(offset) as *mut u8; let cdata_obj = unsafe { Self::from_base_obj(ptr, size, base_obj, index) }; - if proto_metaclass.fast_issubclass(super::structure::PyCStructType::static_type()) + if proto_metaclass.fast_issubclass(super::simple::PyCSimpleType::static_type()) + || proto_metaclass.fast_issubclass(super::structure::PyCStructType::static_type()) || proto_metaclass.fast_issubclass(super::union::PyCUnionType::static_type()) || proto_metaclass.fast_issubclass(super::pointer::PyCPointerType::static_type()) || proto_type.fast_issubclass(super::function::PyCFuncPtr::static_type()) @@ -1697,172 +1781,6 @@ impl GetDescriptor for PyCField { } impl PyCField { - /// Convert a Python value to bytes - fn value_to_bytes(value: &PyObject, size: usize, vm: &VirtualMachine) -> Vec { - // 1. Handle bytes objects - if let Some(bytes) = value.downcast_ref::() { - rustpython_host_env::ctypes::copy_to_sized_bytes(bytes.as_bytes(), size) - } - // 2. Handle ctypes array instances (copy their buffer) - else if let Some(cdata) = value.downcast_ref::() { - let buffer = cdata.buffer.read(); - rustpython_host_env::ctypes::copy_to_sized_bytes(&buffer, size) - } - // 4. Handle float values (check before int, since float.try_int would truncate) - else if let Some(float_val) = value.downcast_ref::() { - let f = float_val.to_f64(); - match size { - 4 | 8 => rustpython_host_env::ctypes::float_to_sized_bytes(f, size) - .expect("float size checked"), - _ => unreachable!("wrong payload size"), - } - } - // 4. Handle integer values - else if let Ok(int_val) = value.try_int(vm) { - let i = int_val.as_bigint(); - match size { - 1 => rustpython_host_env::ctypes::int_to_sized_bytes( - i.to_i8().unwrap_or(0).into(), - size, - ), - 2 => rustpython_host_env::ctypes::int_to_sized_bytes( - i.to_i16().unwrap_or(0).into(), - size, - ), - 4 => rustpython_host_env::ctypes::int_to_sized_bytes( - i.to_i32().unwrap_or(0).into(), - size, - ), - 8 => rustpython_host_env::ctypes::int_to_sized_bytes(i.to_i64().unwrap_or(0), size), - _ => rustpython_host_env::ctypes::zeroed_bytes(size), - } - } else { - rustpython_host_env::ctypes::zeroed_bytes(size) - } - } - - /// Convert a Python value to bytes with type-specific handling for pointer types. - /// Returns (bytes, optional holder for wchar buffer). - fn value_to_bytes_for_type( - type_code: &str, - value: &PyObject, - size: usize, - vm: &VirtualMachine, - ) -> PyResult<(Vec, Option)> { - match type_code { - // c_float: always convert to float first (f_set) - "f" => { - let f = if let Some(float_val) = value.downcast_ref::() { - float_val.to_f64() - } else if let Ok(int_val) = value.try_int(vm) { - int_val.as_bigint().to_i64().unwrap_or(0) as f64 - } else { - return Err(vm.new_type_error(format!( - "float expected instead of {}", - value.class().name() - ))); - }; - Ok(( - rustpython_host_env::ctypes::float_to_sized_bytes(f, 4) - .expect("c_float size is fixed"), - None, - )) - } - // c_double: always convert to float first (d_set) - "d" => { - let f = if let Some(float_val) = value.downcast_ref::() { - float_val.to_f64() - } else if let Ok(int_val) = value.try_int(vm) { - int_val.as_bigint().to_i64().unwrap_or(0) as f64 - } else { - return Err(vm.new_type_error(format!( - "float expected instead of {}", - value.class().name() - ))); - }; - Ok(( - rustpython_host_env::ctypes::float_to_sized_bytes(f, 8) - .expect("c_double size is fixed"), - None, - )) - } - // c_longdouble: convert to float (treated as f64 in RustPython) - "g" => { - let f = if let Some(float_val) = value.downcast_ref::() { - float_val.to_f64() - } else if let Ok(int_val) = value.try_int(vm) { - int_val.as_bigint().to_i64().unwrap_or(0) as f64 - } else { - return Err(vm.new_type_error(format!( - "float expected instead of {}", - value.class().name() - ))); - }; - Ok(( - rustpython_host_env::ctypes::float_to_sized_bytes(f, 8) - .expect("c_longdouble bytes are stored as f64"), - None, - )) - } - "z" => { - // c_char_p with bytes is handled in set_field before this call. - // This handles integer address and None cases. - // Integer address - if let Ok(int_val) = value.try_index(vm) { - let v = int_val.as_bigint().to_usize().unwrap_or(0); - return Ok(( - rustpython_host_env::ctypes::pointer_to_sized_bytes(v, size), - None, - )); - } - // None -> NULL pointer - if vm.is_none(value) { - return Ok((rustpython_host_env::ctypes::zeroed_bytes(size), None)); - } - Ok((Self::value_to_bytes(value, size, vm), None)) - } - "Z" => { - // c_wchar_p: store pointer to null-terminated wchar_t buffer - if let Some(s) = value.downcast_ref::() { - let (holder, ptr) = str_to_wchar_bytes(s.as_wtf8(), vm); - return Ok(( - rustpython_host_env::ctypes::pointer_to_sized_bytes(ptr, size), - Some(holder), - )); - } - // Integer address - if let Ok(int_val) = value.try_index(vm) { - let v = int_val.as_bigint().to_usize().unwrap_or(0); - return Ok(( - rustpython_host_env::ctypes::pointer_to_sized_bytes(v, size), - None, - )); - } - // None -> NULL pointer - if vm.is_none(value) { - return Ok((rustpython_host_env::ctypes::zeroed_bytes(size), None)); - } - Ok((Self::value_to_bytes(value, size, vm), None)) - } - "P" => { - // c_void_p: store integer as pointer - if let Ok(int_val) = value.try_index(vm) { - let v = int_val.as_bigint().to_usize().unwrap_or(0); - return Ok(( - rustpython_host_env::ctypes::pointer_to_sized_bytes(v, size), - None, - )); - } - // None -> NULL pointer - if vm.is_none(value) { - return Ok((rustpython_host_env::ctypes::zeroed_bytes(size), None)); - } - Ok((Self::value_to_bytes(value, size, vm), None)) - } - _ => Ok((Self::value_to_bytes(value, size, vm), None)), - } - } - /// 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) diff --git a/crates/vm/src/stdlib/_ctypes/pointer.rs b/crates/vm/src/stdlib/_ctypes/pointer.rs index 3e9b6058a..badc2811b 100644 --- a/crates/vm/src/stdlib/_ctypes/pointer.rs +++ b/crates/vm/src/stdlib/_ctypes/pointer.rs @@ -1,5 +1,5 @@ use super::base::CDATA_BUFFER_METHODS; -use super::{PyCArray, PyCData, PyCSimple, PyCStructure, StgInfo, StgInfoFlags}; +use super::{PyCData, StgInfo, StgInfoFlags}; use crate::atomic_func; use crate::protocol::{ BufferDescriptor, PyBuffer, PyMappingMethods, PyNumberMethods, PySequenceMethods, @@ -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::{PyBytes, PyInt, PyList, PySlice, PyStr, PyType, PyTypeRef}, - class::StaticType, + builtins::{PyInt, PyList, PySlice, PyStr, PyType, PyTypeRef}, function::{FuncArgs, OptionalArg}, }; use alloc::borrow::Cow; use num_traits::ToPrimitive; use rustpython_host_env::ctypes::{ - AddressValue, AddressWriteValue, IntegerValue, pointer_item_address, read_pointer_char_slice, + AddressValue, IntegerValue, pointer_item_address, read_pointer_char_slice, read_pointer_wchar_slice, }; @@ -577,104 +576,22 @@ impl PyCPointer { return Err(vm.new_value_error("NULL pointer access")); } - // Get element type, size and type_code from StgInfo.proto - let stg_info = zelf.class().stg_info(vm)?; - let proto_type = stg_info.proto(); - - // Get type code from proto's _type_ attribute - let type_code: Option = proto_type - .as_object() - .get_attr("_type_", vm) - .ok() - .and_then(|t| t.downcast_ref::().map(|s| s.to_string())); - - let element_size = proto_type - .stg_info_opt() - .map_or_else(rustpython_host_env::ctypes::pointer_size, |info| info.size); - - // Calculate address + let proto_type = zelf.class().stg_info(vm)?.proto().to_owned(); + let element_size = proto_type.stg_info(vm)?.size; + let prepared = super::base::CDataValue::new( + &proto_type, + &value, + element_size, + super::simple::is_swapped(&proto_type), + vm, + )?; let addr = pointer_item_address(ptr_value, index, element_size); - - // Write value at address - // Handle Structure/Array types by copying their buffer - if type_code.as_deref() == Some("O") - && value.downcast_ref::().is_some() - && !value.fast_isinstance(proto_type) - { - return Err(vm.new_type_error(format!( - "expected {} instance, not {}", - proto_type.name(), - value.class().name() - ))); - } - if let Some(cdata) = value.downcast_ref::() - && (cdata.fast_isinstance(PyCStructure::static_type()) - || cdata.fast_isinstance(PyCArray::static_type()) - || cdata.fast_isinstance(PyCSimple::static_type())) - { - // The value may be a view of this element or overlapping storage. - let bytes = { - let buffer = cdata.buffer.read(); - rustpython_host_env::ctypes::copy_to_sized_bytes(&buffer, element_size) - }; - unsafe { - rustpython_host_env::ctypes::copy_bytes_to_address(addr, &bytes, element_size); - } - zelf.0.keep_cdata(index as usize, &value, vm) - } else { - // Handle z/Z specially to store converted value - if type_code.as_deref() == Some("z") - && let Some(bytes) = value.downcast_ref::() - { - let (kept_alive, ptr_val) = super::base::ensure_z_null_terminated(bytes, vm); - unsafe { - rustpython_host_env::ctypes::write_value_to_address( - addr, - element_size, - AddressWriteValue::Pointer(ptr_val), - ); - } - return zelf.0.keep_ref_with_hidden( - index as usize, - value.clone(), - Some(kept_alive), - vm, - ); - } else if type_code.as_deref() == Some("Z") - && let Some(s) = value.downcast_ref::() - { - let (holder, ptr_val) = super::base::str_to_wchar_bytes(s.as_wtf8(), vm); - unsafe { - rustpython_host_env::ctypes::write_value_to_address( - addr, - element_size, - AddressWriteValue::Pointer(ptr_val), - ); - } - return zelf.0.keep_ref(index as usize, holder, vm); - } - if type_code.as_deref() == Some("O") { - unsafe { - rustpython_host_env::ctypes::write_value_to_address( - addr, - element_size, - AddressWriteValue::Pointer(value.get_id()), - ); - } - } else if super::simple::is_swapped(proto_type) { - let type_code = type_code - .as_deref() - .ok_or_else(|| vm.new_type_error("no _type_ attribute"))?; - let bytes = super::simple::swapped_value_bytes(type_code, &value, vm)?; - unsafe { - rustpython_host_env::ctypes::copy_bytes_to_address(addr, &bytes, element_size); - } - } else { - Self::write_value_at_address(addr, element_size, &value, type_code.as_deref(), vm)?; - } - // KeepRef: store reference to keep value alive using actual index - zelf.0.keep_ref(index as usize, value, vm) + // SAFETY: pointer indexing has the same caller-provided valid-storage + // requirement as reading the element. The source snapshot cannot overlap it. + unsafe { + rustpython_host_env::ctypes::copy_bytes_to_address(addr, &prepared.bytes, element_size); } + prepared.keep(&zelf.0, index as usize, vm) } /// Read a value from memory address @@ -693,82 +610,6 @@ impl PyCPointer { AddressValue::Bytes(bytes) => vm.ctx.new_bytes(bytes).into(), } } - - /// Write a value to memory address - fn write_value_at_address( - addr: usize, - size: usize, - value: &PyObject, - type_code: Option<&str>, - vm: &VirtualMachine, - ) -> PyResult<()> { - unsafe { - // Handle c_char_p (z) and c_wchar_p (Z) - store pointer address - // Note: PyBytes/PyStr cases are handled by caller (setitem_by_index) - if let Some("z" | "Z") = type_code { - let ptr_val = if vm.is_none(value) { - 0usize - } else if let Ok(int_val) = value.try_index(vm) { - super::simple::bigint_to_i128_wrapping(int_val.as_bigint()) as usize - } else { - return Err(vm.new_type_error("bytes/string or integer address expected")); - }; - rustpython_host_env::ctypes::write_value_to_address( - addr, - size, - AddressWriteValue::Pointer(ptr_val), - ); - return Ok(()); - } - - // Try to get value as integer - // Use write_unaligned for safety on strict-alignment architectures - if let Ok(int_val) = value.try_int(vm) { - let i = int_val.as_bigint(); - let wrapped = super::simple::bigint_to_i128_wrapping(i); - let bytes; - let write_value = match size { - 1 => AddressWriteValue::U8(wrapped as u8), - 2 => AddressWriteValue::I16(wrapped as i16), - 4 => AddressWriteValue::I32(wrapped as i32), - 8 => AddressWriteValue::I64(wrapped as i64), - _ => { - bytes = i.to_signed_bytes_le(); - AddressWriteValue::Bytes(&bytes) - } - }; - rustpython_host_env::ctypes::write_value_to_address(addr, size, write_value); - return Ok(()); - } - - // Try to get value as float - if let Ok(float_val) = value.try_float(vm) { - let f = float_val.to_f64(); - rustpython_host_env::ctypes::write_value_to_address( - addr, - size, - AddressWriteValue::Float(f), - ); - return Ok(()); - } - - // Try bytes - if value.check_buffer() { - let bytes = value.try_bytes_like(vm, |b| b.to_vec())?; - rustpython_host_env::ctypes::write_value_to_address( - addr, - size, - AddressWriteValue::Bytes(&bytes), - ); - return Ok(()); - } - - Err(vm.new_type_error(format!( - "cannot convert {} to ctypes data", - value.class().name() - ))) - } - } } impl AsNumber for PyCPointer { diff --git a/crates/vm/src/stdlib/_ctypes/simple.rs b/crates/vm/src/stdlib/_ctypes/simple.rs index 831d8cc3b..54ce7a1a3 100644 --- a/crates/vm/src/stdlib/_ctypes/simple.rs +++ b/crates/vm/src/stdlib/_ctypes/simple.rs @@ -1,7 +1,8 @@ use super::_ctypes::CArgObject; use super::array::PyCArray; use super::base::{ - CArgValue, CDATA_BUFFER_METHODS, ParamFunc, PyCData, StgInfo, StgInfoFlags, bytes_to_pyobject, + CArgValue, CDATA_BUFFER_METHODS, CDataBytes, ParamFunc, PyCData, StgInfo, StgInfoFlags, + bytes_to_pyobject, }; use super::function::PyCFuncPtr; use super::pointer::PyCPointer; @@ -75,6 +76,9 @@ fn new_simple_type( } pub(super) fn bigint_to_i128_wrapping(value: &malachite_bigint::BigInt) -> i128 { + if let Some(value) = value.to_i128() { + return value; + } let bytes = value.to_signed_bytes_le(); let fill = bytes .last() @@ -553,6 +557,7 @@ impl Initializer for PyCSimpleType { let size = simple_type_size(&type_str).expect("invalid ctypes simple type"); let align = simple_type_align(&type_str).expect("invalid ctypes simple type"); let mut stg_info = StgInfo::new(size, align); + stg_info.scalar_code = type_str.chars().next(); // Set format for PEP 3118 buffer protocol stg_info.format = Some(alloc_format_string_for_type( @@ -698,18 +703,108 @@ pub(super) fn returns_scalar(cls: &Py) -> bool { .is_some_and(|base| base.is(PyCSimple::static_type())) } -pub(super) fn swapped_value_bytes( +pub(super) fn assignment_value( type_code: &str, value: &PyObject, + swapped: bool, vm: &VirtualMachine, -) -> PyResult> { +) -> PyResult<(CDataBytes, Option, Option)> { + let mut keep = None; + let mut hidden = None; // Convert once before writing: numeric conversion may reenter ctypes. - let value = match type_code { + let storage = match type_code { + "c" => { + let byte = if let Some(bytes) = value.downcast_ref::() { + match bytes.as_bytes() { + [byte] => *byte, + _ => return Err(vm.new_type_error("one character bytes expected")), + } + } else if let Some(bytes) = value.downcast_ref::() { + match &*bytes.borrow_buf() { + [byte] => *byte, + _ => return Err(vm.new_type_error("one character bytearray expected")), + } + } else { + value + .downcast_ref::() + .and_then(|integer| integer.as_bigint().to_u8()) + .ok_or_else(|| { + vm.new_type_error("one character bytes, bytearray or integer expected") + })? + }; + SimpleStorageValue::Byte(byte) + } + "u" => { + let text = value + .downcast_ref::() + .ok_or_else(|| vm.new_type_error("unicode string expected"))?; + let mut chars = text.as_wtf8().code_points(); + let character = chars + .next() + .filter(|_| chars.next().is_none()) + .ok_or_else(|| vm.new_type_error("one character unicode string expected"))?; + if rustpython_host_env::ctypes::WCHAR_SIZE == 2 && character.to_u32() > 0xffff { + return Err(vm.new_type_error("a single UTF-16 code unit is required")); + } + SimpleStorageValue::Wchar(character.to_u32()) + } "f" | "d" | "g" => SimpleStorageValue::Float(value.try_float(vm)?.to_f64()), "?" | "v" => SimpleStorageValue::Bool(value.to_owned().try_to_bool(vm)?), - _ => SimpleStorageValue::Signed(bigint_to_i128_wrapping(value.try_index(vm)?.as_bigint())), + "O" => { + keep = (!vm.is_none(value)).then(|| value.to_owned()); + SimpleStorageValue::ObjectId(value.get_id()) + } + "z" | "Z" | "P" | "X" => { + // X preserves opaque-address/null writes. BSTR string assignment + // needs COM allocation and ownership, not a Z-style wide buffer. + let address = if vm.is_none(value) { + 0 + } else if type_code == "z" + && let Some(bytes) = value.downcast_ref::() + { + let (holder, address) = super::base::ensure_z_null_terminated(bytes, vm); + keep = Some(value.to_owned()); + hidden = Some(holder); + address + } else if type_code == "Z" + && let Some(text) = value.downcast_ref::() + { + let (holder, address) = super::base::str_to_wchar_bytes(text.as_wtf8(), vm); + keep = Some(holder); + address + } else if let Some(integer) = value.downcast_ref::() { + bigint_to_i128_wrapping(integer.as_bigint()) as usize + } else { + return Err(vm.new_type_error("integer address expected")); + }; + SimpleStorageValue::Pointer(address) + } + _ => { + let index; + let integer = if let Some(integer) = value.downcast_ref_if_exact::(vm) { + integer + } else { + index = value.try_index(vm)?; + &index + }; + SimpleStorageValue::Signed(bigint_to_i128_wrapping(integer.as_bigint())) + } }; - Ok(simple_storage_value_to_bytes_endian(type_code, value, true)) + let bytes = match (type_code, storage) { + ( + "b" | "B" | "h" | "H" | "i" | "I" | "l" | "L" | "q" | "Q", + SimpleStorageValue::Signed(value), + ) => { + let size = simple_type_size(type_code).expect("integer ctypes size"); + let mut bytes = (value as u64).to_le_bytes(); + if super::base::is_big_endian(swapped) { + bytes[..size].reverse(); + } + CDataBytes::Integer { bytes, size } + } + (_, storage) => simple_storage_value_to_bytes_endian(type_code, storage, swapped).into(), + }; + Ok((bytes, keep, hidden)) } fn value_to_bytes_endian( @@ -987,7 +1082,7 @@ impl Constructor for PyCSimple { let (kept_alive, ptr) = super::base::ensure_z_null_terminated(bytes, vm); let buffer = rustpython_host_env::ctypes::pointer_bytes(ptr); let cdata = PyCData::from_bytes(buffer, Some(v.clone())); - cdata.keep_alive(0, kept_alive); + cdata.keep_alive(kept_alive); return Self(cdata).into_ref_with_type(vm, cls).map(Into::into); } } else if _type_ == "Z" @@ -1200,9 +1295,9 @@ impl Py { write_simple_storage_buffer(&mut buffer, &buffer_bytes); drop(buffer); - // For c_char_p (type "z"), c_wchar_p (type "Z"), and py_object (type "O"), - // keep the reference in _objects - if type_code == "z" || type_code == "Z" || type_code == "O" { + // Raw z/Z addresses and None do not replace the old owner: the + // address may still point into its buffer (CPython KeepRef(None)). + if type_code == "O" { zelf.0.keep_value(value, None, vm)?; } Ok(()) diff --git a/extra_tests/snippets/stdlib_ctypes.py b/extra_tests/snippets/stdlib_ctypes.py index a61db9323..d99f47e3d 100644 --- a/extra_tests/snippets/stdlib_ctypes.py +++ b/extra_tests/snippets/stdlib_ctypes.py @@ -855,3 +855,126 @@ except TypeError: else: raise AssertionError("array accepted for an incomplete pointer field") assert bytes(_field) == _before + +# Indexed writes use the element's conversion, not the input object's layout. +for _container, _input, _expected in ( + (ctypes.pointer(ctypes.c_float()), 1.5, 1.5), + ((ctypes.c_char * 1)(), ctypes.c_char(b"x"), b"x"), + ((ctypes.c_byte * 1)(), 257, 1), +): + _container[0] = _input + assert _container[0] == _expected + +for _container in ((ctypes.c_int * 1)(), ctypes.pointer(ctypes.c_int())): + try: + _container[0] = ctypes.c_float(1.5) + except TypeError: + pass + else: + raise AssertionError("incompatible indexed value accepted") + assert _container[0] == 0 + + +class FailingFloat: + def __float__(self): + raise LookupError("conversion failed") + + +for _base in (ctypes.Structure, ctypes.Union): + + class ObjectAndFloat(_base): + _fields_ = [("object", ctypes.py_object), ("number", ctypes.c_float)] + + _record = ObjectAndFloat() + _value = object() + _record.object = _value + assert _record.object is _value + _before = bytes(_record) + try: + _record.number = FailingFloat() + except LookupError: + pass + else: + raise AssertionError("conversion error suppressed") + assert bytes(_record) == _before + + +# Tuple temporaries are finalized after their bytes have reached the target. +_events = [] + + +class TemporaryRecord(ctypes.Structure): + _fields_ = [("number", ctypes.c_int)] + + def __del__(self): + if self._b_base_ is None: + _events.append(_records[0].number) + _records[0].number = 99 + + +_records = (TemporaryRecord * 1)() +_records[0] = (42,) +assert _events == [42] +assert _records[0].number == 99 + + +# Raw addresses can point inside the buffer already owned by a scalar view. +_events = [] + + +class OwnedBytes(bytes): + def __del__(self): + _events.append("released") + + +class StringPointer(ctypes.c_char_p): + pass + + +class OwnedStringField(ctypes.Structure): + _fields_ = [("text", StringPointer)] + + +_record = OwnedStringField() +_record.text.value = OwnedBytes(b"owner") +_address = ctypes.cast(_record.text, ctypes.c_void_p).value +_record.text.value = _address + 1 +gc.collect() +assert _events == [] +assert _record.text.value == b"wner" + +# Self-copy must not build a new private holder around every previous copy. +_marker = b"ctypes-self-copy-backing-store" +_strings = (StringField * 1)(StringField(_marker)) +for _ in range(32): + _strings[0] = _strings[0] +gc.collect() +assert ( + sum( + type(obj) is tuple + and any(type(item) is bytes and item == _marker + b"\0" for item in obj) + for obj in gc.get_objects() + ) + <= 1 +) +assert _strings[0].text == _marker + +# Mutating _type_ must not change the initialized setter or its write width. +for _code in ("h", "q", "f"): + + class MutableInt(ctypes.c_int): + pass + + _array = (MutableInt * 2)(0, 0x1234) + MutableInt._type_ = _code + _array[0] = -2 + assert bytes(_array) == bytes(ctypes.c_int(-2)) + bytes(ctypes.c_int(0x1234)) + +if _sys.platform == "win32": + + class BSTR(ctypes._SimpleCData): + _type_ = "X" + + _bstr = (BSTR * 1)() + _bstr[0] = None + assert bytes(_bstr) == bytes(ctypes.sizeof(ctypes.c_void_p))