0
\$\begingroup\$

I am basically a beginner, so I Wrote some code that implements a zero cost singleton, through it will crash your program if you have a MMU.

use core::cell::UnsafeCell;
use core::mem::{size_of, transmute, MaybeUninit};
/// This type is mean to Static allocation of variables that should life all the program's life
/// without modification more than once in the initialization.
/// **NOTE:** Very unsafe thing to do.
#[repr(transparent)]
pub struct StaticOnce<'a, T: Sized + Sync>(UnsafeCell<MaybeUninit<&'a T>>);

// This is safe because the user must only use the transmute function once in the entire program runlife.
unsafe impl<'a, T: Sized + Sync> Sync for StaticOnce<'a, T> {}

impl<'a, T: Sized + Sync> StaticOnce<'a, T> {
    pub const fn new() -> Self {
        const SIZE: usize = size_of::<usize>();
        let space = [0u8; SIZE];
        // This is safe bacause the transmute converts a array of the same length as the pointer
        // (given that pointers have length usize), to a pointer trougth is wrapped in a tranparent
        // MaybeUninit that is wrapped in a UnsafeCell to allow mutability for once in the
        // entire length of the program runlife.
        Self(unsafe { transmute::<[u8; SIZE], UnsafeCell<MaybeUninit<&T>>>(space) })
    }

    /// Geter of the contents of the singelton.
    /// **NOTE**: Never call before initialization or it may crash the program and will have undefined
    /// behaviour.
    #[inline(always)]
    pub fn get(&self) -> &T {
        // This is safe because the contents of the singleton were unitialized (if the user follow the
        // intructions), so the pointer container by the UnsafeCell will be vaid, and will be
        // right configured in a valid address.
        let maybe = &(unsafe { *UnsafeCell::get(&self.0) }) as &MaybeUninit<&T>;
        // This is true because the user following the intruction of initialized with transmute before
        // celling the getter makes the content of the MaybeUninit valid for when this is called.
        unsafe { maybe.assume_init() }
    }

    /// This is supposed toonly be called once in the program initialization.
    /// **NOTE**: Very unsafe thing to do.
    // This is safe because StaitcOnce::transmute is safe, and the user follow the instructions
    // of call this before anything and only once.
    pub unsafe fn transmute(&self, item: T) {
        StaticOnce::transmute_inner(self, item)
    }

    fn transmute_inner(self_: &Self, item: T) {
        // This is safe beacause (1) this is just make once in the entire program runlife, and (2) the
        // typecast reduce to the correct one.
        let ptr: &mut MaybeUninit<&T> = &mut unsafe { *UnsafeCell::get(&self_.0) };
        // This is safe because we get by compilation the space fo the pointer already allocated, just
        // is needed to initialize it.
        let ptr: &mut &T = &mut unsafe { *ptr.as_mut_ptr() };
        *ptr = &item;
    }
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Welcome to the site! Is there any particular aspect of this code you would like feedback on? \$\endgroup\$
    – Davislor
    May 27, 2022 at 3:57

1 Answer 1

2
\$\begingroup\$

Firstly, its clear that you haven't tried to test or use this code at all, because it doesn't work. Generally speaking here on Code Review we request that you make sure your code actually works before asking for feedback.

Secondly, Rust's power is precisely in catching the kind of errors you make here. But it fails because you have used unsafe code and Rust can't spot the error. Since you are a beginner, you probably shouldn't be messing with unsafe code yet. I heavily recommend sticking with safe rust code until you've mastered it before trying to work with unsafe code.

Thirdly, your code falls into a common genre of people new to Rust, implementing patterns used in other languages, in this case a singleton. In Rust we generally don't want or need those patterns. If you want to get familiar with Rust, write code that solves real problems not one that implements patterns that are useful in other languages.

use core::cell::UnsafeCell;
use core::mem::{size_of, transmute, MaybeUninit};
/// This type is mean to Static allocation of variables that should life all the program's life
/// without modification more than once in the initialization.
/// **NOTE:** Very unsafe thing to do.
#[repr(transparent)]
pub struct StaticOnce<'a, T: Sized + Sync>(UnsafeCell<MaybeUninit<&'a T>>);

This line is the first hint that something is very wrong. You have a lifetime for a variable that purports to live for the length of the program's life. The lifetime for variables that live for the length of the program is 'static.

// This is safe because the user must only use the transmute function once in the entire program runlife.
unsafe impl<'a, T: Sized + Sync> Sync for StaticOnce<'a, T> {}

This misses the way we use safe in Rust. In Rust we say things are safe when the api guarantees that it cannot be misused. But in your case, you do not make this guarantee. Your code is unsafe and it depends on the user to correctly use the api. I can't stop you from doing it that way, but that's not the recommend rust way, and you shouldn't claim that its safe when you do it this way.

impl<'a, T: Sized + Sync> StaticOnce<'a, T> {
    pub const fn new() -> Self {
        const SIZE: usize = size_of::<usize>();
        let space = [0u8; SIZE];
        // This is safe bacause the transmute converts a array of the same length as the pointer
        // (given that pointers have length usize), to a pointer trougth is wrapped in a tranparent
        // MaybeUninit that is wrapped in a UnsafeCell to allow mutability for once in the
        // entire length of the program runlife.
        Self(unsafe { transmute::<[u8; SIZE], UnsafeCell<MaybeUninit<&T>>>(space) })
    }

You are rather defeating the purpose of MaybeUninit by initializing everything to zeros. There is no need to be transmuting anything, just use the apis for constructing MaybeUninit and UnsafeCell.

    Self(UnsafeCell::new(MaybeUninit::uninit()))

or

    Self(UnsafeCell::new(MaybeUninit::zeroed()))




    /// Geter of the contents of the singelton.
    /// **NOTE**: Never call before initialization or it may crash the program and will have undefined
    /// behaviour.
    #[inline(always)]
    pub fn get(&self) -> &T {
        // This is safe because the contents of the singleton were unitialized (if the user follow the
        // intructions), so the pointer container by the UnsafeCell will be vaid, and will be
        // right configured in a valid address.
        let maybe = &(unsafe { *UnsafeCell::get(&self.0) }) as &MaybeUninit<&T>;
        // This is true because the user following the intruction of initialized with transmute before
        // celling the getter makes the content of the MaybeUninit valid for when this is called.
        unsafe { maybe.assume_init() }
    }

This function can be simply written as:

    unsafe {
        (*self.0.get()).assume_init()
    }



    /// This is supposed toonly be called once in the program initialization.
    /// **NOTE**: Very unsafe thing to do.
    // This is safe because StaitcOnce::transmute is safe, and the user follow the instructions
    // of call this before anything and only once.
    pub unsafe fn transmute(&self, item: T) {
        StaticOnce::transmute_inner(self, item)
    }

I'm at a loss why you decided to put the logic into another function. It's also worth nothing, this is safe. Its actuall get() which is unsafe, because you don't guarantee it was called after transmute. Its also a bizarre name, because its purpose isn't really to transmute anything, but to set the singleton.

    fn transmute_inner(self_: &Self, item: T) {

I'm not sure why you are using this wierd self_: &Self instead of &self.

        // This is safe beacause (1) this is just make once in the entire program runlife, and (2) the
        // typecast reduce to the correct one.
        let ptr: &mut MaybeUninit<&T> = &mut unsafe { *UnsafeCell::get(&self_.0) };
        // This is safe because we get by compilation the space fo the pointer already allocated, just
        // is needed to initialize it.
        let ptr: &mut &T = &mut unsafe { *ptr.as_mut_ptr() };

This is not safe. The documentation for as_mut_ptr states:

Reading from this pointer or turning it into a reference is undefined behavior unless the MaybeUninit is initialized.

In your case, it has not been initialized and this is undefined behavior. Instead, you should use the write method of MaybeUninit to assign to it.

        *ptr = &item;

item is a parameter to this function, and its lifetime will end when the function terminates. Here you take a reference to it and store that. But that reference won't be valid once the function finishes. Rust would normally catch this sort of thing, but doesn't because you casting from raw pointers.

    }
}

Finally, your code doesn't really do much above and beyond Rust's builtin global variables. Consider the following code:

use core::mem::{MaybeUninit};

static mut BOB: MaybeUninit<String> = MaybeUninit::uninit();

fn main() {
   unsafe { BOB = MaybeUninit::new(String::new()); }
   
   unsafe { dbg!(BOB.assume_init_ref()); }
}

The only thing your code contributes seems to be pretending to be safe when it's not.

\$\endgroup\$
2
  • \$\begingroup\$ (1) I actually tried to use this in the x86_64 Rust minimal OS that I am working on, but as I said before the MMU was no happy with it. (2) Actually this does in fact have some "real world" use as a zero cost singleton. \$\endgroup\$
    – Delfin
    May 27, 2022 at 22:32
  • \$\begingroup\$ @Delfin, well, ok, apparently you have tried to run it, as it stands its so broken that I assumed you didn't. I question your conclusion that the MMU is unhappy with it. I think the code is just broken. \$\endgroup\$ May 28, 2022 at 17:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.