Is this owning_ref-like API safe?

code review
1

I wrote a little module (playground) inspired by owning_ref for safely using raw pointers. The idea is that it's always safe, because the borrow checker won't let the dereferenced values escape the closure. Does this make sense?

mod r {
#[derive(Copy, Clone)]
pub struct OwnedRef<T>{
    val: *const T
}
impl<T> std::ops::Deref for OwnedRef<T> {
    type Target = T;
    fn deref<'a>(&'a self) -> &'a Self::Target {
        unsafe {
            &*self.val
        }
    }
}
impl<T> OwnedRef<T> {
    pub fn map<'a, U>(&'a self, f: fn(&'a T) -> &'a U) -> OwnedRef<U> {
        OwnedRef{ val: &*f(&**self) }
    }
}
impl<T> std::fmt::Debug for OwnedRef<T> where T: std::fmt::Debug {
// ...
}

pub struct WithRefs<O, T> {
    owner: Box<O>,
    data: T,
}
impl<O> WithRefs<O, OwnedRef<O>> {
    pub fn new(owner: O) -> Self {
        let boxed = Box::into_raw(Box::new(owner));
        let reffed = OwnedRef::<O>{val: boxed};
        unsafe {
            WithRefs{ owner: Box::from_raw(boxed), data: reffed }
        }
    }
}
impl<O, T> WithRefs<O, T> {
    pub fn map<F, U>(self, f: F) -> WithRefs<O, U> 
        where F: for<'a> FnOnce(&'a T) -> U {
        let data1 = f(&self.data);
        WithRefs{ owner: self.owner, data: data1 }
    }
    pub fn with_mut<F>(&mut self, f: F)
        where F: for<'a> FnOnce(&'a mut T) {
        f(&mut self.data);
    }
    pub fn with<F>(&self, f: F)
        where F: for<'a> FnOnce(&'a T) {
        f(&self.data);
    }
}
}

// Example
struct Foo{f: f32, b: u32}
pub fn main() {
    let a = r::WithRefs::new(5);
    let mut b = a.map(|r| vec![*r]);
    b.with_mut(|v| v.push(v[0]));
    b.with(|v| println!("{:?}", v));
    
    let c = r::WithRefs::new(Foo{ f: 3.0, b: 6 });
    let d = c.map(|r| (r.map(|foo| &foo.f), r.map(|foo| &foo.b)));
    d.with(|v| println!("{:?}", v));
}
2

Box implies unique access, so the existence of a Box and a reference to the contents of said box, which was not derived from the Box directly is UB.

3

Oh, okay. So if I change WithRefs::new to

impl<O> WithRefs<O, OwnedRef<O>> {
    pub fn new(owner: O) -> Self {
        let boxed = Box::new(owner);
        let reffed = OwnedRef{val: &*boxed};
        WithRefs{ owner: boxed, data: reffed }
    }
}

then it's ok? It's a bit surprising to me that that this follows the aliasing rules, but it is what owning ref does. Is it ok because any time you examine the referent, you have to borrow or move out of self and thus the box?

4

That's still unsound. The WithRefs type should not be containing a Box at all - it needs to contain a raw pointer or an AliasableBox.

1 Like
5

Thanks! One thing I'm still a little confused about though: What keeps owning_ref from being unsound in the same way?

6

It is unsound in the same way

https://github.com/Kimundi/owning-ref-rs/issues/49

Furthermore, owning_ref::map is also unsound in other ways

https://github.com/Kimundi/owning-ref-rs/issues/71

I'm on mobile right now, so I'm can't really test it, but your API might have the same flaw. (Note that there's even more open soundness issues on the owning_ref repository.)

Judging by the amount of soundness issues that I've seen reported on and even found myself in crates offering self-referential struct functionality, I can tell you: getting something like this right is notoriously hard, even for quite experienced Rust users.

5 Likes
7

Haha fair enough. Thanks!

8

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.