Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OwningRef::map is unsound #71

Open
steffahn opened this issue Jan 18, 2021 · 1 comment · May be fixed by #72
Open

OwningRef::map is unsound #71

steffahn opened this issue Jan 18, 2021 · 1 comment · May be fixed by #72

Comments

@steffahn
Copy link

fn main() {
    let x = OwningRef::new(Box::new(()));
    let z: OwningRef<Box<()>, str>;
    {
        let s = "Hello World!".to_string();
        let s_ref: &str = &s;
        let y: OwningRef<Box<()>, &str> = x.map(|_| &s_ref);
        z = y.map(|s: &&str| *s);
        // s deallocated here
    }
    println!("{}", &*z); // printing garbage, accessing `s` after it’s freed
}

Explanation

Since

pub fn map<F, U: ?Sized>(self, f: F) -> OwningRef<O, U>
where
    O: StableAddress,
    F: FnOnce(&T) -> &U, 

supports non-'static types for U and F, we can use U == &'short T to turn

OwningRef<Box<()>, ()>

into

OwningRef<Box<()>, &'short T>

In the example code above, T == str and the call to map is .map(|_| &s_ref).

Then, on the next call to map, you only need to provide an

FnOnce(&&'short T) -> &U

i.e.

for<'a> FnOnce(&'a &'short T) -> &'a U

Since &'a &'short T comes with an implicit 'short: 'a bound, you can easily turn the

OwningRef<Box<()>, &'short T>

into

OwningRef<Box<()>, T>

in effect liberating yourself from the 'short bound. The call to map in the example is .map(|s: &&str| *s).

Another example

I also managed to create a general transmute_lifetime function:

fn transmute_lifetime<'a, 'b, T>(r: &'a T) -> &'b T {
    let r = &r;
    let x = OwningRef::new(Box::new(())).map(|_| &r).map(|r| &***r);
    &*Box::leak(Box::new(x))
}

which works like this:

OwningRef<Box<()>, ()>
  ⇓    // by OwningRef::map
OwningRef<Box<()>, &'short &'a T>
  ⇓    // by OwningRef::map
OwningRef<Box<()>, T>
  ⇓    // by Box::new
Box<OwningRef<Box<()>, T>>
  ⇓    // by Box::leak
&'b OwningRef<Box<()>, T> 
  ⇓    // dereferencing
&'b T
@steffahn steffahn linked a pull request Jan 18, 2021 that will close this issue
@steffahn
Copy link
Author

A possible fix is to use

pub struct OwningRef<'t, O, T: ?Sized> {
    owner: O,
    reference: *const T,
    marker: PhantomData<&'t T>,
}

and

pub struct OwningRefMut<'t, O, T: ?Sized> {
    owner: O,
    reference: *mut T,
    marker: PhantomData<&'t T>,
}

see for example the draft PR #72.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant