Use SyncUnsafeCell to replace use of static mut

Author: richardeoin

examples/dma.rs

@@ -6,6 +6,9 @@
 
 use core::mem::MaybeUninit;
 
+// TODO: use core::cell::SyncUnsafeCell when stabilized rust-lang/rust#95439
+use utilities::sync_unsafe_cell::SyncUnsafeCell;
+
 use cortex_m_rt::entry;
 #[macro_use]
 mod utilities;
@@ -24,9 +27,11 @@ use log::info;
 //
 // The runtime does not initialise these SRAM banks.
 #[link_section = ".sram4.buffers"]
-static mut SOURCE_BUFFER: MaybeUninit<[u32; 20]> = MaybeUninit::uninit();
+static SOURCE_BUFFER: MaybeUninit<SyncUnsafeCell<[u32; 20]>> =
+    MaybeUninit::uninit();
 #[link_section = ".axisram.buffers"]
-static mut TARGET_BUFFER: MaybeUninit<[u32; 20]> = MaybeUninit::uninit();
+static TARGET_BUFFER: MaybeUninit<SyncUnsafeCell<[u32; 20]>> =
+    MaybeUninit::uninit();
 
 #[entry]
 fn main() -> ! {
@@ -53,29 +58,37 @@ fn main() -> ! {
     info!("stm32h7xx-hal example - Memory to Memory DMA");
     info!("");
 
-    // Initialise the source buffer with truly random data, without taking any
-    // references to uninitialisated memory
-    let source_buffer: &'static mut [u32; 20] = {
-        let buf: &mut [MaybeUninit<u32>; 20] = unsafe {
-            &mut *(core::ptr::addr_of_mut!(SOURCE_BUFFER)
-                as *mut [MaybeUninit<u32>; 20])
-        };
-
-        for value in buf.iter_mut() {
-            unsafe {
-                value.as_mut_ptr().write(rng.gen().unwrap());
-            }
-        }
-        #[allow(static_mut_refs)] // TODO: Fix this
-        unsafe {
-            SOURCE_BUFFER.assume_init_mut()
+    // SOURCE_BUFFER is located in .axisram.buffers, which is not initialized by
+    // the runtime. We must manually initialize it to a valid value, without
+    // taking a reference to the uninitialized value
+    unsafe {
+        let cell = SOURCE_BUFFER.as_ptr();
+        for i in 0..20 {
+            core::ptr::addr_of_mut!((*SyncUnsafeCell::raw_get(cell))[i])
+                .write(rng.gen().unwrap());
         }
-    };
+    }
+    // Now we can take a mutable reference to SOURCE_BUFFER. To avoid aliasing,
+    // this reference must only be taken once
+    let source_buffer =
+        unsafe { &mut *SyncUnsafeCell::raw_get(SOURCE_BUFFER.as_ptr()) };
     // Save a copy on the stack so we can check it later
     let source_buffer_cloned = *source_buffer;
 
-    // NOTE(unsafe): TARGET_BUFFER must also be initialised to prevent undefined
-    // behaviour (taking a mutable reference to uninitialised memory)
+    // TARGET_BUFFER is located in .axisram.buffers, which is not initialized by
+    // the runtime. We must manually initialize it to a valid value, without
+    // taking a reference to the uninitialized value
+    unsafe {
+        let cell = TARGET_BUFFER.as_ptr();
+        for i in 0..20 {
+            core::ptr::addr_of_mut!((*SyncUnsafeCell::raw_get(cell))[i])
+                .write(0);
+        }
+    }
+    // Now we can take a mutable reference to TARGET_BUFFER. To avoid aliasing,
+    // this reference must only be taken once
+    let target_buffer =
+        unsafe { &mut *SyncUnsafeCell::raw_get(TARGET_BUFFER.as_ptr()) };
 
     // Setup DMA
     //
@@ -92,9 +105,7 @@ fn main() -> ! {
         Transfer::init(
             streams.4,
             MemoryToMemory::new(),
-            unsafe {
-                (*core::ptr::addr_of_mut!(TARGET_BUFFER)).assume_init_mut()
-            }, // Uninitialised memory
+            target_buffer,
             Some(source_buffer),
             config,
         );
@@ -104,10 +115,11 @@ fn main() -> ! {
     // Wait for transfer to complete
     while !transfer.get_transfer_complete_flag() {}
 
-    // Now the target memory is actually initialised
-    #[allow(static_mut_refs)] // TODO: Fix this
-    let target_buffer: &'static mut [u32; 20] =
-        unsafe { TARGET_BUFFER.assume_init_mut() };
+    // Take a second(!) reference to the target buffer. Because the transfer has
+    // stopped, we can reason that the first reference is no longer
+    // active. Therefore we can take another reference without causing aliasing
+    let target_buffer: &[u32; 20] =
+        unsafe { &*SyncUnsafeCell::raw_get(TARGET_BUFFER.as_ptr()) };
 
     // Comparison check
     assert_eq!(&source_buffer_cloned, target_buffer);

examples/ethernet-nucleo-h743zi2.rs

@@ -17,6 +17,9 @@ use core::mem::MaybeUninit;
 use core::sync::atomic::{AtomicU32, Ordering};
 use rt::{entry, exception};
 
+// TODO: use core::cell::SyncUnsafeCell when stabilized rust-lang/rust#95439
+use utilities::sync_unsafe_cell::SyncUnsafeCell;
+
 extern crate cortex_m;
 
 #[macro_use]
@@ -52,7 +55,7 @@ const MAC_ADDRESS: [u8; 6] = [0x02, 0x00, 0x11, 0x22, 0x33, 0x44];
 
 /// Ethernet descriptor rings are a global singleton
 #[link_section = ".sram3.eth"]
-static mut DES_RING: MaybeUninit<ethernet::DesRing<4, 4>> =
+static DES_RING: MaybeUninit<SyncUnsafeCell<ethernet::DesRing<4, 4>>> =
     MaybeUninit::uninit();
 
 // the program entry point
@@ -113,9 +116,18 @@ fn main() -> ! {
     assert_eq!(ccdr.clocks.pclk4().raw(), 100_000_000); // PCLK 100MHz
 
     let mac_addr = smoltcp::wire::EthernetAddress::from_bytes(&MAC_ADDRESS);
-    let (_eth_dma, eth_mac) = unsafe {
-        #[allow(static_mut_refs)] // TODO: Fix this
-        DES_RING.write(ethernet::DesRing::new());
+    let (_eth_dma, eth_mac) = {
+        // DES_RING is located in .axisram.eth, which is not initialized by
+        // the runtime. We must manually initialize it to a valid value,
+        // without taking a reference to the uninitialized value
+        unsafe {
+            SyncUnsafeCell::raw_get(DES_RING.as_ptr())
+                .write(ethernet::DesRing::new());
+        }
+        // Now we can take a mutable reference to DES_RING. To avoid
+        // aliasing, this reference must only be taken once
+        let des_ring =
+            unsafe { &mut *SyncUnsafeCell::raw_get(DES_RING.as_ptr()) };
 
         ethernet::new(
             dp.ETHERNET_MAC,
@@ -132,8 +144,7 @@ fn main() -> ! {
                 rmii_txd0,
                 rmii_txd1,
             ),
-            #[allow(static_mut_refs)] // TODO: Fix this
-            DES_RING.assume_init_mut(),
+            des_ring,
             mac_addr,
             ccdr.peripheral.ETH1MAC,
             &ccdr.clocks,

examples/ethernet-rtic-nucleo-h723zg.rs

@@ -20,9 +20,11 @@
 mod utilities;
 
 use core::mem::MaybeUninit;
-use core::ptr::addr_of_mut;
 use core::sync::atomic::AtomicU32;
 
+// TODO: use core::cell::SyncUnsafeCell when stabilized rust-lang/rust#95439
+use utilities::sync_unsafe_cell::SyncUnsafeCell;
+
 use smoltcp::iface::{Config, Interface, SocketSet, SocketStorage};
 use smoltcp::time::Instant;
 use smoltcp::wire::{HardwareAddress, IpAddress, IpCidr};
@@ -49,17 +51,19 @@ const MAC_ADDRESS: [u8; 6] = [0x02, 0x00, 0x11, 0x22, 0x33, 0x44];
 
 /// Ethernet descriptor rings are a global singleton
 #[link_section = ".axisram.eth"]
-static mut DES_RING: MaybeUninit<ethernet::DesRing<4, 4>> =
+static DES_RING: MaybeUninit<SyncUnsafeCell<ethernet::DesRing<4, 4>>> =
     MaybeUninit::uninit();
 
 /// Net storage with static initialisation - another global singleton
+#[derive(Default)]
 pub struct NetStorageStatic<'a> {
     socket_storage: [SocketStorage<'a>; 8],
 }
 
 // MaybeUninit allows us write code that is correct even if STORE is not
 // initialised by the runtime
-static mut STORE: MaybeUninit<NetStorageStatic> = MaybeUninit::uninit();
+static STORE: MaybeUninit<SyncUnsafeCell<NetStorageStatic>> =
+    MaybeUninit::uninit();
 
 pub struct Net<'a> {
     iface: Interface,
@@ -163,9 +167,18 @@ mod app {
         assert_eq!(ccdr.clocks.pclk4().raw(), 100_000_000); // PCLK 100MHz
 
         let mac_addr = smoltcp::wire::EthernetAddress::from_bytes(&MAC_ADDRESS);
-        let (eth_dma, eth_mac) = unsafe {
-            #[allow(static_mut_refs)] // TODO: Fix this
-            DES_RING.write(ethernet::DesRing::new());
+        let (eth_dma, eth_mac) = {
+            // DES_RING is located in .axisram.eth, which is not initialized by
+            // the runtime. We must manually initialize it to a valid value,
+            // without taking a reference to the uninitialized value
+            unsafe {
+                SyncUnsafeCell::raw_get(DES_RING.as_ptr())
+                    .write(ethernet::DesRing::new());
+            }
+            // Now we can take a mutable reference to DES_RING. To avoid
+            // aliasing, this reference must only be taken once
+            let des_ring =
+                unsafe { &mut *SyncUnsafeCell::raw_get(DES_RING.as_ptr()) };
 
             ethernet::new(
                 ctx.device.ETHERNET_MAC,
@@ -182,8 +195,7 @@ mod app {
                     rmii_txd0,
                     rmii_txd1,
                 ),
-                #[allow(static_mut_refs)] // TODO: Fix this
-                DES_RING.assume_init_mut(),
+                des_ring,
                 mac_addr,
                 ccdr.peripheral.ETH1MAC,
                 &ccdr.clocks,
@@ -198,22 +210,14 @@ mod app {
 
         unsafe { ethernet::enable_interrupt() };
 
-        // unsafe: mutable reference to static storage, we only do this once
-        let store = unsafe {
-            #[allow(static_mut_refs)] // TODO: Fix this
-            let store_ptr = STORE.as_mut_ptr();
-
-            // Initialise the socket_storage field. Using `write` instead of
-            // assignment via `=` to not call `drop` on the old, uninitialised
-            // value
-            addr_of_mut!((*store_ptr).socket_storage)
-                .write([SocketStorage::EMPTY; 8]);
-
-            // Now that all fields are initialised we can safely use
-            // assume_init_mut to return a mutable reference to STORE
-            #[allow(static_mut_refs)] // TODO: Fix this
-            STORE.assume_init_mut()
-        };
+        // Initialize STORE
+        unsafe {
+            SyncUnsafeCell::raw_get(STORE.as_ptr())
+                .write(NetStorageStatic::default());
+        }
+        // Now we can take a mutable reference to STORE. To avoid aliasing, this
+        // reference must only be taken once
+        let store = unsafe { &mut *SyncUnsafeCell::raw_get(STORE.as_ptr()) };
 
         let net = Net::new(store, eth_dma, mac_addr.into());
 

examples/ethernet-rtic-stm32h735g-dk.rs

@@ -16,9 +16,11 @@
 mod utilities;
 
 use core::mem::MaybeUninit;
-use core::ptr::addr_of_mut;
 use core::sync::atomic::AtomicU32;
 
+// TODO: use core::cell::SyncUnsafeCell when stabilized rust-lang/rust#95439
+use utilities::sync_unsafe_cell::SyncUnsafeCell;
+
 use smoltcp::iface::{Config, Interface, SocketSet, SocketStorage};
 use smoltcp::time::Instant;
 use smoltcp::wire::{HardwareAddress, IpAddress, IpCidr};
@@ -45,16 +47,19 @@ const MAC_ADDRESS: [u8; 6] = [0x02, 0x00, 0x11, 0x22, 0x33, 0x44];
 
 /// Ethernet descriptor rings are a global singleton
 #[link_section = ".axisram.eth"]
-static mut DES_RING: MaybeUninit<ethernet::DesRing<4, 4>> =
+static DES_RING: MaybeUninit<SyncUnsafeCell<ethernet::DesRing<4, 4>>> =
     MaybeUninit::uninit();
 
 // This data will be held by Net through a mutable reference
+#[derive(Default)]
 pub struct NetStorageStatic<'a> {
     socket_storage: [SocketStorage<'a>; 8],
 }
+
 // MaybeUninit allows us write code that is correct even if STORE is not
 // initialised by the runtime
-static mut STORE: MaybeUninit<NetStorageStatic> = MaybeUninit::uninit();
+static STORE: MaybeUninit<SyncUnsafeCell<NetStorageStatic>> =
+    MaybeUninit::uninit();
 
 pub struct Net<'a> {
     iface: Interface,
@@ -157,9 +162,18 @@ mod app {
         assert_eq!(ccdr.clocks.pclk4().raw(), 100_000_000); // PCLK 100MHz
 
         let mac_addr = smoltcp::wire::EthernetAddress::from_bytes(&MAC_ADDRESS);
-        let (eth_dma, eth_mac) = unsafe {
-            #[allow(static_mut_refs)] // TODO: Fix this
-            DES_RING.write(ethernet::DesRing::new());
+        let (eth_dma, eth_mac) = {
+            // DES_RING is located in .axisram.eth, which is not initialized by
+            // the runtime. We must manually initialize it to a valid value,
+            // without taking a reference to the uninitialized value
+            unsafe {
+                SyncUnsafeCell::raw_get(DES_RING.as_ptr())
+                    .write(ethernet::DesRing::new());
+            }
+            // Now we can take a mutable reference to DES_RING. To avoid
+            // aliasing, this reference must only be taken once
+            let des_ring =
+                unsafe { &mut *SyncUnsafeCell::raw_get(DES_RING.as_ptr()) };
 
             ethernet::new(
                 ctx.device.ETHERNET_MAC,
@@ -176,8 +190,7 @@ mod app {
                     rmii_txd0,
                     rmii_txd1,
                 ),
-                #[allow(static_mut_refs)] // TODO: Fix this
-                DES_RING.assume_init_mut(),
+                des_ring,
                 mac_addr,
                 ccdr.peripheral.ETH1MAC,
                 &ccdr.clocks,
@@ -192,22 +205,14 @@ mod app {
 
         unsafe { ethernet::enable_interrupt() };
 
-        // unsafe: mutable reference to static storage, we only do this once
-        let store = unsafe {
-            #[allow(static_mut_refs)] // TODO: Fix this
-            let store_ptr = STORE.as_mut_ptr();
-
-            // Initialise the socket_storage field. Using `write` instead of
-            // assignment via `=` to not call `drop` on the old, uninitialised
-            // value
-            addr_of_mut!((*store_ptr).socket_storage)
-                .write([SocketStorage::EMPTY; 8]);
-
-            // Now that all fields are initialised we can safely use
-            // assume_init_mut to return a mutable reference to STORE
-            #[allow(static_mut_refs)] // TODO: Fix this
-            STORE.assume_init_mut()
-        };
+        // Initialize STORE
+        unsafe {
+            SyncUnsafeCell::raw_get(STORE.as_ptr())
+                .write(NetStorageStatic::default());
+        }
+        // Now we can take a mutable reference to STORE. To avoid aliasing, this
+        // reference must only be taken once
+        let store = unsafe { &mut *SyncUnsafeCell::raw_get(STORE.as_ptr()) };
 
         let net = Net::new(store, eth_dma, mac_addr.into(), Instant::ZERO);