rust-embedded / embedded-dma

Apache License 2.0
35 stars 12 forks source link

Add API to use stack-allocated buffers safely. #11

Open Dirbaio opened 3 years ago

Dirbaio commented 3 years ago

Currently using stack-allocated buffers is impossible without unsafe. Here are two ideas on how to do this, coming from our conversations in Matrix :)

I'm not sure which one is best, we should explore tradeoffs and decide (or have both?)

Method 1: closure-based


pub struct Buffer<'a> {
    inner: &'a mut [u8],
}

unsafe impl<'a> ReadBuffer for Buffer<'a> {
    type Word = u8;
    unsafe fn read_buffer(&self) -> (*const Self::Word, usize) {
        (self.inner.as_ptr(), self.inner.len())
    }
}

unsafe impl<'a> WriteBuffer for Buffer<'a> {
    type Word = u8;
    unsafe fn write_buffer(&mut self) -> (*mut Self::Word, usize) {
        (self.inner.as_mut_ptr(), self.inner.len())
    }
}

fn with_buffer<'a, R>(buf: &'a mut [u8], f: impl FnOnce(Buffer<'a>) -> (R, Buffer<'a>)) -> R {
    let orig_loc = (buf.as_ptr(), buf.len());
    let (r, ret_buf) = f(Buffer { inner: buf });
    let ret_loc = (ret_buf.inner.as_ptr(), ret_buf.inner.len());
    core::assert_eq!(orig_loc, ret_loc); // Not sure if this is needed for safety.
    r
}

Method 2: flag-based

#[feature(min_const_generics)]
use core::marker::PhantomPinned;
use core::panic;
use core::pin::Pin;
//use embedded_dma::{ReadBuffer, WriteBuffer};
use futures::pin_mut;

struct DmaBuffer<B: ?Sized> {
    pinned: PhantomPinned,
    in_use: bool,
    buf: B,
}

impl<B> DmaBuffer<B> {
    pub fn new(b: B) -> Self {
        Self {
            pinned: PhantomPinned,
            in_use: false,
            buf: b,
        }
    }
}

impl<const N: usize> DmaBuffer<[u8; N]> {
    pub fn borrow<'a>(self: Pin<&'a mut Self>) -> DmaBufferBorrow<'a, [u8]> {
        let this = unsafe { self.get_unchecked_mut() };
        if this.in_use {
            panic!("You can't borrow a buffer two times!")
        }
        this.in_use = true;
        DmaBufferBorrow(unsafe { Pin::new_unchecked(this) })
    }
}

impl<B: ?Sized> Drop for DmaBuffer<B> {
    fn drop(&mut self) {
        if self.in_use {
            panic!("You leaked a buffer borrow!")
        }
    }
}

struct DmaBufferBorrow<'a, B: ?Sized>(Pin<&'a mut DmaBuffer<B>>);

impl<'a, B: ?Sized> Drop for DmaBufferBorrow<'a, B> {
    fn drop(&mut self) {
        unsafe { self.0.as_mut().get_unchecked_mut() }.in_use = false;
    }
}

fn use_buffer<'a>(buf: DmaBufferBorrow<'a, [u8]>) {}

unsafe impl<'a> ReadBuffer for DmaBufferBorrow<'a, [u8]> {
    type Word = u8;
    unsafe fn read_buffer(&self) -> (*const Self::Word, usize) {
        (self.0.buf.as_ptr(), self.0.buf.len())
    }
}

unsafe impl<'a> WriteBuffer for DmaBufferBorrow<'a, [u8]> {
    type Word = u8;
    unsafe fn write_buffer(&mut self) -> (*mut Self::Word, usize) {
        let buf = unsafe { self.0.as_mut().get_unchecked_mut() };
        (buf.buf.as_mut_ptr(), buf.buf.len())
    }
}

fn main() {
    let mut buf = DmaBuffer::new([0u8; 1024]);
    pin_mut!(buf);

    println!("in use: {}", buf.in_use);
    let mut borrow = buf.as_mut().borrow();
    //println!("in use: {}", buf.in_use); This would print true
    use_buffer(borrow);
    println!("in use: {}", buf.in_use);
}
Finomnis commented 1 year ago

Be aware that in your second example, nothing prevents the user from dropping buf while it is still in use. The flag does prevent re-use, but if you forget() the borrow object, we can drop the buf object while the DMA is still running.

We could somewhat mitigate that by checking the in_use flag in drop() of buf, but a forced panic in drop() is usually never a good idea.

EDIT: I guess something similar could be done in the first case, inside of the closure.

Sympatron commented 1 year ago

I don't think that is true. In the first case you can't forget or drop buf because you don't own it and in the second case you can't drop or forget buf because it is borrowed.

Finomnis commented 1 year ago

I think you misunderstand what I'm trying to say; I'm not talking about forgetting buf in the second case. I'm taking about forgetting borrow, releasing it's 'a reference without running its drop function. But maybe I misunderstand your code, of course.

Sympatron commented 1 year ago

You are right, I misunderstood. If you forget(borrow) and then forget(buf) while buf.in_use == true you would have a bad time. But that is not a problem, because using the borrow (write_buffer(), read_buffer()) is still unsafe. Therefore you can't cause this problem without using unsafe.