Skip to content

Use SyncUnsafeCell to replace use of static mut #518

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 40 additions & 28 deletions examples/dma.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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() -> ! {
Expand All @@ -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
//
Expand All @@ -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,
);
Expand All @@ -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);
Expand Down
23 changes: 17 additions & 6 deletions examples/ethernet-nucleo-h743zi2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
52 changes: 28 additions & 24 deletions examples/ethernet-rtic-nucleo-h723zg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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());

Expand Down
53 changes: 29 additions & 24 deletions examples/ethernet-rtic-stm32h735g-dk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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);

Expand Down
Loading
Loading