Next steps for the unsafe code guidelines

#21

So, I wrote contradictory things here. It’s been 1 week, but it’s not March 30th. Do we think we want to try and meet today?

I think people should go ahead and leave notes on their observations in the thread for discussion. In fact, maybe I should start a new thread (just so it has a more appropriate subject line)? I think I’ll do that.

0 Likes

#22

I realize now that (a) I cannot do March 30th, owing to the GNOME coding sprint, and (b) in general the time we elected (2pm EST on Thursdays) is not especially good for me (I have another meeting at that time). Still, I think that synchronous meetings are maybe still of value. We certainly don’t have much of a rhythm yet. Perhaps we could establish a more regular time (earlier on Thu seemed like it would generally work).

I could probably do a meeting today but I’d love to hear from others – has anyone been able to do the research etc? (If so, I encourage you to leave a comment on this thread I spawned.

0 Likes

#23

Responding to your “unsafe code in the wild” thread here so as not to contaminate it:

This looks like a case of missing abstractions and I’m sure it’s not the only one. In this case, a write-only UninitializedVec<T> abstraction would probably help. Something like:

#![feature(untagged_unions)]

use std::ops::{Index, IndexMut};
use std::ptr::drop_in_place;

pub struct UninitializedVec<T: Sized> {
    data: *mut T,
    len: usize,
    capacity: usize,
}

impl<T: Sized> UninitializedVec<T> {
    // All the normal methods except that they operate over `Uninit<T>` instead of `T`...
    // And:
    pub unsafe fn into_vec(self) -> Vec<T> {
        Vec::from_raw_parts(self.data, self.len, self.capacity)
    }
}

impl<T: Sized> Index<usize> for UninitializedVec<T> {
    type Output = Uninit<T>;
    fn index(&self, idx: usize) -> &Uninit<T> {
        unsafe {
            assert!(idx < self.len, "out of bounds");
            &*(self.data.offset(idx as isize) as *const Uninit<T>)
        }
    }
}

impl<T: Sized> IndexMut<usize> for UninitializedVec<T> {
    fn index_mut(&mut self, idx: usize) -> &mut Uninit<T> {
        unsafe {
            assert!(idx < self.len, "out of bounds");
            &mut *(self.data.offset(idx as isize) as *mut Uninit<T>)
        }
    }
}

// I assume the alignment/size will match T...
#[allow(unions_with_drop_fields)]
pub union Uninit<T> {
    inited: T,
}

impl<T> Uninit<T> {
    /// Set to `value`. This will forget the existing value, if any.
    pub fn set(&mut self, value: T) {
        unsafe {
            self.inited = value;
        }
    }
    /// Set to `value` dropping the existing one.
    pub unsafe fn replace(&mut self, value: T) {
        drop_in_place(&mut self.inited);
        self.inited = value;
    }
    pub unsafe fn get_unchecked(&self) -> &T {
        &self.inited
    }
    pub unsafe fn get_mut_unchecked(&mut self) -> &mut T {
        &mut self.inited
    }
}
0 Likes

#24

Actually that’s pretty unclear to me. That is, the existing code…is fairly clean, it’s not obvious to me that it would add much to have an “uninitialized vector”. Put another way, that’s kind of what we wrote. =)

0 Likes

#25

Unless I misunderstood your post, you’re creating references to uninitialized types which may be UB (depending on the final the rules) and is definitely a bit dangerous (you’re producing safe references to types with “safe” methods that are actually unsafe to use). Isn’t that why you initially started out by using *mut T pointers everywhere? The fact that you had to make this choice leads me to believe that there’s something missing so I’m trying to propose a less “unsafe” middle ground (“unsafe” code doesn’t have to be all or nothing).

After thinking about it, you don’t actually need a special vector to deal with this; a simple Uninit<T> wrapper and a Vec<Uninit<T>> should suffice. I’ve forked off a new topic as it doesn’t really seem to belong here:

0 Likes

#26

Indeed. My point was merely that a more direct fix might be for us to use &mut Uninit<T> (where union Uninit<T> { value: T, uninitialized: () }), but otherwise keep working with slices as we have been doing. I guess that’s what you’re saying in your thread though. =)

1 Like

#27

Sorry, meeting today will not work out for me. And I haven’t yet had the time to do my share of the survey, either… :confused:

0 Likes

#28

Yeah, let’s try to communciate over the thread for now. We can meet again in a few weeks – my schedule is a bit crazy for next two weeks, between the GNOME thing, some Mozilla obligations, and the ECOOP PC meeting.

0 Likes

split this topic #29

10 posts were split to a new topic: Overflow checks and unsafe code

0 Likes

closed #30

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.

0 Likes