Eagerly address select Rust 2024 edition changes

logging/src/defmt.rs

@@ -97,7 +97,7 @@ mod frontend {
         // module shows that this is always accessed within the acquire-
         // release critical section, so there's only one access to this
         // mutable data.
-        unsafe { &mut ENCODER }
+        unsafe { &mut *core::ptr::addr_of_mut!(ENCODER) }
     }
 }
 

logging/src/lib.rs

@@ -253,7 +253,7 @@
 //! [log-docs]: https://docs.rs/log/0.4/log/
 
 #![no_std]
-#![warn(missing_docs)]
+#![warn(missing_docs, unsafe_op_in_unsafe_fn)]
 
 #[cfg(feature = "defmt")]
 pub mod defmt;

logging/src/log/frontend.rs

@@ -67,11 +67,13 @@ pub(crate) unsafe fn init(
     // We should be preventing multiple callers with the critical section,
     // so the "only happens once" is to ensure that we're not changing the
     // static while the logger is active.
-    LOGGER.write(Logger {
-        producer: Mutex::new(RefCell::new(producer)),
-        filters: super::Filters(config.filters),
-    });
-    ::log::set_logger(&*LOGGER.as_ptr())
+    let logger = unsafe {
+        LOGGER.write(Logger {
+            producer: Mutex::new(RefCell::new(producer)),
+            filters: super::Filters(config.filters),
+        })
+    };
+    ::log::set_logger(logger)
         .map(|_| ::log::set_max_level(config.max_level))
         .map_err(|_| crate::AlreadySetError::new(()))
 }

logging/src/lpuart.rs

@@ -29,8 +29,9 @@ pub(crate) const VTABLE: crate::PollerVTable = crate::PollerVTable { poll };
 /// of these guarantees. The `Poller` indirectly "owns" the static mut memory,
 /// and the crate design ensures that there's only one `Poller` object in existence.
 unsafe fn poll() {
-    let consumer = CONSUMER.assume_init_mut();
-    let channel = CHANNEL.assume_init_mut();
+    // Safety: caller ensures that these are initializd, and that the function
+    // isn't reentrant.
+    let (consumer, channel) = unsafe { (CONSUMER.assume_init_mut(), CHANNEL.assume_init_mut()) };
 
     // Could be high if the user enabled DMA interrupts.
     while channel.is_interrupt() {
@@ -85,9 +86,14 @@ unsafe fn poll() {
         };
 
         if !new.is_empty() {
-            channel::set_source_linear_buffer(channel, new);
-            channel.set_transfer_iterations(new.len().min(u16::MAX as usize) as u16);
-            channel.enable();
+            // Safety: the buffer is static and will always be valid while the transfer
+            // is active.
+            unsafe { channel::set_source_linear_buffer(channel, new) };
+            // Safety: the iterations are based on the number of elements in the collection,
+            // so we're not indexing out of bounds.
+            unsafe { channel.set_transfer_iterations(new.len().min(u16::MAX as usize) as u16) };
+            // Safety: transfer is correctly set up here, and in the init method.
+            unsafe { channel.enable() };
         }
 
         if !completed.is_empty() {
@@ -111,20 +117,22 @@ pub(crate) unsafe fn init<P, const LPUART: u8>(
     channel.disable();
     channel.clear_complete();
     channel.clear_error();
-    channel.set_minor_loop_bytes(core::mem::size_of::<u8>() as u32);
 
-    CONSUMER.write(consumer);
-    let channel = CHANNEL.write(channel);
+    // Safety: mutable static access. Caller only calls this once, and poll() isn't
+    // accessible by this point. There's no race on the consumer.
+    unsafe { CONSUMER.write(consumer) };
+    // Safety: mutable static access. See above.
+    let channel = unsafe { CHANNEL.write(channel) };
 
     channel.set_disable_on_completion(true);
     channel.set_interrupt_on_completion(interrupts == crate::Interrupts::Enabled);
 
     channel.set_channel_configuration(channel::Configuration::enable(lpuart.destination_signal()));
-    // Safety: size is appropriate for the buffer type.
-    channel.set_minor_loop_bytes(core::mem::size_of::<u8>() as u32);
+    // Safety: element size is appropriate for the buffer type.
+    unsafe { channel.set_minor_loop_bytes(core::mem::size_of::<u8>() as u32) };
 
     // Safety: hardware address is valid.
-    channel::set_destination_hardware(channel, lpuart.destination_address());
+    unsafe { channel::set_destination_hardware(channel, lpuart.destination_address()) };
 
     lpuart.disable(|lpuart| {
         lpuart.disable_fifo(Direction::Tx);

logging/src/usbd.rs

@@ -46,8 +46,22 @@ pub(crate) const VTABLE: crate::PollerVTable = crate::PollerVTable { poll };
 /// and the crate design ensures that there's only one `Poller` object in existence.
 unsafe fn poll() {
     static mut CONFIGURED: bool = false;
-    let device = DEVICE.assume_init_mut();
-    let class = CLASS.assume_init_mut();
+    #[inline(always)]
+    fn is_configured() -> bool {
+        // Safety: poll() isn't reentrant. Only poll() can
+        // read this state.
+        unsafe { CONFIGURED }
+    }
+    #[inline(always)]
+    fn set_configured(configured: bool) {
+        // Safety: poll() isn't reentrant. Only poll() can
+        // modify this state.s
+        unsafe { CONFIGURED = configured };
+    }
+
+    // Safety: caller ensures that these are initializd, and that the function
+    // isn't reentrant.
+    let (device, class) = unsafe { (DEVICE.assume_init_mut(), CLASS.assume_init_mut()) };
 
     // Is there a CDC class event, like a completed transfer? If so, check
     // the consumer immediately, even if a timer hasn't expired.
@@ -79,18 +93,18 @@ unsafe fn poll() {
     let check_consumer = class_event || timer_event;
 
     if device.state() != UsbDeviceState::Configured {
-        if CONFIGURED {
+        if is_configured() {
             // Turn off the timer, but only if we were previously configured.
             device.bus().gpt_mut(GPT_INSTANCE, |gpt| gpt.stop());
         }
-        CONFIGURED = false;
+        set_configured(false);
         // We can't use the class if we're not configured,
         // so bail out here.
         return;
     }
 
     // We're now configured. Are we newly configured?
-    if !CONFIGURED {
+    if !is_configured() {
         // Must call this when we transition into configured.
         device.bus().configure();
         device.bus().gpt_mut(GPT_INSTANCE, |gpt| {
@@ -102,7 +116,7 @@ unsafe fn poll() {
                 gpt.run()
             }
         });
-        CONFIGURED = true;
+        set_configured(true);
     }
 
     // If the host sends us data, pretend to read it.
@@ -112,7 +126,9 @@ unsafe fn poll() {
 
     // There's no need to wait if we were are newly configured.
     if check_consumer {
-        let consumer = CONSUMER.assume_init_mut();
+        // Safety: consumer is initialized if poll() is running. Caller ensures
+        // that poll() is not reentrant.
+        let consumer = unsafe { CONSUMER.assume_init_mut() };
         if let Ok(grant) = consumer.read() {
             let buf = grant.buf();
             // Don't try to write more than we can fit in a single packet!
@@ -140,15 +156,22 @@ pub(crate) unsafe fn init<P: imxrt_usbd::Peripherals>(
     consumer: super::Consumer,
     config: &UsbdConfig,
 ) {
-    CONSUMER.write(consumer);
+    // Safety: mutable static write. This only occurs once, and poll()
+    // cannot run by the time we're writing this memory.
+    unsafe { CONSUMER.write(consumer) };
 
-    let bus = {
-        let bus = imxrt_usbd::BusAdapter::without_critical_sections(
-            peripherals,
-            &ENDPOINT_MEMORY,
-            &ENDPOINT_STATE,
-            crate::config::USB_SPEED,
-        );
+    let bus: &mut usb_device::class_prelude::UsbBusAllocator<imxrt_usbd::BusAdapter> = {
+        // Safety: we ensure that the bus, class, and all other related USB objects
+        // are accessed in poll(). poll() is not reentrant, so there's no racing
+        // occuring across executing contexts.
+        let bus = unsafe {
+            imxrt_usbd::BusAdapter::without_critical_sections(
+                peripherals,
+                &ENDPOINT_MEMORY,
+                &ENDPOINT_STATE,
+                crate::config::USB_SPEED,
+            )
+        };
         bus.set_interrupts(interrupts == crate::Interrupts::Enabled);
         bus.gpt_mut(GPT_INSTANCE, |gpt| {
             gpt.stop();
@@ -159,12 +182,15 @@ pub(crate) unsafe fn init<P: imxrt_usbd::Peripherals>(
             gpt.reset();
         });
         let bus = usb_device::bus::UsbBusAllocator::new(bus);
-        BUS.write(bus)
+        // Safety: mutable static write. This only occurs once, and poll()
+        // cannot run by the time we're writing this memory.
+        unsafe { BUS.write(bus) }
     };
 
     {
         let class = usbd_serial::CdcAcmClass::new(bus, MAX_PACKET_SIZE as u16);
-        CLASS.write(class);
+        // Safety: mutable static write. See the above discussion for BUS.
+        unsafe { CLASS.write(class) };
     }
 
     {
@@ -185,7 +211,8 @@ pub(crate) unsafe fn init<P: imxrt_usbd::Peripherals>(
             }
         }
 
-        DEVICE.write(device);
+        // Safety: mutable static write. See the above discussion for BUS.
+        unsafe { DEVICE.write(device) };
     }
 }
 

src/common/pit.rs

@@ -129,7 +129,9 @@ impl<const CHAN: u8> Pit<CHAN> {
         Const<CHAN>: Valid,
     {
         let register_block: &'_ crate::ral::pit::RegisterBlock = instance;
-        let register_block: &'static _ = core::mem::transmute(register_block);
+        // Safety: extending lifetime when we know that the register block has
+        // static lifetime.
+        let register_block: &'static _ = unsafe { core::mem::transmute(register_block) };
         Self {
             instance: register_block,
         }

src/lib.rs

@@ -81,7 +81,7 @@
 //! `"unproven"` feature enabled in embedded-hal 0.2.
 
 #![no_std]
-#![warn(missing_docs)]
+#![warn(missing_docs, unsafe_op_in_unsafe_fn)]
 
 use imxrt_ral as ral;