Impossible to blackbox copy T -> unaligned [u8]?

21

Change the vector to Vec<MaybeUninit<u8>>. The copy in the resize is just another copy of bytes, and we just established that this isn't UB.

1 Like
23

This compiles. Is this what you had in mind ?


pub struct Buffer {
    data: Vec<MaybeUninit<u8>>,}

impl Buffer {
    pub fn new() -> Buffer {
        Buffer { data: vec![]}}

    pub fn push<T: Field>(&mut self, t: &T) {
        unsafe {
            let ptr = t as *const T as *const MaybeUninit<u8>;
            let bytes = std::slice::from_raw_parts(ptr, std::mem::size_of::<T>());
            println!("len: {:?}", bytes.len());
            self.data.extend(bytes);}}

    pub fn pop<T: Field>(&mut self) -> T {
        assert!(self.data.len() >= std::mem::size_of::<T>());
        let offset = self.data.len() - std::mem::size_of::<T>();
        let bytes = self.data.split_off(offset);

        let mut t = T::default();
        unsafe {
            let ptr = &mut t as *mut T as *mut MaybeUninit<u8>;
            let out_bytes = std::slice::from_raw_parts_mut(ptr, std::mem::size_of::<T>());
            out_bytes.copy_from_slice(bytes.as_ref());}
        t}}
24

Yes, that's what I meant, however I still think the pop method is incredibly sketchy.

1 Like
25

Combining @Michael-F-Bryan 's suggestion for read_unaligned, we have:


pub struct Buffer {
    data: Vec<MaybeUninit<u8>>,}

impl Buffer {
    pub fn new() -> Buffer {
        Buffer { data: vec![]}}

    pub fn push<T: Field>(&mut self, t: &T) {
        unsafe {
            let ptr = t as *const T as *const MaybeUninit<u8>;
            let bytes = std::slice::from_raw_parts(ptr, std::mem::size_of::<T>());
            self.data.extend(bytes);}}

    pub fn pop<T: Field>(&mut self) -> T {
        assert!(self.data.len() >= std::mem::size_of::<T>());
        let offset = self.data.len() - std::mem::size_of::<T>();
        let bytes = self.data.split_off(offset);

        unsafe {
            let ptr = bytes.as_ptr() as *const T;
            std::ptr::read_unaligned(ptr)}}}

I'm actually happy with this. It seems the two sources are (1) alignment, which we have handled, and (2) possibly uninitialized u8's, which we handle via MaybeUnint<u8>

26

To elaborate on pop being sketchy, the reason is that if you push with one type, then pop with some other type, then any padding bytes in the first type may end up as non-padding in the other type. This would be UB.

1 Like
27

Good suggestion! Switching to Vec<MaybeUninit<u8>> for the backing bytes would make my previous implementation sound and side-step the issue of observing uninitialized padding bytes.

You still need to make sure pop() is an unsafe function because it's possible to trigger UB by popping in the wrong order or if T has lifetimes, but that's easy enough to document.

I'm not sure what should happen if you tried to push a dynamically-sized object like dyn Display, though. It's possible to determine the underlying type's size (std::mem::size_of_val()), but on the pop() side you'd need to know the concrete type. My gut feel would just be to add a Sized requirement to T.

1 Like
28

Pushing a trait object would be equivalent to pushing the underlying type behind the trait object.

2 Likes
29

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.