Search code examples
optimizationrustcompiler-optimizationundefined-behavior

Safety of using std::ptr::write_volatile for interior mutability in a Copy type (i.e. without UnsafeCell)


I'm trying to achieve interior mutability in a Copy value type (for caching purposes).

The problem is that, as far as I can tell, none of the types available for interior mutability (e.g. UnsafeCell and related types, atomic types) allows for the Copy trait. This is stable Rust btw.

My question is: how safe is using std::ptr::write_volatile and std::ptr::read_volatile for achieving interior mutability in this case?

Specifically, I have this code and I want to know if there's any gotcha or undefined behavior in this:

use std::fmt::{Debug, Display, Formatter};

#[derive(Copy, Clone)]
pub struct CopyCell<T: Copy> {
    value: T,
}

impl<T: Copy> CopyCell<T> {
    pub fn new(value: T) -> Self {
        Self { value }
    }

    pub fn get(&self) -> T {
        unsafe { std::ptr::read_volatile(&self.value) }
    }

    pub fn set(&self, value: T) {
        let ptr = &self.value as *const T;
        let ptr = ptr as *mut T;
        unsafe { std::ptr::write_volatile(ptr, value) }
    }
}

impl<T: Default + Copy> Default for CopyCell<T> {
    fn default() -> Self {
        CopyCell { value: T::default() }
    }
}

impl<T: Display + Copy> Display for CopyCell<T> {
    fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
        let value = self.get();
        write!(f, "{value}")
    }
}

impl<T: Debug + Copy> Debug for CopyCell<T> {
    fn fmt(&self, f: &mut Formatter) -> std::fmt::Result {
        let value = self.get();
        write!(f, "{value:?}")
    }
}

My goal is to use that to implement a local cache inside a value type (e.g. a sort of memoization). I specifically want Copy semantics (i.e. instead of Clone) because they have better ergonomics for what I'm trying to achieve.

Here is a sample usage:

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn simple_test() {
        let a = CopyCell::default();
        let b = CopyCell::new(42);

        assert_eq!(a.get(), 0);
        assert_eq!(b.get(), 42);

        a.set(123);
        b.set(b.get() * 10);
        assert_eq!(a.get(), 123);
        assert_eq!(b.get(), 420);

        let a1 = a;
        let b1 = b;

        a.set(0);
        b.set(0);

        assert_eq!(a1.get(), 123);
        assert_eq!(b1.get(), 420);
    }

    #[test]
    fn cached_compute() {
        let a = CachedCompute::new(10);
        assert_eq!(a.compute(), 100);
        assert_eq!(a.compute(), 100);

        let b = a;
        assert_eq!(b.compute(), 100);
    }

    #[derive(Copy, Clone)]
    pub struct CachedCompute {
        source: i32,
        result: CopyCell<Option<i32>>,
    }

    impl CachedCompute {
        pub fn new(source: i32) -> Self {
            Self {
                source,
                result: Default::default(),
            }
        }

        pub fn compute(&self) -> i32 {
            if let Some(value) = self.result.get() {
                value
            } else {
                let result = self.source * self.source; // assume this is expensive
                self.result.set(Some(result));
                result
            }
        }
    }
}

The above code works on a first blush. But I would like to know if there are any potential problems with this approach which might not be obvious on a superficial level.

I don't have a good enough understanding of Rust's memory model, compiler optimizations, machine-code level semantics, etc., that could cause problems in this case.

And even if that approach would work for current platforms, I would like to know if I could run into potential issues in the future regarding undefined behavior.


Solution

  • No. There is absolutely no way to have interior mutability in Rust without UnsafeCell.

    Your code is UB and Miri marks it as such:

    error: Undefined Behavior: attempting a write access using <1698> at alloc880[0x0], but that tag only grants SharedReadOnly permission for this location
      --> src/main.rs:20:18
       |
    20 |         unsafe { std::ptr::write_volatile(ptr, value) }
       |                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
       |                  |
       |                  attempting a write access using <1698> at alloc880[0x0], but that tag only grants SharedReadOnly permission for this location
       |                  this error occurs as part of an access at alloc880[0x0..0x4]
       |
       = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
       = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
    help: <1698> was created by a SharedReadOnly retag at offsets [0x0..0x4]
      --> src/main.rs:18:19
       |
    18 |         let ptr = &self.value as *const T;
       |                   ^^^^^^^^^^^
       = note: BACKTRACE (of the first span):
       = note: inside `CopyCell::<i32>::set` at src/main.rs:20:18: 20:54
    note: inside `main`
      --> src/main.rs:51:5
       |
    51 |     a.set(123);
       |     ^^^^^^^^^^
    

    By using volatile you've just tricked the optimizer so your code "works", but it's still UB.

    People want indeed an UnsafeCell that is Copy, but it is a problem to add it now as people are relying on T: Copy to mean T does not have an UnsafeCell inside. Perhaps it'll be added some day, though.