Skip to content

Commit 8e3dc45

Browse files
davidhewittdavidhewitt
committed
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
1 parent 969300d commit 8e3dc45

File tree

8 files changed

+279
-59
lines changed

8 files changed

+279
-59
lines changed

‎newsfragments/4599.fixed.md

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix crash calling `PyType_GetSlot` on static types before Python 3.10.

‎src/internal.rs

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

‎src/internal/get_slot.rs

+234
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,234 @@
1+
use crate::{
2+
ffi,
3+
types::{PyType, PyTypeMethods},
4+
Borrowed, Bound,
5+
};
6+
use std::os::raw::c_int;
7+
8+
impl Bound<'_, PyType> {
9+
#[inline]
10+
pub(crate) fn get_slot<const S: c_int>(&self, slot: Slot<S>) -> <Slot<S> as GetSlotImpl>::Type
11+
where
12+
Slot<S>: GetSlotImpl,
13+
{
14+
slot.get_slot(self.as_borrowed())
15+
}
16+
}
17+
18+
impl Borrowed<'_, '_, PyType> {
19+
#[inline]
20+
pub(crate) fn get_slot<const S: c_int>(self, slot: Slot<S>) -> <Slot<S> as GetSlotImpl>::Type
21+
where
22+
Slot<S>: GetSlotImpl,
23+
{
24+
slot.get_slot(self)
25+
}
26+
}
27+
28+
pub(crate) trait GetSlotImpl {
29+
type Type;
30+
fn get_slot(self, tp: Borrowed<'_, '_, PyType>) -> Self::Type;
31+
}
32+
33+
#[derive(Copy, Clone)]
34+
pub(crate) struct Slot<const S: c_int>;
35+
36+
macro_rules! impl_slots {
37+
($($name:ident: ($slot:ident, $field:ident) -> $tp:ty),+ $(,)?) => {
38+
$(
39+
pub (crate) const $name: Slot<{ ffi::$slot }> = Slot;
40+
41+
impl GetSlotImpl for Slot<{ ffi::$slot }> {
42+
type Type = $tp;
43+
44+
#[inline]
45+
fn get_slot(self, tp: Borrowed<'_, '_, PyType>) -> Self::Type {
46+
let ptr = tp.as_type_ptr();
47+
48+
#[cfg(not(Py_LIMITED_API))]
49+
unsafe {
50+
(*ptr).$field
51+
}
52+
53+
#[cfg(Py_LIMITED_API)]
54+
{
55+
#[cfg(not(Py_3_10))]
56+
{
57+
// Calling PyType_GetSlot on static types is not valid before Python 3.10
58+
// ... so the workaround is to first do a runtime check for these versions
59+
// (3.7, 3.8, 3.9) and then look in the type object anyway. This is only ok
60+
// because we know that the interpreter is not going to change the size
61+
// of the type objects for these historical versions.
62+
if !is_runtime_3_10(tp.py())
63+
&& unsafe { ffi::PyType_HasFeature(ptr, ffi::Py_TPFLAGS_HEAPTYPE) } == 0
64+
{
65+
return unsafe { (*ptr.cast::<PyTypeObject39Snapshot>()).$field };
66+
}
67+
}
68+
69+
// SAFETY: slot type is set carefully to be valid
70+
unsafe { std::mem::transmute(ffi::PyType_GetSlot(ptr, ffi::$slot)) }
71+
}
72+
}
73+
}
74+
)*
75+
};
76+
}
77+
78+
// Slots are implemented on-demand as needed.
79+
impl_slots! {
80+
TP_ALLOC: (Py_tp_alloc, tp_alloc) -> Option<ffi::allocfunc>,
81+
TP_DESCR_GET: (Py_tp_descr_get, tp_descr_get) -> Option<ffi::descrgetfunc>,
82+
TP_FREE: (Py_tp_free, tp_free) -> Option<ffi::freefunc>,
83+
}
84+
85+
#[cfg(all(Py_LIMITED_API, not(Py_3_10)))]
86+
fn is_runtime_3_10(py: crate::Python<'_>) -> bool {
87+
use crate::sync::GILOnceCell;
88+
89+
static IS_RUNTIME_3_10: GILOnceCell<bool> = GILOnceCell::new();
90+
*IS_RUNTIME_3_10.get_or_init(py, || py.version_info() >= (3, 10))
91+
}
92+
93+
#[repr(C)]
94+
#[cfg(all(Py_LIMITED_API, not(Py_3_10)))]
95+
pub struct PyNumberMethods39Snapshot {
96+
pub nb_add: Option<ffi::binaryfunc>,
97+
pub nb_subtract: Option<ffi::binaryfunc>,
98+
pub nb_multiply: Option<ffi::binaryfunc>,
99+
pub nb_remainder: Option<ffi::binaryfunc>,
100+
pub nb_divmod: Option<ffi::binaryfunc>,
101+
pub nb_power: Option<ffi::ternaryfunc>,
102+
pub nb_negative: Option<ffi::unaryfunc>,
103+
pub nb_positive: Option<ffi::unaryfunc>,
104+
pub nb_absolute: Option<ffi::unaryfunc>,
105+
pub nb_bool: Option<ffi::inquiry>,
106+
pub nb_invert: Option<ffi::unaryfunc>,
107+
pub nb_lshift: Option<ffi::binaryfunc>,
108+
pub nb_rshift: Option<ffi::binaryfunc>,
109+
pub nb_and: Option<ffi::binaryfunc>,
110+
pub nb_xor: Option<ffi::binaryfunc>,
111+
pub nb_or: Option<ffi::binaryfunc>,
112+
pub nb_int: Option<ffi::unaryfunc>,
113+
pub nb_reserved: *mut std::os::raw::c_void,
114+
pub nb_float: Option<ffi::unaryfunc>,
115+
pub nb_inplace_add: Option<ffi::binaryfunc>,
116+
pub nb_inplace_subtract: Option<ffi::binaryfunc>,
117+
pub nb_inplace_multiply: Option<ffi::binaryfunc>,
118+
pub nb_inplace_remainder: Option<ffi::binaryfunc>,
119+
pub nb_inplace_power: Option<ffi::ternaryfunc>,
120+
pub nb_inplace_lshift: Option<ffi::binaryfunc>,
121+
pub nb_inplace_rshift: Option<ffi::binaryfunc>,
122+
pub nb_inplace_and: Option<ffi::binaryfunc>,
123+
pub nb_inplace_xor: Option<ffi::binaryfunc>,
124+
pub nb_inplace_or: Option<ffi::binaryfunc>,
125+
pub nb_floor_divide: Option<ffi::binaryfunc>,
126+
pub nb_true_divide: Option<ffi::binaryfunc>,
127+
pub nb_inplace_floor_divide: Option<ffi::binaryfunc>,
128+
pub nb_inplace_true_divide: Option<ffi::binaryfunc>,
129+
pub nb_index: Option<ffi::unaryfunc>,
130+
pub nb_matrix_multiply: Option<ffi::binaryfunc>,
131+
pub nb_inplace_matrix_multiply: Option<ffi::binaryfunc>,
132+
}
133+
134+
#[repr(C)]
135+
#[cfg(all(Py_LIMITED_API, not(Py_3_10)))]
136+
pub struct PySequenceMethods39Snapshot {
137+
pub sq_length: Option<ffi::lenfunc>,
138+
pub sq_concat: Option<ffi::binaryfunc>,
139+
pub sq_repeat: Option<ffi::ssizeargfunc>,
140+
pub sq_item: Option<ffi::ssizeargfunc>,
141+
pub was_sq_slice: *mut std::os::raw::c_void,
142+
pub sq_ass_item: Option<ffi::ssizeobjargproc>,
143+
pub was_sq_ass_slice: *mut std::os::raw::c_void,
144+
pub sq_contains: Option<ffi::objobjproc>,
145+
pub sq_inplace_concat: Option<ffi::binaryfunc>,
146+
pub sq_inplace_repeat: Option<ffi::ssizeargfunc>,
147+
}
148+
149+
#[repr(C)]
150+
#[cfg(all(Py_LIMITED_API, not(Py_3_10)))]
151+
pub struct PyMappingMethods39Snapshot {
152+
pub mp_length: Option<ffi::lenfunc>,
153+
pub mp_subscript: Option<ffi::binaryfunc>,
154+
pub mp_ass_subscript: Option<ffi::objobjargproc>,
155+
}
156+
157+
#[repr(C)]
158+
#[cfg(all(Py_LIMITED_API, not(Py_3_10)))]
159+
pub struct PyAsyncMethods39Snapshot {
160+
pub am_await: Option<ffi::unaryfunc>,
161+
pub am_aiter: Option<ffi::unaryfunc>,
162+
pub am_anext: Option<ffi::unaryfunc>,
163+
}
164+
165+
#[repr(C)]
166+
#[cfg(all(Py_LIMITED_API, not(Py_3_10)))]
167+
pub struct PyBufferProcs39Snapshot {
168+
// not available in limited api, but structure needs to have the right size
169+
pub bf_getbuffer: *mut std::os::raw::c_void,
170+
pub bf_releasebuffer: *mut std::os::raw::c_void,
171+
}
172+
173+
/// Snapshot of the structure of PyTypeObject for Python 3.7 through 3.9.
174+
///
175+
/// This is used as a fallback for static types in abi3 when the Python version is less than 3.10;
176+
/// this is a bit of a hack but there's no better option and the structure of the type object is
177+
/// not going to change for those historical versions.
178+
#[repr(C)]
179+
#[cfg(all(Py_LIMITED_API, not(Py_3_10)))]
180+
struct PyTypeObject39Snapshot {
181+
pub ob_base: ffi::PyVarObject,
182+
pub tp_name: *const std::os::raw::c_char,
183+
pub tp_basicsize: ffi::Py_ssize_t,
184+
pub tp_itemsize: ffi::Py_ssize_t,
185+
pub tp_dealloc: Option<ffi::destructor>,
186+
#[cfg(not(Py_3_8))]
187+
pub tp_print: *mut std::os::raw::c_void, // stubbed out, not available in limited API
188+
#[cfg(Py_3_8)]
189+
pub tp_vectorcall_offset: ffi::Py_ssize_t,
190+
pub tp_getattr: Option<ffi::getattrfunc>,
191+
pub tp_setattr: Option<ffi::setattrfunc>,
192+
pub tp_as_async: *mut PyAsyncMethods39Snapshot,
193+
pub tp_repr: Option<ffi::reprfunc>,
194+
pub tp_as_number: *mut PyNumberMethods39Snapshot,
195+
pub tp_as_sequence: *mut PySequenceMethods39Snapshot,
196+
pub tp_as_mapping: *mut PyMappingMethods39Snapshot,
197+
pub tp_hash: Option<ffi::hashfunc>,
198+
pub tp_call: Option<ffi::ternaryfunc>,
199+
pub tp_str: Option<ffi::reprfunc>,
200+
pub tp_getattro: Option<ffi::getattrofunc>,
201+
pub tp_setattro: Option<ffi::setattrofunc>,
202+
pub tp_as_buffer: *mut PyBufferProcs39Snapshot,
203+
pub tp_flags: std::os::raw::c_ulong,
204+
pub tp_doc: *const std::os::raw::c_char,
205+
pub tp_traverse: Option<ffi::traverseproc>,
206+
pub tp_clear: Option<ffi::inquiry>,
207+
pub tp_richcompare: Option<ffi::richcmpfunc>,
208+
pub tp_weaklistoffset: ffi::Py_ssize_t,
209+
pub tp_iter: Option<ffi::getiterfunc>,
210+
pub tp_iternext: Option<ffi::iternextfunc>,
211+
pub tp_methods: *mut ffi::PyMethodDef,
212+
pub tp_members: *mut ffi::PyMemberDef,
213+
pub tp_getset: *mut ffi::PyGetSetDef,
214+
pub tp_base: *mut ffi::PyTypeObject,
215+
pub tp_dict: *mut ffi::PyObject,
216+
pub tp_descr_get: Option<ffi::descrgetfunc>,
217+
pub tp_descr_set: Option<ffi::descrsetfunc>,
218+
pub tp_dictoffset: ffi::Py_ssize_t,
219+
pub tp_init: Option<ffi::initproc>,
220+
pub tp_alloc: Option<ffi::allocfunc>,
221+
pub tp_new: Option<ffi::newfunc>,
222+
pub tp_free: Option<ffi::freefunc>,
223+
pub tp_is_gc: Option<ffi::inquiry>,
224+
pub tp_bases: *mut ffi::PyObject,
225+
pub tp_mro: *mut ffi::PyObject,
226+
pub tp_cache: *mut ffi::PyObject,
227+
pub tp_subclasses: *mut ffi::PyObject,
228+
pub tp_weaklist: *mut ffi::PyObject,
229+
pub tp_del: Option<ffi::destructor>,
230+
pub tp_version_tag: std::os::raw::c_uint,
231+
pub tp_finalize: Option<ffi::destructor>,
232+
#[cfg(Py_3_8)]
233+
pub tp_vectorcall: Option<ffi::vectorcallfunc>,
234+
}

‎src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,7 @@ mod tests;
437437

438438
#[macro_use]
439439
mod internal_tricks;
440+
mod internal;
440441

441442
pub mod buffer;
442443
#[doc(hidden)]

‎src/pycell/impl_.rs

+20-7
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ use std::mem::ManuallyDrop;
88
use crate::impl_::pyclass::{
99
PyClassBaseType, PyClassDict, PyClassImpl, PyClassThreadChecker, PyClassWeakRef,
1010
};
11-
use crate::type_object::{get_tp_free, PyLayout, PySizedLayout};
11+
use crate::internal::get_slot::TP_FREE;
12+
use crate::type_object::{PyLayout, PySizedLayout};
13+
use crate::types::{PyType, PyTypeMethods};
1214
use crate::{ffi, PyClass, PyTypeInfo, Python};
1315

1416
use super::{PyBorrowError, PyBorrowMutError};
@@ -221,26 +223,37 @@ where
221223
Ok(())
222224
}
223225
unsafe fn tp_dealloc(py: Python<'_>, slf: *mut ffi::PyObject) {
224-
let type_obj = T::type_object_raw(py);
226+
// FIXME: there is potentially subtle issues here if the base is overwritten
227+
// at runtime? To be investigated.
228+
let type_obj = T::type_object_bound(py);
229+
let type_ptr = type_obj.as_type_ptr();
230+
let actual_type = PyType::from_borrowed_type_ptr(py, ffi::Py_TYPE(slf));
231+
225232
// For `#[pyclass]` types which inherit from PyAny, we can just call tp_free
226-
if type_obj == std::ptr::addr_of_mut!(ffi::PyBaseObject_Type) {
227-
return get_tp_free(ffi::Py_TYPE(slf))(slf.cast());
233+
if type_ptr == std::ptr::addr_of_mut!(ffi::PyBaseObject_Type) {
234+
let tp_free = actual_type
235+
.get_slot(TP_FREE)
236+
.expect("PyBaseObject_Type should have tp_free");
237+
return tp_free(slf.cast());
228238
}
229239

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

‎src/pyclass_init.rs

+13-4
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@
22
use crate::callback::IntoPyCallbackOutput;
33
use crate::ffi_ptr_ext::FfiPtrExt;
44
use crate::impl_::pyclass::{PyClassBaseType, PyClassDict, PyClassThreadChecker, PyClassWeakRef};
5-
use crate::types::PyAnyMethods;
6-
use crate::{ffi, Bound, Py, PyClass, PyErr, PyResult, Python};
5+
use crate::internal::get_slot::TP_ALLOC;
6+
use crate::types::{PyAnyMethods, PyType};
7+
use crate::{ffi, Borrowed, Bound, Py, PyClass, PyErr, PyResult, Python};
78
use crate::{
89
ffi::PyTypeObject,
910
pycell::impl_::{PyClassBorrowChecker, PyClassMutability, PyClassObjectContents},
10-
type_object::{get_tp_alloc, PyTypeInfo},
11+
type_object::PyTypeInfo,
1112
};
1213
use std::{
1314
cell::UnsafeCell,
@@ -50,8 +51,16 @@ impl<T: PyTypeInfo> PyObjectInit<T> for PyNativeTypeInitializer<T> {
5051
) -> PyResult<*mut ffi::PyObject> {
5152
// HACK (due to FIXME below): PyBaseObject_Type's tp_new isn't happy with NULL arguments
5253
let is_base_object = type_object == std::ptr::addr_of_mut!(ffi::PyBaseObject_Type);
54+
let subtype_borrowed: Borrowed<'_, '_, PyType> = subtype
55+
.cast::<ffi::PyObject>()
56+
.assume_borrowed_unchecked(py)
57+
.downcast_unchecked();
58+
5359
if is_base_object {
54-
let alloc = get_tp_alloc(subtype).unwrap_or(ffi::PyType_GenericAlloc);
60+
let alloc = subtype_borrowed
61+
.get_slot(TP_ALLOC)
62+
.unwrap_or(ffi::PyType_GenericAlloc);
63+
5564
let obj = alloc(subtype, 0);
5665
return if obj.is_null() {
5766
Err(PyErr::fetch(py))

‎src/type_object.rs

-29
Original file line numberDiff line numberDiff line change
@@ -227,32 +227,3 @@ where
227227
T::is_type_of_bound(object)
228228
}
229229
}
230-
231-
#[inline]
232-
pub(crate) unsafe fn get_tp_alloc(tp: *mut ffi::PyTypeObject) -> Option<ffi::allocfunc> {
233-
#[cfg(not(Py_LIMITED_API))]
234-
{
235-
(*tp).tp_alloc
236-
}
237-
238-
#[cfg(Py_LIMITED_API)]
239-
{
240-
let ptr = ffi::PyType_GetSlot(tp, ffi::Py_tp_alloc);
241-
std::mem::transmute(ptr)
242-
}
243-
}
244-
245-
#[inline]
246-
pub(crate) unsafe fn get_tp_free(tp: *mut ffi::PyTypeObject) -> ffi::freefunc {
247-
#[cfg(not(Py_LIMITED_API))]
248-
{
249-
(*tp).tp_free.unwrap()
250-
}
251-
252-
#[cfg(Py_LIMITED_API)]
253-
{
254-
let ptr = ffi::PyType_GetSlot(tp, ffi::Py_tp_free);
255-
debug_assert_ne!(ptr, std::ptr::null_mut());
256-
std::mem::transmute(ptr)
257-
}
258-
}

0 commit comments

Comments
 (0)