Implementation of PyLayout::get_ptr for native types is broken

[programmerjake]

It's returning the address of the local variable self, which then is destroyed when the function returns:

pyo3/src/pycell.rs

Lines 28 to 30 in fee755a

	unsafe fn get_ptr(&self) -> *mut T {
	(&self) as *const &Self as *const _ as *mut _
	}

Several of the other implementations appear to be broken in the same way.

Demo code:

pub struct S(u64);
pub struct PyAny;

impl S {
    pub unsafe fn get_ptr(&self) -> *mut PyAny {
        // body copied from implementations of PyLayout::get_ptr
        (&self) as *const &Self as *const _ as *mut _
    }
}

fn main() {
    let a = [S(0), S(0)];
    for i in &a {
        let i: &S = i; // i is of type `&S`
        // note objects' address is different
        dbg!(i as *const _);
        // totally broken: note same address printed for different objects
        // also, address computed is before the beginning of either object
        dbg!(unsafe { i.get_ptr() });
    }
}

(Playground)

Output:

   Compiling playground v0.0.1 (/playground)
    Finished release [optimized] target(s) in 0.45s
     Running `target/release/playground`
[src/main.rs:16] i as *const _ = 0x00007ffe69ee9cf0
[src/main.rs:19] unsafe { i.get_ptr() } = 0x00007ffe69ee9ce8
[src/main.rs:16] i as *const _ = 0x00007ffe69ee9cf8
[src/main.rs:19] unsafe { i.get_ptr() } = 0x00007ffe69ee9ce8

[programmerjake]

This can be regression tested for by seeing if get_ptr() returns an in-bounds pointer.

[davidhewitt]

Thanks for noticing and reporting so fast!

I'm a little concerned that the existing regression tests did not pick this up. From what you're showing above, any test that exercises this code reads the wrong memory, so I would hope that at least one test would have gone awry as a result..

[kngwyu]

The key point is PyCell is always on Python heap(and PyCellBase is always a part of PyCell).
Ah, sorry I understand the problem now.

[kngwyu]

The problem is &PyCell is simply a pointer but &PyAny is a pointer to a pointer, so when trying to make them have the same method, this kind of confusion happens.