avoid calling PyType_GetSlot on static types before Python 3.10 (#4599

)

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

* use the correct tp_free

* fixup

newsfragments/4599.fixed.md

@@ -0,0 +1 @@
+Fix crash calling `PyType_GetSlot` on static types before Python 3.10.

src/internal.rs

@@ -0,0 +1,3 @@
+//! Holding place for code which is not intended to be reachable from outside of PyO3.
+
+pub(crate) mod get_slot;

src/internal/get_slot.rs

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

src/lib.rs

@@ -437,6 +437,7 @@ mod tests;
 
 #[macro_use]
 mod internal_tricks;
+mod internal;
 
 pub mod buffer;
 #[doc(hidden)]

src/pycell/impl_.rs

@@ -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};
@@ -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());
             }
         }
 

src/pyclass_init.rs

@@ -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,
@@ -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))

src/type_object.rs

@@ -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)
-    }
-}