pycell: fix calculation of dictoffset on 32-bit Windows

Author: davidhewitt

CHANGELOG.md

@@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Fix regression in 0.14.0 rejecting usage of `#[doc(hidden)]` on structs and functions annotated with PyO3 macros. [#1722](https://github.com/PyO3/pyo3/pull/1722)
 - Fix regression in 0.14.0 leading to incorrect code coverage being computed for `#[pyfunction]`s. [#1726](https://github.com/PyO3/pyo3/pull/1726)
 - Fix incorrect FFI definition of `Py_Buffer` on PyPy. [#1737](https://github.com/PyO3/pyo3/pull/1737)
+- Fix incorrect calculation of `dictoffset` on 32-bit Windows. [#1475](https://github.com/PyO3/pyo3/pull/1475)
 
 ## [0.14.1] - 2021-07-04
 

build.rs

@@ -192,11 +192,18 @@ fn configure_pyo3() -> Result<()> {
     )?;
     interpreter_config.emit_pyo3_cfgs();
 
+    let rustc_minor_version = rustc_minor_version().unwrap_or(0);
+
     // Enable use of const generics on Rust 1.51 and greater
-    if rustc_minor_version().unwrap_or(0) >= 51 {
+    if rustc_minor_version >= 51 {
         println!("cargo:rustc-cfg=min_const_generics");
     }
 
+    // Enable use of std::ptr::addr_of! on Rust 1.51 and greater
+    if rustc_minor_version >= 51 {
+        println!("cargo:rustc-cfg=addr_of");
+    }
+
     Ok(())
 }
 

src/pycell.rs

@@ -104,34 +104,84 @@ pub struct PyCell<T: PyClass> {
 pub(crate) struct PyCellContents<T: PyClass> {
     pub(crate) value: ManuallyDrop<UnsafeCell<T>>,
     pub(crate) thread_checker: T::ThreadChecker,
-    // DO NOT CHANGE THE ORDER OF THESE FIELDS WITHOUT CHANGING PyCell::dict_offset()
-    // AND PyCell::weakref_offset()
     pub(crate) dict: T::Dict,
     pub(crate) weakref: T::WeakRef,
 }
 
 impl<T: PyClass> PyCell<T> {
     /// Get the offset of the dictionary from the start of the struct in bytes.
     #[cfg(not(all(Py_LIMITED_API, not(Py_3_9))))]
-    pub(crate) fn dict_offset() -> Option<usize> {
+    pub(crate) fn dict_offset() -> Option<ffi::Py_ssize_t> {
+        use std::convert::TryInto;
         if T::Dict::IS_DUMMY {
             None
         } else {
-            Some(
-                std::mem::size_of::<Self>()
-                    - std::mem::size_of::<T::Dict>()
-                    - std::mem::size_of::<T::WeakRef>(),
-            )
+            #[cfg(addr_of)]
+            let offset = {
+                // With std::ptr::addr_of - can measure offset using uninit memory without UB.
+                let cell = std::mem::MaybeUninit::<Self>::uninit();
+                let base_ptr = cell.as_ptr();
+                let dict_ptr = unsafe { std::ptr::addr_of!((*base_ptr).contents.dict) };
+                unsafe { (dict_ptr as *const u8).offset_from(base_ptr as *const u8) }
+            };
+            #[cfg(not(addr_of))]
+            let offset = {
+                // No std::ptr::addr_of - need to take references to PyCell to measure offsets;
+                // make a zero-initialised "fake" one so that referencing it is not UB.
+                let mut cell = std::mem::MaybeUninit::<Self>::uninit();
+                unsafe {
+                    std::ptr::write_bytes(cell.as_mut_ptr(), 0, 1);
+                }
+                let cell = unsafe { cell.assume_init() };
+                let dict_ptr = &cell.contents.dict;
+                // offset_from wasn't stabilised until 1.47, so we also have to work around
+                // that...
+                let offset = (dict_ptr as *const _ as usize) - (&cell as *const _ as usize);
+                // This isn't a valid cell, so ensure no Drop code runs etc.
+                std::mem::forget(cell);
+                offset
+            };
+            // Py_ssize_t may not be equal to isize on all platforms
+            #[allow(clippy::useless_conversion)]
+            Some(offset.try_into().expect("offset should fit in Py_ssize_t"))
         }
     }
 
     /// Get the offset of the weakref list from the start of the struct in bytes.
     #[cfg(not(all(Py_LIMITED_API, not(Py_3_9))))]
-    pub(crate) fn weakref_offset() -> Option<usize> {
+    pub(crate) fn weakref_offset() -> Option<ffi::Py_ssize_t> {
+        use std::convert::TryInto;
         if T::WeakRef::IS_DUMMY {
             None
         } else {
-            Some(std::mem::size_of::<Self>() - std::mem::size_of::<T::WeakRef>())
+            #[cfg(addr_of)]
+            let offset = {
+                // With std::ptr::addr_of - can measure offset using uninit memory without UB.
+                let cell = std::mem::MaybeUninit::<Self>::uninit();
+                let base_ptr = cell.as_ptr();
+                let weaklist_ptr = unsafe { std::ptr::addr_of!((*base_ptr).contents.weakref) };
+                unsafe { (weaklist_ptr as *const u8).offset_from(base_ptr as *const u8) }
+            };
+            #[cfg(not(addr_of))]
+            let offset = {
+                // No std::ptr::addr_of - need to take references to PyCell to measure offsets;
+                // make a zero-initialised "fake" one so that referencing it is not UB.
+                let mut cell = std::mem::MaybeUninit::<Self>::uninit();
+                unsafe {
+                    std::ptr::write_bytes(cell.as_mut_ptr(), 0, 1);
+                }
+                let cell = unsafe { cell.assume_init() };
+                let weaklist_ptr = &cell.contents.weakref;
+                // offset_from wasn't stabilised until 1.47, so we also have to work around
+                // that...
+                let offset = (weaklist_ptr as *const _ as usize) - (&cell as *const _ as usize);
+                // This isn't a valid cell, so ensure no Drop code runs etc.
+                std::mem::forget(cell);
+                offset
+            };
+            // Py_ssize_t may not be equal to isize on all platforms
+            #[allow(clippy::useless_conversion)]
+            Some(offset.try_into().expect("offset should fit in Py_ssize_t"))
         }
     }
 }

src/pyclass.rs

@@ -174,13 +174,13 @@ fn tp_init_additional<T: PyClass>(type_object: *mut ffi::PyTypeObject) {
         // __dict__ support
         if let Some(dict_offset) = PyCell::<T>::dict_offset() {
             unsafe {
-                (*type_object).tp_dictoffset = dict_offset as ffi::Py_ssize_t;
+                (*type_object).tp_dictoffset = dict_offset;
             }
         }
         // weakref support
         if let Some(weakref_offset) = PyCell::<T>::weakref_offset() {
             unsafe {
-                (*type_object).tp_weaklistoffset = weakref_offset as ffi::Py_ssize_t;
+                (*type_object).tp_weaklistoffset = weakref_offset;
             }
         }
     }
@@ -233,11 +233,11 @@ fn py_class_method_defs(
 #[cfg(Py_3_9)]
 fn py_class_members<T: PyClass>() -> Vec<ffi::structmember::PyMemberDef> {
     #[inline(always)]
-    fn offset_def(name: &'static str, offset: usize) -> ffi::structmember::PyMemberDef {
+    fn offset_def(name: &'static str, offset: ffi::Py_ssize_t) -> ffi::structmember::PyMemberDef {
         ffi::structmember::PyMemberDef {
             name: name.as_ptr() as _,
             type_code: ffi::structmember::T_PYSSIZET,
-            offset: offset as _,
+            offset,
             flags: ffi::structmember::READONLY,
             doc: std::ptr::null_mut(),
         }