Skip to content

Commit

Permalink
avoid calling PyType_GetSlot on static types before Python 3.10 (#4599
Browse files Browse the repository at this point in the history
)

* avoid calling `PyType_GetSlot` on static types before Python 3.10

* use the correct tp_free

* fixup
  • Loading branch information
davidhewitt committed Oct 11, 2024
1 parent 10cd042 commit 73b061b
Show file tree
Hide file tree
Showing 8 changed files with 279 additions and 59 deletions.
1 change: 1 addition & 0 deletions newsfragments/4599.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix crash calling `PyType_GetSlot` on static types before Python 3.10.
3 changes: 3 additions & 0 deletions src/internal.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
//! Holding place for code which is not intended to be reachable from outside of PyO3.

pub(crate) mod get_slot;
234 changes: 234 additions & 0 deletions src/internal/get_slot.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,234 @@
use crate::{
ffi,
types::{PyType, PyTypeMethods},
Borrowed, Bound,
};
use std::os::raw::c_int;

impl Bound<'_, PyType> {
#[inline]
pub(crate) fn get_slot<const S: c_int>(&self, slot: Slot<S>) -> <Slot<S> as GetSlotImpl>::Type
where
Slot<S>: GetSlotImpl,
{
slot.get_slot(self.as_borrowed())
}
}

impl Borrowed<'_, '_, PyType> {
#[inline]
pub(crate) fn get_slot<const S: c_int>(self, slot: Slot<S>) -> <Slot<S> as GetSlotImpl>::Type
where
Slot<S>: GetSlotImpl,
{
slot.get_slot(self)
}
}

pub(crate) trait GetSlotImpl {
type Type;
fn get_slot(self, tp: Borrowed<'_, '_, PyType>) -> Self::Type;
}

#[derive(Copy, Clone)]
pub(crate) struct Slot<const S: c_int>;

macro_rules! impl_slots {
($($name:ident: ($slot:ident, $field:ident) -> $tp:ty),+ $(,)?) => {
$(
pub (crate) const $name: Slot<{ ffi::$slot }> = Slot;

impl GetSlotImpl for Slot<{ ffi::$slot }> {
type Type = $tp;

#[inline]
fn get_slot(self, tp: Borrowed<'_, '_, PyType>) -> Self::Type {
let ptr = tp.as_type_ptr();

#[cfg(not(Py_LIMITED_API))]
unsafe {
(*ptr).$field
}

#[cfg(Py_LIMITED_API)]
{
#[cfg(not(Py_3_10))]
{
// Calling PyType_GetSlot on static types is not valid before Python 3.10
// ... so the workaround is to first do a runtime check for these versions
// (3.7, 3.8, 3.9) and then look in the type object anyway. This is only ok
// because we know that the interpreter is not going to change the size
// of the type objects for these historical versions.
if !is_runtime_3_10(tp.py())
&& unsafe { ffi::PyType_HasFeature(ptr, ffi::Py_TPFLAGS_HEAPTYPE) } == 0
{
return unsafe { (*ptr.cast::<PyTypeObject39Snapshot>()).$field };
}
}

// SAFETY: slot type is set carefully to be valid
unsafe { std::mem::transmute(ffi::PyType_GetSlot(ptr, ffi::$slot)) }
}
}
}
)*
};
}

// Slots are implemented on-demand as needed.
impl_slots! {
TP_ALLOC: (Py_tp_alloc, tp_alloc) -> Option<ffi::allocfunc>,
TP_DESCR_GET: (Py_tp_descr_get, tp_descr_get) -> Option<ffi::descrgetfunc>,
TP_FREE: (Py_tp_free, tp_free) -> Option<ffi::freefunc>,
}

#[cfg(all(Py_LIMITED_API, not(Py_3_10)))]
fn is_runtime_3_10(py: crate::Python<'_>) -> bool {
use crate::sync::GILOnceCell;

static IS_RUNTIME_3_10: GILOnceCell<bool> = GILOnceCell::new();
*IS_RUNTIME_3_10.get_or_init(py, || py.version_info() >= (3, 10))
}

#[repr(C)]
#[cfg(all(Py_LIMITED_API, not(Py_3_10)))]
pub struct PyNumberMethods39Snapshot {
pub nb_add: Option<ffi::binaryfunc>,
pub nb_subtract: Option<ffi::binaryfunc>,
pub nb_multiply: Option<ffi::binaryfunc>,
pub nb_remainder: Option<ffi::binaryfunc>,
pub nb_divmod: Option<ffi::binaryfunc>,
pub nb_power: Option<ffi::ternaryfunc>,
pub nb_negative: Option<ffi::unaryfunc>,
pub nb_positive: Option<ffi::unaryfunc>,
pub nb_absolute: Option<ffi::unaryfunc>,
pub nb_bool: Option<ffi::inquiry>,
pub nb_invert: Option<ffi::unaryfunc>,
pub nb_lshift: Option<ffi::binaryfunc>,
pub nb_rshift: Option<ffi::binaryfunc>,
pub nb_and: Option<ffi::binaryfunc>,
pub nb_xor: Option<ffi::binaryfunc>,
pub nb_or: Option<ffi::binaryfunc>,
pub nb_int: Option<ffi::unaryfunc>,
pub nb_reserved: *mut std::os::raw::c_void,
pub nb_float: Option<ffi::unaryfunc>,
pub nb_inplace_add: Option<ffi::binaryfunc>,
pub nb_inplace_subtract: Option<ffi::binaryfunc>,
pub nb_inplace_multiply: Option<ffi::binaryfunc>,
pub nb_inplace_remainder: Option<ffi::binaryfunc>,
pub nb_inplace_power: Option<ffi::ternaryfunc>,
pub nb_inplace_lshift: Option<ffi::binaryfunc>,
pub nb_inplace_rshift: Option<ffi::binaryfunc>,
pub nb_inplace_and: Option<ffi::binaryfunc>,
pub nb_inplace_xor: Option<ffi::binaryfunc>,
pub nb_inplace_or: Option<ffi::binaryfunc>,
pub nb_floor_divide: Option<ffi::binaryfunc>,
pub nb_true_divide: Option<ffi::binaryfunc>,
pub nb_inplace_floor_divide: Option<ffi::binaryfunc>,
pub nb_inplace_true_divide: Option<ffi::binaryfunc>,
pub nb_index: Option<ffi::unaryfunc>,
pub nb_matrix_multiply: Option<ffi::binaryfunc>,
pub nb_inplace_matrix_multiply: Option<ffi::binaryfunc>,
}

#[repr(C)]
#[cfg(all(Py_LIMITED_API, not(Py_3_10)))]
pub struct PySequenceMethods39Snapshot {
pub sq_length: Option<ffi::lenfunc>,
pub sq_concat: Option<ffi::binaryfunc>,
pub sq_repeat: Option<ffi::ssizeargfunc>,
pub sq_item: Option<ffi::ssizeargfunc>,
pub was_sq_slice: *mut std::os::raw::c_void,
pub sq_ass_item: Option<ffi::ssizeobjargproc>,
pub was_sq_ass_slice: *mut std::os::raw::c_void,
pub sq_contains: Option<ffi::objobjproc>,
pub sq_inplace_concat: Option<ffi::binaryfunc>,
pub sq_inplace_repeat: Option<ffi::ssizeargfunc>,
}

#[repr(C)]
#[cfg(all(Py_LIMITED_API, not(Py_3_10)))]
pub struct PyMappingMethods39Snapshot {
pub mp_length: Option<ffi::lenfunc>,
pub mp_subscript: Option<ffi::binaryfunc>,
pub mp_ass_subscript: Option<ffi::objobjargproc>,
}

#[repr(C)]
#[cfg(all(Py_LIMITED_API, not(Py_3_10)))]
pub struct PyAsyncMethods39Snapshot {
pub am_await: Option<ffi::unaryfunc>,
pub am_aiter: Option<ffi::unaryfunc>,
pub am_anext: Option<ffi::unaryfunc>,
}

#[repr(C)]
#[cfg(all(Py_LIMITED_API, not(Py_3_10)))]
pub struct PyBufferProcs39Snapshot {
// not available in limited api, but structure needs to have the right size
pub bf_getbuffer: *mut std::os::raw::c_void,
pub bf_releasebuffer: *mut std::os::raw::c_void,
}

/// Snapshot of the structure of PyTypeObject for Python 3.7 through 3.9.
///
/// This is used as a fallback for static types in abi3 when the Python version is less than 3.10;
/// this is a bit of a hack but there's no better option and the structure of the type object is
/// not going to change for those historical versions.
#[repr(C)]
#[cfg(all(Py_LIMITED_API, not(Py_3_10)))]
struct PyTypeObject39Snapshot {
pub ob_base: ffi::PyVarObject,
pub tp_name: *const std::os::raw::c_char,
pub tp_basicsize: ffi::Py_ssize_t,
pub tp_itemsize: ffi::Py_ssize_t,
pub tp_dealloc: Option<ffi::destructor>,
#[cfg(not(Py_3_8))]
pub tp_print: *mut std::os::raw::c_void, // stubbed out, not available in limited API
#[cfg(Py_3_8)]
pub tp_vectorcall_offset: ffi::Py_ssize_t,
pub tp_getattr: Option<ffi::getattrfunc>,
pub tp_setattr: Option<ffi::setattrfunc>,
pub tp_as_async: *mut PyAsyncMethods39Snapshot,
pub tp_repr: Option<ffi::reprfunc>,
pub tp_as_number: *mut PyNumberMethods39Snapshot,
pub tp_as_sequence: *mut PySequenceMethods39Snapshot,
pub tp_as_mapping: *mut PyMappingMethods39Snapshot,
pub tp_hash: Option<ffi::hashfunc>,
pub tp_call: Option<ffi::ternaryfunc>,
pub tp_str: Option<ffi::reprfunc>,
pub tp_getattro: Option<ffi::getattrofunc>,
pub tp_setattro: Option<ffi::setattrofunc>,
pub tp_as_buffer: *mut PyBufferProcs39Snapshot,
pub tp_flags: std::os::raw::c_ulong,
pub tp_doc: *const std::os::raw::c_char,
pub tp_traverse: Option<ffi::traverseproc>,
pub tp_clear: Option<ffi::inquiry>,
pub tp_richcompare: Option<ffi::richcmpfunc>,
pub tp_weaklistoffset: ffi::Py_ssize_t,
pub tp_iter: Option<ffi::getiterfunc>,
pub tp_iternext: Option<ffi::iternextfunc>,
pub tp_methods: *mut ffi::PyMethodDef,
pub tp_members: *mut ffi::PyMemberDef,
pub tp_getset: *mut ffi::PyGetSetDef,
pub tp_base: *mut ffi::PyTypeObject,
pub tp_dict: *mut ffi::PyObject,
pub tp_descr_get: Option<ffi::descrgetfunc>,
pub tp_descr_set: Option<ffi::descrsetfunc>,
pub tp_dictoffset: ffi::Py_ssize_t,
pub tp_init: Option<ffi::initproc>,
pub tp_alloc: Option<ffi::allocfunc>,
pub tp_new: Option<ffi::newfunc>,
pub tp_free: Option<ffi::freefunc>,
pub tp_is_gc: Option<ffi::inquiry>,
pub tp_bases: *mut ffi::PyObject,
pub tp_mro: *mut ffi::PyObject,
pub tp_cache: *mut ffi::PyObject,
pub tp_subclasses: *mut ffi::PyObject,
pub tp_weaklist: *mut ffi::PyObject,
pub tp_del: Option<ffi::destructor>,
pub tp_version_tag: std::os::raw::c_uint,
pub tp_finalize: Option<ffi::destructor>,
#[cfg(Py_3_8)]
pub tp_vectorcall: Option<ffi::vectorcallfunc>,
}
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,7 @@ mod tests;

#[macro_use]
mod internal_tricks;
mod internal;

pub mod buffer;
#[doc(hidden)]
Expand Down
27 changes: 20 additions & 7 deletions src/pycell/impl_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ use std::mem::ManuallyDrop;
use crate::impl_::pyclass::{
PyClassBaseType, PyClassDict, PyClassImpl, PyClassThreadChecker, PyClassWeakRef,
};
use crate::type_object::{get_tp_free, PyLayout, PySizedLayout};
use crate::internal::get_slot::TP_FREE;
use crate::type_object::{PyLayout, PySizedLayout};
use crate::types::{PyType, PyTypeMethods};
use crate::{ffi, PyClass, PyTypeInfo, Python};

use super::{PyBorrowError, PyBorrowMutError};
Expand Down Expand Up @@ -221,26 +223,37 @@ where
Ok(())
}
unsafe fn tp_dealloc(py: Python<'_>, slf: *mut ffi::PyObject) {
let type_obj = T::type_object_raw(py);
// FIXME: there is potentially subtle issues here if the base is overwritten
// at runtime? To be investigated.
let type_obj = T::type_object_bound(py);
let type_ptr = type_obj.as_type_ptr();
let actual_type = PyType::from_borrowed_type_ptr(py, ffi::Py_TYPE(slf));

// For `#[pyclass]` types which inherit from PyAny, we can just call tp_free
if type_obj == std::ptr::addr_of_mut!(ffi::PyBaseObject_Type) {
return get_tp_free(ffi::Py_TYPE(slf))(slf.cast());
if type_ptr == std::ptr::addr_of_mut!(ffi::PyBaseObject_Type) {
let tp_free = actual_type
.get_slot(TP_FREE)
.expect("PyBaseObject_Type should have tp_free");
return tp_free(slf.cast());
}

// More complex native types (e.g. `extends=PyDict`) require calling the base's dealloc.
#[cfg(not(Py_LIMITED_API))]
{
if let Some(dealloc) = (*type_obj).tp_dealloc {
// FIXME: should this be using actual_type.tp_dealloc?
if let Some(dealloc) = (*type_ptr).tp_dealloc {
// Before CPython 3.11 BaseException_dealloc would use Py_GC_UNTRACK which
// assumes the exception is currently GC tracked, so we have to re-track
// before calling the dealloc so that it can safely call Py_GC_UNTRACK.
#[cfg(not(any(Py_3_11, PyPy)))]
if ffi::PyType_FastSubclass(type_obj, ffi::Py_TPFLAGS_BASE_EXC_SUBCLASS) == 1 {
if ffi::PyType_FastSubclass(type_ptr, ffi::Py_TPFLAGS_BASE_EXC_SUBCLASS) == 1 {
ffi::PyObject_GC_Track(slf.cast());
}
dealloc(slf);
} else {
get_tp_free(ffi::Py_TYPE(slf))(slf.cast());
(*actual_type.as_type_ptr())
.tp_free
.expect("type missing tp_free")(slf.cast());
}
}

Expand Down
17 changes: 13 additions & 4 deletions src/pyclass_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@
use crate::callback::IntoPyCallbackOutput;
use crate::ffi_ptr_ext::FfiPtrExt;
use crate::impl_::pyclass::{PyClassBaseType, PyClassDict, PyClassThreadChecker, PyClassWeakRef};
use crate::types::PyAnyMethods;
use crate::{ffi, Bound, Py, PyClass, PyErr, PyResult, Python};
use crate::internal::get_slot::TP_ALLOC;
use crate::types::{PyAnyMethods, PyType};
use crate::{ffi, Borrowed, Bound, Py, PyClass, PyErr, PyResult, Python};
use crate::{
ffi::PyTypeObject,
pycell::impl_::{PyClassBorrowChecker, PyClassMutability, PyClassObjectContents},
type_object::{get_tp_alloc, PyTypeInfo},
type_object::PyTypeInfo,
};
use std::{
cell::UnsafeCell,
Expand Down Expand Up @@ -50,8 +51,16 @@ impl<T: PyTypeInfo> PyObjectInit<T> for PyNativeTypeInitializer<T> {
) -> PyResult<*mut ffi::PyObject> {
// HACK (due to FIXME below): PyBaseObject_Type's tp_new isn't happy with NULL arguments
let is_base_object = type_object == std::ptr::addr_of_mut!(ffi::PyBaseObject_Type);
let subtype_borrowed: Borrowed<'_, '_, PyType> = subtype
.cast::<ffi::PyObject>()
.assume_borrowed_unchecked(py)
.downcast_unchecked();

if is_base_object {
let alloc = get_tp_alloc(subtype).unwrap_or(ffi::PyType_GenericAlloc);
let alloc = subtype_borrowed
.get_slot(TP_ALLOC)
.unwrap_or(ffi::PyType_GenericAlloc);

let obj = alloc(subtype, 0);
return if obj.is_null() {
Err(PyErr::fetch(py))
Expand Down
29 changes: 0 additions & 29 deletions src/type_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,32 +227,3 @@ where
T::is_type_of_bound(object)
}
}

#[inline]
pub(crate) unsafe fn get_tp_alloc(tp: *mut ffi::PyTypeObject) -> Option<ffi::allocfunc> {
#[cfg(not(Py_LIMITED_API))]
{
(*tp).tp_alloc
}

#[cfg(Py_LIMITED_API)]
{
let ptr = ffi::PyType_GetSlot(tp, ffi::Py_tp_alloc);
std::mem::transmute(ptr)
}
}

#[inline]
pub(crate) unsafe fn get_tp_free(tp: *mut ffi::PyTypeObject) -> ffi::freefunc {
#[cfg(not(Py_LIMITED_API))]
{
(*tp).tp_free.unwrap()
}

#[cfg(Py_LIMITED_API)]
{
let ptr = ffi::PyType_GetSlot(tp, ffi::Py_tp_free);
debug_assert_ne!(ptr, std::ptr::null_mut());
std::mem::transmute(ptr)
}
}
Loading

0 comments on commit 73b061b

Please sign in to comment.