FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Validate native subclass object layouts by 1ndahous3 · Pull Request #8905 · RustPython/RustPython · GitHub

Repository navigation

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension .rs  (6) All 1 file type selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
44 changes: 33 additions & 11 deletions crates/derive-impl/src/pyclass.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand All @@ -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!(
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
};
Expand All @@ -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!(
Expand All @@ -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! {},
};
Expand Down
3 changes: 3 additions & 0 deletions crates/derive/src/lib.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions crates/vm/src/builtins/dict.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down
75 changes: 75 additions & 0 deletions crates/vm/src/object/core.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
36 changes: 36 additions & 0 deletions crates/vm/src/object/payload.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -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>();

Expand Down
2 changes: 2 additions & 0 deletions crates/vm/src/stdlib/_io.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading

Back | FazBrowse Home | New Git URL