From 55a5f1c60007f0e3ee71ed696586d925cb42d9fb Mon Sep 17 00:00:00 2001 From: 1ndahous3 <9205230+1ndahous3@users.noreply.github.com> Date: Tue, 29 Sep 2026 20:11:38 +0300 Subject: [PATCH] Validate native subclass object layouts (#8905) Assisted-by: Codex:GPT-6 --- crates/derive-impl/src/pyclass.rs | 44 +++++++++++++----- crates/derive/src/lib.rs | 3 ++ crates/vm/src/builtins/dict.rs | 2 + crates/vm/src/object/core.rs | 75 +++++++++++++++++++++++++++++++ crates/vm/src/object/payload.rs | 36 +++++++++++++++ crates/vm/src/stdlib/_io.rs | 2 + 6 files changed, 151 insertions(+), 11 deletions(-) diff --git a/crates/derive-impl/src/pyclass.rs b/crates/derive-impl/src/pyclass.rs index 4b613629e..61ff932da 100644 --- a/crates/derive-impl/src/pyclass.rs +++ b/crates/derive-impl/src/pyclass.rs @@ -348,11 +348,10 @@ pub(crate) fn impl_pyclass_impl(attr: PunctuatedNestedMeta, item: Item) -> Resul /// Validates that when a base class is specified, the struct has the base type as its first /// *declared* field. Returns a token naming that field (e.g. `_base` or `0` for tuple structs) -/// so the caller can emit a compile-time `offset_of!` assertion, or `None` for non-structs. -fn validate_base_field(item: &Item, base_path: &syn::Path) -> Result> { +/// so the caller can emit compile-time layout assertions. +fn validate_base_field(item: &Item, base_path: &syn::Path) -> Result { let Item::Struct(item_struct) = item else { - // Only validate structs - enums with base are already an error elsewhere - return Ok(None); + bail_span!(item, "#[pyclass] with base requires a struct"); }; // Get the base type name for error messages @@ -375,8 +374,11 @@ fn validate_base_field(item: &Item, base_path: &syn::Path) -> Result { let Some(first_field) = fields.unnamed.first() else { @@ -391,7 +393,7 @@ fn validate_base_field(item: &Item, base_path: &syn::Path) -> Result { bail_span!( @@ -591,12 +593,12 @@ fn generate_class_def( }); // If repr(transparent) with a base, the type has the same memory layout as base, // so basicsize stays 0 and type creation copies the base's full tp_basicsize. - // Otherwise, basicsize is the object header plus the payload. + // Otherwise, include any alignment padding between the header and payload. let basicsize = if is_repr_transparent && base.is_some() { quote!(0) } else { quote!( - ::rustpython_vm::object::SIZEOF_PYOBJECT_HEAD + ::core::mem::size_of::<#ident>() + ::rustpython_vm::object::payload_offset::<#ident>() + ::core::mem::size_of::<#ident>() ) }; if base.is_some() && is_pystruct { @@ -731,8 +733,10 @@ pub(crate) fn impl_pyclass(attr: PunctuatedNestedMeta, item: Item) -> Result and Py. + // 5. Require Py to meet Py's alignment, including packed payloads. let base_field_token = if let Some(ref base_path) = base { - validate_base_field(&item, base_path)? + Some(validate_base_field(&item, base_path)?) } else { None }; @@ -747,7 +751,7 @@ pub(crate) fn impl_pyclass(attr: PunctuatedNestedMeta, item: Item) -> Result quote! { + (Some(base_type), Some(field)) => quote! { const _: () = ::core::assert!( ::core::mem::offset_of!(#ident, #field) == 0, concat!( @@ -757,6 +761,24 @@ pub(crate) fn impl_pyclass(attr: PunctuatedNestedMeta, item: Item) -> Result() + == ::rustpython_vm::object::payload_offset::<#base_type>(), + concat!( + "The payload offsets of `", stringify!(#ident), "` and its base `", + stringify!(#base_type), "` differ inside `Py`. \ + Adjust the payload alignment so inherited methods read the same address. \ + A base field at offset 0 alone is not sufficient." + ) + ); + const _: () = ::core::assert!( + ::core::mem::align_of::<::rustpython_vm::Py<#ident>>() + >= ::core::mem::align_of::<::rustpython_vm::Py<#base_type>>(), + concat!( + "The object alignment of `", stringify!(#ident), "` is smaller than its base `", + stringify!(#base_type), "`. Packed payloads must not weaken the base object's alignment." + ) + ); }, _ => quote! {}, }; diff --git a/crates/derive/src/lib.rs b/crates/derive/src/lib.rs index d9564da7f..d6cd7bc2b 100644 --- a/crates/derive/src/lib.rs +++ b/crates/derive/src/lib.rs @@ -116,6 +116,9 @@ pub fn derive_from_args(input: TokenStream) -> TokenStream { /// - `name`: the name of the Python class, by default it is the name of the struct. /// - `base`: the base class of the Python class. /// This does not cause inheritance of functions or attributes that must be done by a separate trait. +/// The native payload must be a struct with the base as its first field. +/// The macro adds `repr(C)` if no explicit representation is present and +/// checks the base field offset, the payload offset in `Py`, and object alignment. /// # Impl /// This part implements `PyClassImpl` for the struct. /// This includes methods, getters/setters, etc.; only annotated methods will be included. diff --git a/crates/vm/src/builtins/dict.rs b/crates/vm/src/builtins/dict.rs index 7c5291617..d709521ea 100644 --- a/crates/vm/src/builtins/dict.rs +++ b/crates/vm/src/builtins/dict.rs @@ -33,6 +33,8 @@ pub(crate) type DictContentType = dict_inner::Dict; #[pyclass(module = false, name = "dict", unhashable = true, traverse = "manual")] #[derive(Default)] +// OrderedDict contains eight-byte-aligned fields on 32-bit targets too. +#[repr(align(8))] pub struct PyDict { entries: DictContentType, } diff --git a/crates/vm/src/object/core.rs b/crates/vm/src/object/core.rs index 4f98bd7e2..c259ff03c 100644 --- a/crates/vm/src/object/core.rs +++ b/crates/vm/src/object/core.rs @@ -3247,6 +3247,81 @@ pub(crate) fn init_type_hierarchy() -> BootstrapTypeHierarchy { mod tests { use super::*; + #[test] + fn native_type_basicsize_includes_payload_padding() { + use crate::class::PyClassDef; + + #[pyclass(module = false, name = "PaddedPayload")] + #[derive(Debug, PyPayload)] + #[repr(align(64))] + struct PaddedPayload; + + #[pyclass] + impl PaddedPayload {} + + assert_eq!( + PaddedPayload::BASICSIZE, + core::mem::size_of::>() + ); + } + + #[test] + fn native_subclass_inherits_getter_with_mixed_field_sizes() { + use crate::class::PyClassImpl; + + #[pyclass(module = false, name = "LayoutBase")] + #[derive(Debug, PyPayload)] + // Keep the base and derived payload aligned alike on 32-bit targets too. + #[repr(align(8))] + struct LayoutBase { + value: PyObjectRef, + } + + #[pyclass(flags(BASETYPE))] + impl Py { + #[pygetset] + fn value(&self) -> PyObjectRef { + self.value.clone() + } + } + + #[pyclass(module = false, name = "LayoutDerived", base = LayoutBase)] + #[derive(Debug)] + struct LayoutDerived { + base: LayoutBase, + extra: Option, + } + + #[pyclass] + impl LayoutDerived { + #[pygetset] + fn extra(&self) -> Option { + self.extra + } + } + + assert_eq!(core::mem::offset_of!(LayoutDerived, base), 0); + crate::Interpreter::without_stdlib(Default::default()).enter(|vm| { + let _ = LayoutBase::make_static_type(); + let _ = LayoutDerived::make_static_type(); + let value: PyObjectRef = vm.ctx.new_int(42).into(); + let obj = vm.new_pyobj(LayoutDerived { + base: LayoutBase { + value: value.clone(), + }, + extra: Some(99), + }); + assert!(obj.get_attr("value", vm).unwrap().is(&value)); + assert_eq!( + obj.get_attr("extra", vm) + .unwrap() + .try_to_value::(vm) + .unwrap(), + 99 + ); + }); + } + #[test] fn clear_reuses_storage_and_preserves_existing_edges() { use crate::builtins::PyList; diff --git a/crates/vm/src/object/payload.rs b/crates/vm/src/object/payload.rs index 632086095..a5a4bf35b 100644 --- a/crates/vm/src/object/payload.rs +++ b/crates/vm/src/object/payload.rs @@ -27,6 +27,42 @@ pub(crate) fn cold_downcast_type_error( vm.new_downcast_type_error(class, obj) } +/// Native subclasses declared with `#[pyclass(base = Base)]` share the base's +/// payload type ID. Their base field must start at offset zero, and their +/// payload must start at the same offset inside `Py` as the base payload. +/// The derived object must also meet the base object's alignment requirement. +/// The macro checks these conditions at compile time. `repr(C)` on the payload +/// alone does not guarantee the second condition: +/// +/// ```compile_fail,E0080 +/// use rustpython_vm::{builtins::PyDict, pyclass}; +/// +/// #[pyclass(module = false, name = "MisalignedDict", base = PyDict)] +/// #[derive(Debug)] +/// #[repr(C, align(64))] +/// struct MisalignedDict { +/// base: PyDict, +/// } +/// +/// #[pyclass] +/// impl MisalignedDict {} +/// ``` +/// +/// An enum cannot provide the required base field layout: +/// +/// ```compile_fail +/// use rustpython_vm::{builtins::PyDict, pyclass}; +/// +/// #[pyclass(module = false, name = "EnumDict", base = PyDict)] +/// #[derive(Debug)] +/// enum EnumDict { +/// Dict(PyDict), +/// Empty, +/// } +/// +/// #[pyclass] +/// impl EnumDict {} +/// ``` pub trait PyPayload: MaybeTraverse + PyThreadingConstraint + Sized + 'static { const PAYLOAD_TYPE_ID: core::any::TypeId = core::any::TypeId::of::(); diff --git a/crates/vm/src/stdlib/_io.rs b/crates/vm/src/stdlib/_io.rs index 401476fbc..d4cd66e14 100644 --- a/crates/vm/src/stdlib/_io.rs +++ b/crates/vm/src/stdlib/_io.rs @@ -514,6 +514,8 @@ mod _io { #[pyattr] #[pyclass(name = "_IOBase")] #[derive(Debug, Default, PyPayload)] + // Buffered and text subclasses contain eight-byte-aligned fields on 32-bit targets. + #[repr(align(8))] pub(super) struct _IOBase; #[pyclass(