diff --git a/Lib/test/test_ctypes/test_win32_com_foreign_func.py b/Lib/test/test_ctypes/test_win32_com_foreign_func.py index 01db602149b..7e54f8f6c31 100644 --- a/Lib/test/test_ctypes/test_win32_com_foreign_func.py +++ b/Lib/test/test_ctypes/test_win32_com_foreign_func.py @@ -157,7 +157,6 @@ class IPersist(IUnknown): self.assertEqual(0, ppst.Release()) - @unittest.expectedFailure # TODO: RUSTPYTHON; - COM iid parameter handling not implemented def test_with_paramflags_and_iid(self): class IUnknown(c_void_p): QueryInterface = proto_query_interface(None, IID_IUnknown) diff --git a/crates/host_env/src/ctypes.rs b/crates/host_env/src/ctypes.rs index d28ca3cf2db..19bf397f502 100644 --- a/crates/host_env/src/ctypes.rs +++ b/crates/host_env/src/ctypes.rs @@ -1835,7 +1835,11 @@ pub const HRESULT_S_OK: i32 = crate::windows::HRESULT_S_OK; #[cfg(windows)] pub fn format_error_message(code: Option) -> Option { - crate::windows::format_error_message(code) + let code = code + .filter(|&code| code != 0) + .unwrap_or_else(crate::windows::get_last_error); + crate::windows::format_error_message_with_language(code, 1 << 10) + .map(|message| message.trim_end().to_owned()) } #[cfg(windows)] @@ -3860,3 +3864,157 @@ mod tests { } } } + +/// Owned UTF-16 fields returned by COM's error-information interfaces. +#[cfg(windows)] +#[derive(Default)] +pub struct ComErrorDetails { + pub description: Option>, + pub source: Option>, + pub helpfile: Option>, + pub help_context: u32, + pub progid: Option>, +} + +/// Format a COM error like ctypes' FormatError (retain trailing punctuation). +#[cfg(windows)] +pub fn format_com_error_message(status: i32) -> Option { + crate::windows::format_error_message_with_language(status as u32, 1 << 10) + .map(|message| message.trim_end().to_owned()) +} + +/// Obtain the error information associated with a failed COM method call. +/// +/// The caller must detach its interpreter while this runs: COM methods can +/// call back into Python. No borrowed Python buffers may be held here. +/// +/// # Safety +/// `com_ptr` must identify a live IUnknown-compatible COM interface for the +/// duration of this call. `iid` is the native byte representation of its GUID. +#[cfg(windows)] +pub unsafe fn com_error_details(com_ptr: usize, iid: &[u8; 16]) -> ComErrorDetails { + use windows_sys::core::GUID; + + type QueryInterface = + unsafe extern "system" fn(*mut c_void, *const GUID, *mut *mut c_void) -> i32; + type Release = unsafe extern "system" fn(*mut c_void) -> u32; + type SupportsErrorInfo = unsafe extern "system" fn(*mut c_void, *const GUID) -> i32; + type GetGuid = unsafe extern "system" fn(*mut c_void, *mut GUID) -> i32; + type GetString = unsafe extern "system" fn(*mut c_void, *mut *mut u16) -> i32; + type GetContext = unsafe extern "system" fn(*mut c_void, *mut u32) -> i32; + + struct ComRef(*mut c_void); + impl Drop for ComRef { + fn drop(&mut self) { + // SAFETY: ComRef owns one reference on an IUnknown-compatible + // interface, whose third vtable entry is Release. + unsafe { + let release: Release = core::mem::transmute(method(self.0, 2)); + release(self.0); + } + } + } + + unsafe fn method(interface: *mut c_void, index: usize) -> usize { + // SAFETY: the caller provides a live interface and a valid slot for + // that interface's documented COM ABI. + unsafe { *(*(interface as *const *const usize)).add(index) } + } + + unsafe fn string_field(interface: *mut c_void, index: usize) -> Option> { + let mut value = core::ptr::null_mut(); + // SAFETY: IErrorInfo slots 4–6 all return an owned BSTR through an + // initialized output pointer. Free even if the method reports failure. + unsafe { + let get: GetString = core::mem::transmute(method(interface, index)); + get(interface, &mut value); + if value.is_null() { + return None; + } + let units = core::slice::from_raw_parts(value, sys_string_len(value)); + // CPython builds these fields from NUL-terminated wide strings. + let end = units + .iter() + .position(|&unit| unit == 0) + .unwrap_or(units.len()); + let result = units[..end].to_vec(); + sys_free_string(value); + Some(result) + } + } + + let mut details = ComErrorDetails::default(); + if com_ptr == 0 { + return details; + } + // IID_ISupportErrorInfo = {DF0B3D60-548F-101B-8E65-08002B2BD119}. + let support_iid = GUID { + data1: 0xdf0b3d60, + data2: 0x548f, + data3: 0x101b, + data4: [0x8e, 0x65, 0x08, 0x00, 0x2b, 0x2b, 0xd1, 0x19], + }; + // SAFETY: GUID is a 16-byte POD value; the input need not be aligned. + let iid = unsafe { core::ptr::read_unaligned(iid.as_ptr().cast::()) }; + let interface = com_ptr as *mut c_void; + let mut support = core::ptr::null_mut(); + // SAFETY: the live input interface implements IUnknown. A successful + // QueryInterface returns one owned ISupportErrorInfo reference. + let status = unsafe { + let query: QueryInterface = core::mem::transmute(method(interface, 0)); + query(interface, &support_iid, &mut support) + }; + if status < 0 || support.is_null() { + return details; + } + let support = ComRef(support); + // SAFETY: ISupportErrorInfo's fourth slot accepts the requested IID. + let status = unsafe { + let supports: SupportsErrorInfo = core::mem::transmute(method(support.0, 3)); + supports(support.0, &iid) + }; + drop(support); + if status < 0 { + return details; + } + let Some(error_info) = get_error_info() else { + return details; + }; + if error_info.is_null() { + return details; + } + let error_info = ComRef(error_info); + let mut guid = GUID { + data1: 0, + data2: 0, + data3: 0, + data4: [0; 8], + }; + // SAFETY: GetErrorInfo supplied a live IErrorInfo. Its slots have the + // documented signatures, and all output storage is initialized. + let guid_status = unsafe { + details.description = string_field(error_info.0, 5); + let get_guid: GetGuid = core::mem::transmute(method(error_info.0, 3)); + let status = get_guid(error_info.0, &mut guid); + let get_context: GetContext = core::mem::transmute(method(error_info.0, 7)); + get_context(error_info.0, &mut details.help_context); + details.helpfile = string_field(error_info.0, 6); + details.source = string_field(error_info.0, 4); + status + }; + drop(error_info); + if guid_status >= 0 { + // SAFETY: GUID is POD; copy the initialized native representation. + let guid_bytes = + unsafe { core::ptr::read_unaligned((&guid as *const GUID).cast::<[u8; 16]>()) }; + if let Some(progid) = prog_id_from_clsid(&guid_bytes) { + // SAFETY: ProgIDFromCLSID returns a NUL-terminated allocation + // owned by this call and released with CoTaskMemFree. + unsafe { + details.progid = Some(WideCStr::from_ptr_str(progid).as_slice().to_vec()); + co_task_mem_free(progid.cast()); + } + } + } + details +} diff --git a/crates/vm/src/builtins/set.rs b/crates/vm/src/builtins/set.rs index 8f289d1e2e9..06597bce225 100644 --- a/crates/vm/src/builtins/set.rs +++ b/crates/vm/src/builtins/set.rs @@ -1316,7 +1316,7 @@ impl Representable for PySet { } if let Some(_guard) = ReprGuard::enter(vm, zelf.as_object()) { - let name = (class_name != "set").then_some(class_name); + let name = (!class.is(vm.ctx.types.set_type)).then_some(class_name); zelf.inner.repr(name, vm) } else { Ok(Wtf8Buf::from(format!("{class_name}(...)"))) diff --git a/crates/vm/src/exceptions.rs b/crates/vm/src/exceptions.rs index 758091d3004..2fd09c2f2aa 100644 --- a/crates/vm/src/exceptions.rs +++ b/crates/vm/src/exceptions.rs @@ -2631,7 +2631,7 @@ pub(super) mod types { // We need this method, because of how `CPython` copies `init` // from `BaseException` in `SimpleExtendsException` macro. // See: `BaseException_new` - if *cls.name() == *vm.ctx.exceptions.os_error.name() { + if cls.is(vm.ctx.exceptions.os_error) { let args_vec = args.args.to_vec(); let len = args_vec.len(); if (2..=5).contains(&len) { diff --git a/crates/vm/src/stdlib/_ctypes.rs b/crates/vm/src/stdlib/_ctypes.rs index 2ab9d0457e6..e5bab0b54bc 100644 --- a/crates/vm/src/stdlib/_ctypes.rs +++ b/crates/vm/src/stdlib/_ctypes.rs @@ -109,6 +109,8 @@ pub(crate) mod _ctypes { use crate::convert::ToPyObject; use crate::function::{ArgStrictInt, Either, OptionalArg}; use crate::types::Representable; + #[cfg(windows)] + use crate::types::{Constructor, Initializer}; use crate::{AsObject, Py, PyObjectRef, PyPayload, PyResult, VirtualMachine}; use num_traits::ToPrimitive; @@ -294,76 +296,49 @@ pub(crate) mod _ctypes { ) } - #[cfg(target_os = "windows")] - #[pyattr(name = "COMError", once)] - fn com_error(vm: &VirtualMachine) -> PyTypeRef { - use crate::PyObject; - use crate::builtins::type_::PyAttributes; - use crate::function::FuncArgs; - use crate::types::{PyTypeFlags, PyTypeSlots}; - - // Sets hresult, text, details as instance attributes in __init__ - // This function has InitFunc signature for direct slots.init use - fn comerror_init(zelf: &PyObject, args: FuncArgs, vm: &VirtualMachine) -> PyResult<()> { - let (hresult, text, details): ( - Option, - Option, - Option, - ) = args.bind(vm)?; - let hresult = hresult.unwrap_or_else(|| vm.ctx.none()); - let text = text.unwrap_or_else(|| vm.ctx.none()); - let details = details.unwrap_or_else(|| vm.ctx.none()); - - // Set instance attributes - zelf.set_attr("hresult", hresult, vm)?; - zelf.set_attr("text", text.clone(), vm)?; - zelf.set_attr("details", details.clone(), vm)?; + /// Raised when a COM method call failed. + #[cfg(windows)] + #[pyattr] + #[pyexception(name = "COMError", module = "_ctypes", base = crate::builtins::PyException)] + #[derive(Debug)] + #[repr(transparent)] + struct PyComError(crate::builtins::PyException); - // self.args = args[1:] = (text, details) - // via: PyObject_SetAttrString(self, "args", PySequence_GetSlice(args, 1, size)) - let args_tuple: PyObjectRef = vm.ctx.new_tuple(vec![text, details]).into(); - zelf.set_attr("args", args_tuple, vm)?; + #[cfg(windows)] + #[pyexception(with(Initializer), flags(HEAPTYPE))] + impl PyComError {} + #[cfg(windows)] + impl Initializer for PyComError { + type Args = crate::function::FuncArgs; + + fn slot_init( + zelf: &crate::PyObject, + args: Self::Args, + vm: &VirtualMachine, + ) -> PyResult<()> { + let (hresult, text, details): (PyObjectRef, PyObjectRef, PyObjectRef) = + args.bind(vm)?; + let full_args = vec![hresult.clone(), text.clone(), details.clone()]; + // Match the observable attribute assignments before restoring the + // base payload directly, without invoking an overriding descriptor. + let short_args: PyObjectRef = + vm.ctx.new_tuple(vec![text.clone(), details.clone()]).into(); + zelf.set_attr("args", short_args, vm)?; + zelf.set_attr("hresult", hresult, vm)?; + zelf.set_attr("text", text, vm)?; + zelf.set_attr("details", details, vm)?; + crate::builtins::PyBaseException::slot_init( + zelf, + crate::function::FuncArgs::from(full_args), + vm, + )?; Ok(()) } - // Create exception type with IMMUTABLETYPE flag - let mut attrs = PyAttributes::default(); - attrs.insert( - vm.ctx.intern_str("__module__"), - vm.ctx.new_str("_ctypes").into(), - ); - attrs.insert( - vm.ctx.intern_str("__doc__"), - vm.ctx - .new_str("Raised when a COM method call failed.") - .into(), - ); - - // Create slots with IMMUTABLETYPE flag - let slots = PyTypeSlots { - name: "COMError", - flags: crate::types::AtomicPyTypeFlags::from_plain( - PyTypeFlags::HEAP_TYPE_DICT_IMMUTABLE, - ), - ..PyTypeSlots::default() - }; - - let exc_type = PyType::new_heap( - "COMError", - vec![vm.ctx.exceptions.exception_type.to_owned()], - attrs, - slots, - vm.ctx.types.type_type.to_owned(), - &vm.ctx, - ) - .unwrap(); - - // Set our custom init after new_heap, which runs init_slots that would - // otherwise overwrite slots.init with init_wrapper (due to __init__ in MRO). - exc_type.slots.init.store(Some(comerror_init)); - - exc_type + fn init(_zelf: &Py, _args: Self::Args, _vm: &VirtualMachine) -> PyResult<()> { + unreachable!("slot_init is defined") + } } #[pyfunction] @@ -608,7 +583,7 @@ pub(crate) mod _ctypes { #[cfg(target_os = "windows")] #[pyfunction(name = "_check_HRESULT")] - fn check_hresult(hr: i32, vm: &VirtualMachine) -> PyResult { + pub(super) fn check_hresult(hr: i32, vm: &VirtualMachine) -> PyResult { if hr < 0 { let errno = crate::host_env::os::winerror_to_errno(hr); let message = crate::host_env::uuid::format_error_message(hr); @@ -1042,9 +1017,11 @@ pub(crate) mod _ctypes { #[cfg(windows)] #[pyfunction(name = "FormatError")] - fn format_error_func(code: OptionalArg, _vm: &VirtualMachine) -> String { - rustpython_host_env::ctypes::format_error_message(code.into_option()) - .unwrap_or_else(|| "".to_string()) + fn format_error_func(code: OptionalArg, _vm: &VirtualMachine) -> String { + rustpython_host_env::ctypes::format_error_message( + code.into_option().map(|code| code as u32), + ) + .unwrap_or_else(|| "".to_string()) } #[cfg(windows)] diff --git a/crates/vm/src/stdlib/_ctypes/base.rs b/crates/vm/src/stdlib/_ctypes/base.rs index 742a91e12ce..d4f8e212314 100644 --- a/crates/vm/src/stdlib/_ctypes/base.rs +++ b/crates/vm/src/stdlib/_ctypes/base.rs @@ -100,6 +100,9 @@ pub struct StgInfo { // Function parameter conversion pub(super) paramfunc: ParamFunc, // how to pass to foreign functions + pub(super) argtypes: Option, + pub(super) restype: Option, + pub(super) checker: Option, // Byte order (for _swappedbytes_) pub big_endian: bool, // true if big endian, false if little endian @@ -154,6 +157,9 @@ impl Default for StgInfo { format: None, shape: Vec::new(), paramfunc: ParamFunc::None, + argtypes: None, + restype: None, + checker: None, big_endian: cfg!(target_endian = "big"), // native endian by default field_layouts: Vec::new(), pointer_type: None, @@ -167,12 +173,18 @@ unsafe impl Traverse for StgInfo { self.proto.traverse(tracer); self.element_type.traverse(tracer); self.pointer_type.traverse(tracer); + self.argtypes.traverse(tracer); + self.restype.traverse(tracer); + self.checker.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()); + out.extend(self.argtypes.take()); + out.extend(self.restype.take()); + out.extend(self.checker.take()); } } @@ -191,6 +203,9 @@ impl StgInfo { format: None, shape: Vec::new(), paramfunc: ParamFunc::None, + argtypes: None, + restype: None, + checker: None, big_endian: cfg!(target_endian = "big"), // native endian by default field_layouts: Vec::new(), pointer_type: None, @@ -244,6 +259,9 @@ impl StgInfo { format, shape, paramfunc: ParamFunc::Array, + argtypes: None, + restype: None, + checker: None, big_endian: cfg!(target_endian = "big"), // native endian by default field_layouts: Vec::new(), pointer_type: None, @@ -3067,7 +3085,7 @@ impl PyCData { pub(super) fn into_view(self, cls: PyTypeRef, vm: &VirtualMachine) -> PyResult { if cls.fast_issubclass(super::function::PyCFuncPtr::static_type()) { - super::function::PyCFuncPtr::from_cdata(self) + super::function::PyCFuncPtr::from_cdata(self, vm) .into_ref_with_type(vm, cls) .map(Into::into) } else { @@ -4102,16 +4120,6 @@ pub(super) fn get_usize_attr( Ok(val.to_usize().unwrap_or(default)) } -/// Check if a type is a "simple instance" (direct subclass of a simple type) -/// Returns TRUE for c_int, c_void_p, etc. (simple types with _type_ attribute) -/// Returns FALSE for Structure, Array, POINTER(T), etc. -pub(super) fn is_simple_instance(typ: &Py) -> bool { - // _ctypes_simple_instance - // Check if the type's metaclass is PyCSimpleType - let metaclass = typ.class(); - metaclass.fast_issubclass(super::simple::PyCSimpleType::static_type()) -} - /// Set or initialize StgInfo on a type pub(super) fn set_or_init_stginfo(type_ref: &PyType, stg_info: StgInfo) { if type_ref.init_traced_type_data(stg_info.clone()).is_err() diff --git a/crates/vm/src/stdlib/_ctypes/function.rs b/crates/vm/src/stdlib/_ctypes/function.rs index a2c8ae6feec..094c265dfbd 100644 --- a/crates/vm/src/stdlib/_ctypes/function.rs +++ b/crates/vm/src/stdlib/_ctypes/function.rs @@ -13,11 +13,13 @@ use crate::{ AsObject, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine, builtins::{PyBytes, PyDict, PyInt, PyStr, PyTuple, PyType, PyTypeRef}, class::StaticType, - function::FuncArgs, + function::{FuncArgs, PySetterValue}, protocol::{BufferDescriptor, PyBuffer, PyNumberMethods, PySequenceMethods}, types::{AsBuffer, AsNumber, AsSequence, Callable, Constructor, Initializer, Representable}, - vm::thread::with_current_vm, + vm::thread::{PyOperation, with_current_vm}, }; +#[cfg(windows)] +use crate::{TryFromObject, function::ArgBytesLike}; use alloc::borrow::Cow; use core::ffi::{c_int, c_long, c_ulong, c_void}; use core::fmt::Debug; @@ -369,6 +371,10 @@ impl Initializer for PyCFuncPtrType { stg_info.length = 1; stg_info.flags |= StgInfoFlags::TYPEFLAG_ISPOINTER; stg_info.paramfunc = ParamFunc::Pointer; // CFuncPtr is passed as a pointer + // Function-type metadata comes from this class's namespace, not its MRO. + stg_info.argtypes = new_type.attributes.get(vm.ctx.intern_str("_argtypes_")); + stg_info.restype = new_type.attributes.get(vm.ctx.intern_str("_restype_")); + stg_info.checker = restype_checker(stg_info.restype.as_deref(), vm)?; let _ = new_type.init_traced_type_data(stg_info); Ok(()) @@ -428,6 +434,8 @@ pub(super) struct PyCFuncPtr { pub argtypes: PyRwLock>, /// Instance-level restype override pub restype: PyRwLock>, + /// Serialize restype updates while allowing Python finalizers to re-enter. + restype_operation: PyOperation, /// Checker function pub checker: PyRwLock>, /// Error checking function @@ -762,7 +770,7 @@ pub(super) fn cast_impl( } impl PyCFuncPtr { - pub(super) fn from_cdata(data: PyCData) -> Self { + pub(super) fn from_cdata(data: PyCData, vm: &VirtualMachine) -> Self { Self { _base: data, thunk: PyRwLock::new(None), @@ -770,6 +778,7 @@ impl PyCFuncPtr { converters: PyRwLock::new(None), argtypes: PyRwLock::new(None), restype: PyRwLock::new(None), + restype_operation: PyOperation::new(vm), checker: PyRwLock::new(None), errcheck: PyRwLock::new(None), #[cfg(windows)] @@ -805,7 +814,7 @@ impl Constructor for PyCFuncPtr { let ptr_size = pointer_size(); if args.args.is_empty() { - return Self::from_cdata(PyCData::from_bytes(vec![0u8; ptr_size], None)) + return Self::from_cdata(PyCData::from_bytes(vec![0u8; ptr_size], None), vm) .into_ref_with_type(vm, cls) .map(Into::into); } @@ -821,19 +830,30 @@ impl Constructor for PyCFuncPtr { #[cfg(windows)] let index = first_arg.try_int(vm)?.as_bigint().to_usize().unwrap_or(0); - // args[3] is iid (GUID struct, optional) - // Also check if args[2] is a GUID (has Data1 attribute) when args[3] is not present + // Retain the accepted IID owner rather than a borrowed raw pointer. + // Mutable CData GUIDs remain observable; copy bytes only at call time. #[cfg(windows)] - let iid = args.args.get(3).cloned().or_else(|| { - args.args.get(2).and_then(|arg| { - // If it's a GUID struct (has Data1 attribute), use it as IID - if arg.get_attr("Data1", vm).is_ok() { - Some(arg.clone()) - } else { - None - } - }) - }); + let iid = if let Some(iid) = args.args.get(3).filter(|arg| !vm.is_none(arg)) { + let owner: PyObjectRef = if let Some(text) = iid.downcast_ref::() { + vm.state + .codec_registry + .encode_text(text.to_owned(), "utf-8", None, vm)? + .into() + } else if iid.downcast_ref::().is_some() + || iid.downcast_ref::().is_some() + { + iid.clone() + } else { + return Err(vm.new_type_error( + "iid must be a read-only bytes-like object, str, or None", + )); + }; + let buffer = ArgBytesLike::try_from_object(vm, owner.clone())?; + let valid_size = buffer.borrow_buf().len() == 16; + valid_size.then_some(owner) + } else { + None + }; // args[2] is paramflags (tuple or None) let paramflags = args.args.get(2).filter(|arg| !vm.is_none(arg)).cloned(); @@ -845,6 +865,7 @@ impl Constructor for PyCFuncPtr { converters: PyRwLock::new(None), argtypes: PyRwLock::new(None), restype: PyRwLock::new(None), + restype_operation: PyOperation::new(vm), checker: PyRwLock::new(None), errcheck: PyRwLock::new(None), #[cfg(windows)] @@ -867,6 +888,7 @@ impl Constructor for PyCFuncPtr { converters: PyRwLock::new(None), argtypes: PyRwLock::new(None), restype: PyRwLock::new(None), + restype_operation: PyOperation::new(vm), checker: PyRwLock::new(None), errcheck: PyRwLock::new(None), #[cfg(windows)] @@ -933,6 +955,7 @@ impl Constructor for PyCFuncPtr { converters: PyRwLock::new(None), argtypes: PyRwLock::new(None), restype: PyRwLock::new(None), + restype_operation: PyOperation::new(vm), checker: PyRwLock::new(None), errcheck: PyRwLock::new(None), #[cfg(windows)] @@ -948,8 +971,27 @@ impl Constructor for PyCFuncPtr { // Check if first argument is a Python callable (callback creation) if first_arg.is_callable() { // Get argument types and result type from the class - let class_argtypes = cls.get_attr(vm.ctx.intern_str("_argtypes_")); - let class_restype = cls.get_attr(vm.ctx.intern_str("_restype_")); + let (class_argtypes, class_restype, class_checker) = cls + .stg_info_opt() + .map(|info| { + ( + info.argtypes.clone(), + info.restype.clone(), + info.checker.clone(), + ) + }) + .unwrap_or_default(); + if class_argtypes.is_none() { + return Err( + vm.new_type_error("cannot construct instance of this class: no argtypes") + ); + } + // Missing metadata is not an explicit void return. CPython 3.15 + // dereferences NULL here; reject it rather than build a void thunk + // whose caller would interpret an uninitialized C int result. + if class_restype.is_none() { + return Err(vm.new_type_error("invalid result type for callback function")); + } let class_flags = cls .get_attr(vm.ctx.intern_str("_flags_")) .and_then(|f| f.try_to_value::(vm).ok()) @@ -980,7 +1022,8 @@ impl Constructor for PyCFuncPtr { converters: PyRwLock::new(None), argtypes: PyRwLock::new(class_argtypes), restype: PyRwLock::new(class_restype), - checker: PyRwLock::new(None), + restype_operation: PyOperation::new(vm), + checker: PyRwLock::new(class_checker), errcheck: PyRwLock::new(None), #[cfg(windows)] index: PyRwLock::new(None), @@ -1073,6 +1116,9 @@ enum RetSpec { struct CallInfo { explicit_arg_types: Option>, restype_obj: Option, + checker: Option, + #[cfg(windows)] + check_hresult: bool, ret: RetSpec, } @@ -1153,11 +1199,21 @@ fn compute_ret_spec( RetSpec::Code('i') } +fn restype_checker( + restype: Option<&PyObject>, + vm: &VirtualMachine, +) -> PyResult> { + match restype { + Some(restype) => vm.get_attribute_opt(restype, "_check_retval_"), + _ => Ok(None), + } +} + /// Extract call information (argtypes, restype) from PyCFuncPtr fn extract_call_info(zelf: &Py, vm: &VirtualMachine) -> PyResult { // Clone before calling Python sequence methods, which can change argtypes. let argtypes = zelf.argtypes.read().clone(); - // Get argtypes - first from instance, then from type's _argtypes_ + // Get argtypes from the instance, then cached class metadata. let explicit_arg_types: Option> = if let Some(argtypes_obj) = argtypes.as_ref() { if !vm.is_none(argtypes_obj) { Some(extract_arg_types(argtypes_obj, vm)?) @@ -1165,9 +1221,9 @@ fn extract_call_info(zelf: &Py, vm: &VirtualMachine) -> PyResult use ConvParam } } else if let Some(class_argtypes) = zelf - .as_object() .class() - .get_attr(vm.ctx.intern_str("_argtypes_")) + .stg_info_opt() + .and_then(|info| info.argtypes.clone()) && !vm.is_none(&class_argtypes) { Some(extract_arg_types(&class_argtypes, vm)?) @@ -1175,12 +1231,20 @@ fn extract_call_info(zelf: &Py, vm: &VirtualMachine) -> PyResult use ConvParam }; - // Get restype - first from instance, then from class's _restype_ - let restype_obj = zelf.restype.read().clone().or_else(|| { - zelf.as_object() - .class() - .get_attr(vm.ctx.intern_str("_restype_")) - }); + // Retain both references before argument conversion can re-enter. + let (restype_obj, checker) = { + let restype = zelf.restype.read(); + let checker = zelf.checker.read(); + let class_info = zelf.class().stg_info_opt(); + ( + restype + .clone() + .or_else(|| class_info.as_ref().and_then(|info| info.restype.clone())), + checker + .clone() + .or_else(|| class_info.as_ref().and_then(|info| info.checker.clone())), + ) + }; // Check if restype is explicitly None (return void) let restype_is_none = restype_obj.as_deref().is_some_and(|t| vm.is_none(t)); @@ -1189,6 +1253,10 @@ fn extract_call_info(zelf: &Py, vm: &VirtualMachine) -> PyResult::_flags_(zelf, vm) & StgInfoFlags::FUNCFLAG_HRESULT.bits() + != 0, ret, }) } @@ -1243,10 +1311,10 @@ fn resolve_com_method( zelf: &Py, args: &FuncArgs, vm: &VirtualMachine, -) -> PyResult<(Option, bool)> { +) -> PyResult<(Option, Option)> { let com_index = zelf.index.read(); let Some(idx) = *com_index else { - return Ok((None, false)); + return Ok((None, None)); }; // First arg must be the COM object pointer @@ -1282,7 +1350,7 @@ fn resolve_com_method( } }; - Ok((Some(code_ptr), true)) + Ok((Some(code_ptr), Some(com_ptr))) } /// Single argument for FFI call @@ -1509,36 +1577,67 @@ fn ctypes_callproc( /// Check and handle HRESULT errors (Windows) #[cfg(windows)] -fn check_hresult(hresult: i32, zelf: &Py, vm: &VirtualMachine) -> PyResult<()> { +fn check_hresult( + hresult: i32, + zelf: &Py, + com_ptr: Option, + vm: &VirtualMachine, +) -> PyResult<()> { if hresult >= 0 { return Ok(()); } - if zelf.iid.read().is_some() { - // Raise COMError + let iid = zelf.iid.read().clone(); + if let (Some(com_ptr), Some(iid)) = (com_ptr, iid) { + let iid: [u8; 16] = { + let buffer = ArgBytesLike::try_from_object(vm, iid)?; + let bytes = buffer.borrow_buf(); + bytes + .get(..16) + .ok_or_else(|| vm.new_value_error("IID buffer is too small"))? + .try_into() + .expect("checked IID size") + }; + // The call arguments keep the COM pointer owner alive. No Python data + // guard survives detaching; the native helper owns/releases its COM refs. + let details = vm.allow_threads(|| unsafe { + rustpython_host_env::ctypes::com_error_details(com_ptr, &iid) + }); + let wide = |value: Option>| -> PyObjectRef { + value.map_or_else( + || vm.ctx.none(), + |units| { + vm.ctx + .new_str(rustpython_common::wtf8::Wtf8Buf::from_wide(&units)) + .into() + }, + ) + }; + let details: PyObjectRef = vm + .ctx + .new_tuple(vec![ + wide(details.description), + wide(details.source), + wide(details.helpfile), + vm.ctx.new_int(details.help_context).into(), + wide(details.progid), + ]) + .into(); + let text = rustpython_host_env::ctypes::format_com_error_message(hresult) + .map_or_else(|| vm.ctx.none(), |text| vm.ctx.new_str(text).into()); let ctypes_module = vm.import("_ctypes", 0)?; let com_error_type = ctypes_module.get_attr("COMError", vm)?; let com_error_type = com_error_type .downcast::() .map_err(|_| vm.new_type_error("COMError is not a type"))?; - let hresult_obj: PyObjectRef = vm.ctx.new_int(hresult).into(); - let text: PyObjectRef = vm - .ctx - .new_str(format!("HRESULT: 0x{:08X}", hresult as u32)) - .into(); - let details: PyObjectRef = vm.ctx.none(); - let exc = vm.invoke_exception(&com_error_type, vec![text.to_owned(), details.clone()])?; - let _ = exc.as_object().set_attr("hresult", hresult_obj, vm); - let _ = exc.as_object().set_attr("text", text, vm); - let _ = exc.as_object().set_attr("details", details, vm); - Err(exc) + Err(vm.invoke_exception( + &com_error_type, + vec![vm.ctx.new_int(hresult).into(), text, details], + )?) } else { - // Raise OSError - let exc = vm.new_os_error(format!("HRESULT: 0x{:08X}", hresult as u32)); - let _ = exc - .as_object() - .set_attr("winerror", vm.ctx.new_int(hresult), vm); - Err(exc) + // Use the same native Windows error conversion as _check_HRESULT: + // errno mapping, signed winerror, and the system's formatted message. + super::_ctypes::check_hresult(hresult, vm).map(|_| ()) } } @@ -1610,10 +1709,11 @@ fn convert_raw_result( let is_pointer_type = info.flags.contains(StgInfoFlags::TYPEFLAG_ISPOINTER); let has_proto = info.proto.is_some(); let scalar_code = info.scalar_code; + let storage_size = info.size; drop(info); // py_object: interpret return value as PyObject* and materialize it. - if scalar_code == Some('O') { + if scalar_code == Some('O') && super::simple::returns_scalar(&restype_type) { let ptr = NonNull::new(result_word as *mut PyObject) .ok_or_else(|| vm.new_value_error("PyObject is NULL"))?; unsafe { @@ -1623,8 +1723,8 @@ fn convert_raw_result( } // 5. Simple type with getfunc → use bytes_to_pyobject (info->getfunc) - // is_simple_instance returns TRUE for c_int, c_void_p, etc. - if super::base::is_simple_instance(&restype_type) { + // Only direct simple types return Python scalars; subclasses return instances. + if super::simple::returns_scalar(&restype_type) { return Ok(Some(super::base::bytes_to_pyobject( &restype_type, &result_bytes, @@ -1632,6 +1732,22 @@ fn convert_raw_result( ))); } + if restype_type.fast_issubclass(PyCSimple::static_type()) { + // Native return values bypass Python __new__ and __init__. A py_object + // return transfers an owned reference, retained by the wrapper. + let objects = if scalar_code == Some('O') { + NonNull::new(result_word as *mut PyObject) + .map(|ptr| unsafe { PyObjectRef::from_raw(ptr) }) + } else { + None + }; + let mut bytes = vec![0; storage_size]; + write_prefix_limited(&mut bytes, &result_bytes, result_size); + return PyCSimple(PyCData::from_bytes(bytes, objects)) + .into_ref_with_type(vm, restype_type) + .map(|value| Some(value.into())); + } + // 6. Complex type → create ctypes instance (PyCData_FromBaseObj) // This handles POINTER(T), Structure, Array, etc. @@ -1688,26 +1804,41 @@ fn build_result( out_buffers: OutBuffers, zelf: &Py, args: &FuncArgs, + #[cfg(windows)] com_ptr: Option, vm: &VirtualMachine, ) -> PyResult { - // Check HRESULT on Windows - #[cfg(windows)] - if let CallValue::Scalar(bytes) = &call_result { - let is_hresult = call_info - .restype_obj - .as_ref() - .and_then(|t| t.to_owned().downcast::().ok()) - .is_some_and(|t| t.name().to_string() == "HRESULT"); - if is_hresult { - let mut word = [0u8; size_of::()]; - let n = bytes.len().min(word.len()); - word[..n].copy_from_slice(&bytes[..n]); - check_hresult(usize::from_ne_bytes(word) as i32, zelf, vm)?; + let convert_result = || -> PyResult> { + let mut result = convert_raw_result(&call_result, call_info, vm)?; + if let Some(checker) = &call_info.checker + && call_info + .restype_obj + .as_ref() + .and_then(|restype| restype.downcast_ref::()) + .is_some_and(|restype| restype.stg_info_opt().is_some()) + && let Some(value) = result.as_ref() + { + result = Some(checker.call((value.clone(),), vm)?); } - } + Ok(result) + }; - // Convert the foreign-call result to a Python object - let result = convert_raw_result(&call_result, call_info, vm)?; + // COM functions request HRESULT checking through their calling convention. + // Ordinary HRESULT restypes use _check_retval_, like other checked types. + #[cfg(windows)] + let result = if (call_info.check_hresult || (com_ptr.is_some() && zelf.iid.read().is_some())) + && let CallValue::Scalar(bytes) = &call_result + { + let mut word = [0u8; size_of::()]; + let n = bytes.len().min(word.len()); + word[..n].copy_from_slice(&bytes[..n]); + let hresult = usize::from_ne_bytes(word) as i32; + check_hresult(hresult, zelf, com_ptr, vm)?; + Some(vm.ctx.new_int(hresult).into()) + } else { + convert_result()? + }; + #[cfg(not(windows))] + let result = convert_result()?; // Apply errcheck if set // Clone before calling Python so the callback can change errcheck. @@ -1745,7 +1876,9 @@ impl Callable for PyCFuncPtr { // 2. Resolve function pointer (COM or direct) #[cfg(windows)] - let (func_ptr, is_com_method) = resolve_com_method(zelf, &args, vm)?; + let (func_ptr, com_ptr) = resolve_com_method(zelf, &args, vm)?; + #[cfg(windows)] + let is_com_method = com_ptr.is_some(); #[cfg(not(windows))] let (func_ptr, is_com_method) = (None::, false); @@ -1817,7 +1950,16 @@ impl Callable for PyCFuncPtr { } // 9. Build result - build_result(call_result, &call_info, out_buffers, zelf, &args, vm) + build_result( + call_result, + &call_info, + out_buffers, + zelf, + &args, + #[cfg(windows)] + com_ptr, + vm, + ) } } @@ -1860,36 +2002,57 @@ impl AsBuffer for PyCFuncPtr { impl Py { // restype getter/setter #[pygetset] - fn restype(zelf: &Self, vm: &VirtualMachine) -> Option { - let restype = zelf.restype.read().clone(); - restype.or_else(|| { - zelf.as_object() - .class() - .get_attr(vm.ctx.intern_str("_restype_")) + fn restype(zelf: &Self, vm: &VirtualMachine) -> PyResult> { + zelf.restype_operation.with(vm, || { + let restype = zelf.restype.read().clone(); + Ok(restype.or_else(|| { + zelf.class() + .stg_info_opt() + .and_then(|info| info.restype.clone()) + })) }) } #[pygetset(setter)] - fn set_restype(&self, value: PyObjectRef, vm: &VirtualMachine) -> PyResult<()> { - // Must be type, callable, or None - if !vm.is_none(&value) && value.downcast_ref::().is_none() && !value.is_callable() { - return Err(vm.new_type_error("restype must be a type, a callable, or None")); - } - // An explicit None requests a void result; it is distinct from unset. - let old = self.restype.write().replace(value); - // Finalizers on the previous value can re-enter this setter. - drop(old); - Ok(()) + fn set_restype(&self, value: PySetterValue, vm: &VirtualMachine) -> PyResult<()> { + self.restype_operation.with(vm, || { + let PySetterValue::Assign(value) = value else { + // Deletion uses the reverse replacement order from assignment. + // Drop each value with no data guard held, allowing finalizers + // to observe and re-enter the current state, as in CPython. + let old_restype = self.restype.write().take(); + drop(old_restype); + let old_checker = self.checker.write().take(); + drop(old_checker); + return Ok(()); + }; + // Must be type, callable, or None + if !vm.is_none(&value) + && value.downcast_ref::().is_none() + && !value.is_callable() + { + return Err(vm.new_type_error("restype must be a type, a callable, or None")); + } + let checker = restype_checker(Some(&value), vm)?; + // An explicit None requests a void result; it is distinct from unset. + // Match CPython's sequential replacement: the old checker's finalizer + // can still observe the old restype and can re-enter this setter. + let old_checker = core::mem::replace(&mut *self.checker.write(), checker); + drop(old_checker); + let old_restype = self.restype.write().replace(value); + drop(old_restype); + Ok(()) + }) } // argtypes getter/setter #[pygetset] - fn argtypes(zelf: &Self, vm: &VirtualMachine) -> Option { + fn argtypes(zelf: &Self, _vm: &VirtualMachine) -> Option { let argtypes = zelf.argtypes.read().clone(); argtypes.or_else(|| { - zelf.as_object() - .class() - .get_attr(vm.ctx.intern_str("_argtypes_")) + zelf.class() + .stg_info_opt() + .and_then(|info| info.argtypes.clone()) }) } diff --git a/crates/vm/src/stdlib/_ctypes/simple.rs b/crates/vm/src/stdlib/_ctypes/simple.rs index 65e29ec9b0b..a7dbf9efd30 100644 --- a/crates/vm/src/stdlib/_ctypes/simple.rs +++ b/crates/vm/src/stdlib/_ctypes/simple.rs @@ -753,11 +753,7 @@ impl Representable for PyCSimple { // Check if base is _SimpleCData (direct simple type like c_int, c_char) // vs subclass of simple type (like class X(c_int): pass) - let bases = cls.bases.read(); - let is_direct_simple = bases - .as_slice() - .iter() - .any(|base| base.name().to_string() == "_SimpleCData"); + let is_direct_simple = returns_scalar(cls); if is_direct_simple { // Direct SimpleCData: "typename(repr(value))" @@ -895,15 +891,7 @@ impl Py { ) -> PyResult { // _ctypes_simple_instance: returns true if NOT a direct subclass of Simple_Type // i.e., c_int (direct) -> false, MyInt(c_int) (subclass) -> true - let is_subclass_of_simple = { - let cls = zelf.class(); - let bases = cls.bases.read(); - // If base is NOT _SimpleCData, then it's a subclass of a subclass - !bases - .as_slice() - .iter() - .any(|base| base.name().to_string() == "_SimpleCData") - }; + let is_subclass_of_simple = !returns_scalar(zelf.class()); if is_subclass_of_simple { // Subclass of simple type (e.g., MyInt(c_int)): return self diff --git a/crates/vm/src/types/slot.rs b/crates/vm/src/types/slot.rs index 3e89d0ecf0d..1f2fd67bd09 100644 --- a/crates/vm/src/types/slot.rs +++ b/crates/vm/src/types/slot.rs @@ -2211,26 +2211,14 @@ pub trait Initializer: PyPayload { #[inline] #[pyslot] fn slot_init(zelf: &PyObject, args: FuncArgs, vm: &VirtualMachine) -> PyResult<()> { - #[cfg(debug_assertions)] - let class_name_for_debug = zelf.class().name().to_string(); - let zelf = match zelf.try_to_ref::(vm) { Ok(zelf) => zelf, Err(err) => { - #[cfg(debug_assertions)] - { - if let Ok(msg) = err.as_object().repr(vm) { - let double_appearance = msg - .to_string_lossy() - .matches(&class_name_for_debug as &str) - .count() - == 2; - assert!( - !double_appearance, - "This type `{class_name_for_debug}` doesn't seem to support `init`. Override `slot_init` instead: {msg}" - ) - } - } + debug_assert!( + !zelf.fast_isinstance(Self::class(&vm.ctx)), + "This type `{}` doesn't seem to support `init`. Override `slot_init` instead", + zelf.class().name() + ); return Err(err); } }; diff --git a/extra_tests/snippets/builtin_exceptions.py b/extra_tests/snippets/builtin_exceptions.py index 080294a3c8a..388beb42e07 100644 --- a/extra_tests/snippets/builtin_exceptions.py +++ b/extra_tests/snippets/builtin_exceptions.py @@ -410,3 +410,22 @@ class SubError(MyError): assert restored.args == ("m",) assert restored.name == "n" assert restored.path == "p" + + +# Same-named subclasses must not take the builtin's errno dispatch path. +def check_oserror_type_identity(): + import errno + + class OSError(builtins.OSError): + pass + + class RenamedError(builtins.OSError): + pass + + RenamedError.__name__ = "OSError" + for cls in (OSError, RenamedError): + for code in (errno.ENOENT, errno.EACCES): + assert type(cls(code, "message")) is cls + + +check_oserror_type_identity() diff --git a/extra_tests/snippets/builtin_set.py b/extra_tests/snippets/builtin_set.py index 05edaaebeef..1f05d200834 100644 --- a/extra_tests/snippets/builtin_set.py +++ b/extra_tests/snippets/builtin_set.py @@ -728,3 +728,22 @@ def __del__(self): for predicate in (set().issuperset, frozenset().isdisjoint): with assert_raises(TypeError): predicate([{1}]) + + +# A subclass named like the builtin still needs the subclass repr wrapper. +SetNamedSet = type("set", (set,), {}) +assert repr(SetNamedSet([1])) == "set({1})" +assert repr(SetNamedSet()) == "set()" +assert repr({1}) == "{1}" + + +class RenamedSet(set): + pass + + +renamed_set = RenamedSet([1]) +assert repr(renamed_set) == "RenamedSet({1})" +RenamedSet.__name__ = "set" +assert repr(renamed_set) == "set({1})" +RenamedSet.__name__ = "RenamedAgain" +assert repr(renamed_set) == "RenamedAgain({1})" diff --git a/extra_tests/snippets/stdlib_ctypes.py b/extra_tests/snippets/stdlib_ctypes.py index b29eed45d16..71ca7aa5a21 100644 --- a/extra_tests/snippets/stdlib_ctypes.py +++ b/extra_tests/snippets/stdlib_ctypes.py @@ -1276,3 +1276,519 @@ def __index__(self): _pointer[0] = _value assert _original.value is _object assert _redirected.value is (None if _value is None else _object) + + +# These identity and checker-cache cases extend the canonical ctypes tests. +import sys as _sys +import unittest as _unittest + + +class _CtypesTypeIdentityTests(_unittest.TestCase): + def test_callback_requires_class_local_argtypes_metadata(self): + class Base(ctypes._CFuncPtr): + _flags_ = ctypes._FUNCFLAG_CDECL + _argtypes_ = () + _restype_ = c_int + + class Child(Base): + _flags_ = ctypes._FUNCFLAG_CDECL + _restype_ = c_int + + for _ in range(2): + with self.assertRaisesRegex(TypeError, "no argtypes"): + Child(lambda: 37) + Child._argtypes_ = () + + callback = ctypes.CFUNCTYPE(c_int)(lambda: 37) + ordinary = Child(ctypes.cast(callback, ctypes.c_void_p).value) + self.assertIsNone(ordinary.argtypes) + self.assertEqual(ordinary(), 37) + + def test_function_uses_cached_argtypes_metadata(self): + class Prototype(ctypes._CFuncPtr): + _flags_ = ctypes._FUNCFLAG_CDECL + _argtypes_ = () + _restype_ = c_int + + Prototype._argtypes_ = (c_int,) + function = Prototype(lambda: 37) + self.assertEqual(function.argtypes, ()) + self.assertEqual(function(), 37) + ordinary = Prototype(ctypes.cast(function, ctypes.c_void_p).value) + self.assertEqual(ordinary.argtypes, ()) + self.assertEqual(ordinary(), 37) + ordinary.argtypes = (c_int,) + self.assertEqual(ordinary.argtypes, (c_int,)) + ordinary.argtypes = None + self.assertEqual(ordinary.argtypes, ()) + self.assertEqual(ordinary(), 37) + + @_unittest.skipUnless( + _sys.implementation.name == "rustpython", + "CPython 3.15 crashes on missing callback restype", + ) + def test_callback_missing_restype_is_rejected_safely(self): + class Prototype(ctypes._CFuncPtr): + _flags_ = ctypes._FUNCFLAG_CDECL + _argtypes_ = () + + with self.assertRaisesRegex( + TypeError, "invalid result type for callback function" + ): + Prototype(lambda: 37) + + def test_restype_deletion_restores_class_checker(self): + class Checked(c_int): + _check_retval_ = staticmethod(lambda value: ("class", value.value)) + + class Prototype(ctypes._CFuncPtr): + _flags_ = ctypes._FUNCFLAG_CDECL + _argtypes_ = () + _restype_ = Checked + + callback = ctypes.CFUNCTYPE(c_int)(lambda: 23) + function = Prototype(ctypes.cast(callback, ctypes.c_void_p).value) + function.restype = None + del function.restype + self.assertIs(function.restype, Checked) + self.assertEqual(function(), ("class", 23)) + del function.restype + self.assertEqual(function(), ("class", 23)) + + def test_restype_deletion_reenters_before_clearing_checker(self): + function = ctypes.CFUNCTYPE(c_int)(lambda: 17) + events = [] + + class Nested(c_int): + _check_retval_ = staticmethod(lambda value: ("nested", value.value)) + + class Restype: + def __call__(self, value): + return value + + def __del__(self): + events.append(function.restype) + function.restype = Nested + + function.restype = Restype() + del function.restype + self.assertEqual(events, [c_int]) + self.assertIs(function.restype, Nested) + result = function() + self.assertIs(type(result), Nested) + self.assertEqual(result.value, 17) + + def test_redundant_simple_base_uses_primary_base(self): + class Redundant(c_int, ctypes._SimpleCData): + pass + + value = Redundant(42) + self.assertIs(Redundant.__base__, c_int) + self.assertTrue(repr(value).startswith("