Validate native subclass object layouts (#8905)

Assisted-by: Codex:GPT-6

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<Option<TokenStream>> {
+/// so the caller can emit compile-time layout assertions.
+fn validate_base_field(item: &Item, base_path: &syn::Path) -> Result<TokenStream> {
     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<Option<Toke
                     "#[pyclass] with base = {base_name} requires the first field to be of type {base_name}"
                 );
             }
-            let ident = first_field.ident.as_ref().map(|id| quote! { #id });
-            Ok(ident)
+            let ident = first_field
+                .ident
+                .as_ref()
+                .expect("named fields always have identifiers");
+            Ok(quote! { #ident })
         }
         syn::Fields::Unnamed(fields) => {
             let Some(first_field) = fields.unnamed.first() else {
@@ -391,7 +393,7 @@ fn validate_base_field(item: &Item, base_path: &syn::Path) -> Result<Option<Toke
                     "#[pyclass] with base = {base_name} requires the first field to be of type {base_name}"
                 );
             }
-            Ok(Some(quote! { 0 }))
+            Ok(quote! { 0 })
         }
         syn::Fields::Unit => {
             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<Tok
     //      keeping the base field at offset 0 as the inherited getter dispatcher requires.
     //   3. Emit a compile-time offset_of! assertion as a safety net for structs that
     //      already carry an explicit repr that does not guarantee offset 0.
+    //   4. Require the payload to start at the same offset in Py<Base> and Py<Derived>.
+    //   5. Require Py<Derived> to meet Py<Base>'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<Tok
     let (ident, attrs) = pyclass_ident_and_attrs(&item)?;
 
     let offset_assert = match (&base, &base_field_token) {
-        (Some(_), Some(field)) => 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<Tok
                      reads the correct memory."
                 )
             );
+            const _: () = ::core::assert!(
+                ::rustpython_vm::object::payload_offset::<#ident>()
+                    == ::rustpython_vm::object::payload_offset::<#base_type>(),
+                concat!(
+                    "The payload offsets of `", stringify!(#ident), "` and its base `",
+                    stringify!(#base_type), "` differ inside `Py<T>`. \
+                     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! {},
     };

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<T>`, and object alignment.
 /// # Impl
 /// This part implements `PyClassImpl` for the struct.
 /// This includes methods, getters/setters, etc.; only annotated methods will be included.

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,
 }

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::<Py<PaddedPayload>>()
+        );
+    }
+
+    #[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<LayoutBase> {
+            #[pygetset]
+            fn value(&self) -> PyObjectRef {
+                self.value.clone()
+            }
+        }
+
+        #[pyclass(module = false, name = "LayoutDerived", base = LayoutBase)]
+        #[derive(Debug)]
+        struct LayoutDerived {
+            base: LayoutBase,
+            extra: Option<u64>,
+        }
+
+        #[pyclass]
+        impl LayoutDerived {
+            #[pygetset]
+            fn extra(&self) -> Option<u64> {
+                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::<u64>(vm)
+                    .unwrap(),
+                99
+            );
+        });
+    }
+
     #[test]
     fn clear_reuses_storage_and_preserves_existing_edges() {
         use crate::builtins::PyList;

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<T>` 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::<Self>();
 

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(