estebank / makeit

Zero-overhead type-safe builder pattern `derive` macro for your Rust structs
223 stars 11 forks source link

The way ptr_XX and inner_set_XX are implemented is UB #10

Open Voultapher opened 2 years ago

Voultapher commented 2 years ago

For example

pub unsafe fn ptr_0(&mut self) -> *mut String {
    let inner = self.inner.as_mut_ptr();
    &raw mut (*inner).a
}

A reference in Rust gurantees that the memory is initialized. You need to use addr_of_mut to correctly model this.


Somewhat unrleated but I thought I'd mention it.

let ptr =
    &self as *const OrangeBuilder<AUnset, FieldB> as
        *const OrangeBuilder<ASet, FieldB>;
::core::mem::forget(self);
unsafe { ptr.read() }

That looks a lot like transmute. Is there a specific reason not to use transmute here? Also this is UB too, the docs for mem::forget

Any resources the value manages, such as heap memory or a file handle, will linger forever in an unreachable state. However, it does not guarantee that pointers to this memory will remain valid.

cyphersnake commented 2 years ago

About transmute:

// We do the following instead of `::core::mem::transmute(self)` here
// because we can't `transmute` on fields that involve generics.

But I have not yet understood why not just recreate the structure, passing the same fields, but changing the generic parameters. Does it interfere with zero-cost?

mo8it commented 1 year ago

@estebank I am not sure if the comment about transmute is still relevant since this works for me now:

image

Maybe this made it possible?

Voultapher commented 1 year ago

Just a small suggestion, when using transmute I advise to always fully specify the from and to types e.g. transmute::<&[T], &[i32]>(v)`.

Looking back at this comment:

// We do the following instead of ::core::mem::transmute(self) here // because we can't transmute on fields that involve generics.

I'm not sure that's correct. You can only transmute between things that have the same size, same applies to casting pointers which have a fixed size known at compile time. Maybe you tried to transmute the objects directly and not references to them?

This code works in rustc version 1.0 https://godbolt.org/z/Yj9jqhEEf:

use std::mem;

pub fn x<A, B>(a: &A) {
    unsafe {
        let _b = mem::transmute::<&A, &B>(a);
    }
}

pub struct OrangeBuilder<A, B> {
    a: A,
    b: B,
}

pub fn y<AUnset, ASet, FieldB>(orange_builder: &OrangeBuilder<AUnset, FieldB>) {
    unsafe {
        let _ = mem::transmute::<&OrangeBuilder<AUnset, FieldB>, &OrangeBuilder<ASet, FieldB>>(
            orange_builder,
        );
    }
}