Amanieu / intrusive-rs

Intrusive collections for Rust
Apache License 2.0
412 stars 48 forks source link

`clone_pointer` causes a stacked borrows violation in Miri. #83

Closed icmccorm closed 1 year ago

icmccorm commented 1 year ago

Every test case of the form clone_[type] and test_clone_[type] fails in Miri with the following error:

    --> .../src/rust/library/core/src/ptr/non_null.rs:376:18

376  |         unsafe { &*self.as_ptr() }
     |                  ^^^^^^^^^^^^^^^
     |                  |
     |                  trying to retag from <238235> for SharedReadWrite permission at alloc94157[0x0], but that tag does not exist in the borrow stack for this location
     |                  this error occurs as part of retag at alloc94157[0x0..0x28]

It seems like in each case, the creation of the raw pointer and the origin of the error are both within a call to clone_pointer.

    #[inline]
    pub fn clone_pointer(&self) -> Option<<A::PointerOps as PointerOps>::Pointer>
    where
        <A::PointerOps as PointerOps>::Pointer: Clone,
    {
        let raw_pointer = self.get()? as *const <A::PointerOps as PointerOps>::Value;
        Some(unsafe {
            crate::pointer_ops::clone_pointer_from_raw(self.list.adapter.pointer_ops(), raw_pointer)
        })
    }

The pointer is created with self.get()? as *const..., and then the call to clone_pointer_from_raw eventually leads to a call to as_ref(), which triggers the stacked borrows violation. I believe this is a false positive; somewhere along the line the pointer is read or modified in a way that causes Miri to lose its metadata. I think the following snippet is an accurate, minimal recreation of what's happening here; it triggers the same error in Miri:

fn main() {
    unsafe {
        let p = Rc::new(1);
        let raw = &*p as *const i32;
        let p2: Rc<i32> = Rc::from_raw(raw);
        assert_eq!(1, Rc::strong_count(&p2));
    }
}

This seems similar to this example with UnsafeCell (link) or this simpler example with raw pointers (link). I figured I'd report this to get confirmation that this truly is a false positive, though.

Amanieu commented 1 year ago

Thanks! It was indeed a bug in my code. It should be fixed now.