Skip to content
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

Remove aliasing &mut references to DMA buffers (possible UB) #41

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
6 changes: 3 additions & 3 deletions .github/bors.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ block_labels = ["wip"]
delete_merged_branches = true
status = [
"Rustfmt",
"ci (1.68.2, log-rtt)",
"ci (1.68.2, log-itm)",
"ci (1.68.2, log-semihosting)",
"ci (1.82.0, log-rtt)",
"ci (1.82.0, log-itm)",
"ci (1.82.0, log-semihosting)",
"ci (stable, log-rtt)",
"ci (stable, log-itm))",
"ci (stable, log-semihosting)",
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
strategy:
matrix: # All permutations of {rust, mcu}
rust:
- 1.70.0 # MSRV
- 1.82.0 # MSRV
- stable
logger:
- log-rtt
Expand Down
3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ cortex-m-log = { version = "~0.8", features = ["itm", "semihosting", "log-integr
panic-itm = { version = "~0.4.2", optional = true }
panic-semihosting = { version = "0.6.0", optional = true }
cortex-m-semihosting = { version = "0.5.0", optional = true }
stable_deref_trait = { version = "1.2.0", default-features = false }

[features]
default = []
Expand Down Expand Up @@ -55,7 +56,7 @@ debug = true # symbols are nice and they don't increase the size in flash
lto = true # better optimizations
opt-level = "s" # optimize for binary size

[dev_dependencies]
[dev-dependencies]
embedded-sdmmc = "0.5.0"
usbd-midi = "0.3.0"
num_enum = { version = "0.7.3", default-features = false }
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ cargo objcopy --example passthru --release -- -O binary passthru.bin
[cargo-binutils-url]: https://github.com/rust-embedded/cargo-binutils

# Minimum supported Rust version
The Minimum Supported Rust Version (MSRV) at the moment is 1.70.0
The Minimum Supported Rust Version (MSRV) at the moment is 1.82.0
# Demos

[Looper](https://github.com/mtthw-meyer/daisy-looper) - Basic one button looper.
5 changes: 4 additions & 1 deletion examples/usb_midi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ mod app {
midi_device::MidiClass,
};

// Warning: EP_MEMORY may only be used for the UsbBusAllocator. Any
// additional references are UB.
static mut EP_MEMORY: [u32; 1024] = [0; 1024];

#[shared]
Expand Down Expand Up @@ -68,7 +70,7 @@ mod app {
let mut timer2 = device.TIM2.timer(
MilliSeconds::from_ticks(200).into_rate(),
ccdr.peripheral.TIM2,
&mut ccdr.clocks,
&ccdr.clocks,
);
timer2.listen(Event::TimeOut);

Expand Down Expand Up @@ -127,6 +129,7 @@ mod app {
&ccdr.clocks,
);

#[allow(static_mut_refs)]
let usb_bus = cortex_m::singleton!(
: usb_device::class_prelude::UsbBusAllocator<UsbBus<USB2>> =
UsbBus::new(usb, unsafe { &mut EP_MEMORY })
Expand Down
65 changes: 49 additions & 16 deletions src/audio.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
//! Audio module. Handles audio startup and I/O.
//! As well as converting between the S24 input and f32 for processing.
use core::ops::{Deref, DerefMut};

use log::info;

use stm32h7xx_hal::{
dma,
gpio::{gpioe, Analog},
pac, rcc,
rcc::rec,
sai,
sai::*,
stm32,
stm32::rcc::d2ccip1r::SAI1SEL_A,
pac::{self},
rcc::{self, rec},
sai::{self, *},
stm32::{self, rcc::d2ccip1r::SAI1SEL_A},
traits::i2s::FullDuplex,
};

Expand Down Expand Up @@ -41,19 +41,44 @@ pub const MAX_TRANSFER_SIZE: usize = BLOCK_SIZE_MAX * 2;

pub type AudioBuffer = [(f32, f32); BLOCK_SIZE_MAX];

/// Raw pointer backed reference to the DMA buffers. It exists to avoid storing multiple aliasing
/// `&mut` references to `TX_BUFFER` and `RX_BUFFER`, which is UB.
/// # Safety
/// References are created whenever the underlying buffer is accessed, but since the access is single
/// threaded and the references are short lived this should be fine.
/// This wrapper is only, and may only be, pointing to memory with a 'static lifetime.
struct DmaBufferRawRef {
ptr: *mut DmaBuffer,
}
impl Deref for DmaBufferRawRef {
type Target = DmaBuffer;

fn deref(&self) -> &Self::Target {
unsafe { &*self.ptr }
}
}
impl DerefMut for DmaBufferRawRef {
fn deref_mut(&mut self) -> &mut Self::Target {
unsafe { &mut *self.ptr }
}
}
unsafe impl stable_deref_trait::StableDeref for DmaBufferRawRef {}
// Required for using the buffer in practice. No more dangerous than sending a `&mut DmaBuffer`
unsafe impl Send for DmaBufferRawRef {}

type DmaInputStream = dma::Transfer<
dma::dma::Stream1<stm32::DMA1>,
stm32::SAI1,
dma::PeripheralToMemory,
&'static mut [u32; DMA_BUFFER_SIZE],
DmaBufferRawRef,
dma::DBTransfer,
>;

type DmaOutputStream = dma::Transfer<
dma::dma::Stream0<stm32::DMA1>,
stm32::SAI1,
dma::MemoryToPeripheral,
&'static mut [u32; DMA_BUFFER_SIZE],
DmaBufferRawRef,
dma::DBTransfer,
>;

Expand Down Expand Up @@ -138,7 +163,9 @@ impl Audio {
let dma1_streams = dma::dma::StreamsTuple::new(dma1_d, dma1_p);

// dma1 stream 0
let tx_buffer: &'static mut [u32; DMA_BUFFER_SIZE] = unsafe { &mut TX_BUFFER };
let tx_buffer = DmaBufferRawRef {
ptr: &raw mut TX_BUFFER,
};
let dma_config = dma::dma::DmaConfig::default()
.priority(dma::config::Priority::High)
.memory_increment(true)
Expand All @@ -155,7 +182,9 @@ impl Audio {
);

// dma1 stream 1
let rx_buffer: &'static mut [u32; DMA_BUFFER_SIZE] = unsafe { &mut RX_BUFFER };
let rx_buffer = DmaBufferRawRef {
ptr: &raw mut RX_BUFFER,
};
let dma_config = dma_config
.transfer_complete_interrupt(true)
.half_transfer_interrupt(true);
Expand Down Expand Up @@ -207,8 +236,12 @@ impl Audio {
sai.enable();
sai.try_send(0, 0).unwrap();
});
let input = Input::new(unsafe { &mut RX_BUFFER });
let output = Output::new(unsafe { &mut TX_BUFFER });
let input = Input::new(DmaBufferRawRef {
ptr: &raw mut RX_BUFFER,
});
let output = Output::new(DmaBufferRawRef {
ptr: &raw mut TX_BUFFER,
});
info!(
"{:?}, {:?}",
&input.buffer[0] as *const u32, &output.buffer[0] as *const u32
Expand Down Expand Up @@ -289,12 +322,12 @@ impl Audio {

struct Input {
index: usize,
buffer: &'static DmaBuffer,
buffer: DmaBufferRawRef,
}

impl Input {
/// Create a new Input from a DmaBuffer
fn new(buffer: &'static DmaBuffer) -> Self {
fn new(buffer: DmaBufferRawRef) -> Self {
Self { index: 0, buffer }
}

Expand All @@ -310,12 +343,12 @@ impl Input {

struct Output {
index: usize,
buffer: &'static mut DmaBuffer,
buffer: DmaBufferRawRef,
}

impl Output {
/// Create a new Input from a DmaBuffer
fn new(buffer: &'static mut DmaBuffer) -> Self {
fn new(buffer: DmaBufferRawRef) -> Self {
Self { index: 0, buffer }
}

Expand Down
Loading