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

Review DMA implementation for Undefined Behavior (UB) #906

Open
ryan-summers opened this issue Jun 18, 2024 · 3 comments
Open

Review DMA implementation for Undefined Behavior (UB) #906

ryan-summers opened this issue Jun 18, 2024 · 3 comments

Comments

@ryan-summers
Copy link
Member

It was noted by James Munns that we may need to use an UnsafeCell internal wrapper on the DMA reception buffers to prevent invoking undefined behavior by the Rust compiler (i.e. two owners of the memory cell)

@jamesmunns
Copy link

@ryan-summers if you can point me to the driver impl(s) I can give a review, or at least my opinions.

@ryan-summers
Copy link
Member Author

ryan-summers commented Jun 18, 2024

The implementations are specifically in:

Note that there's a few single-byte transfers in there that are not concerning - the memory is static and entirely owned by the DMA module and is never modified.

It's worth noting that this is a double-buffered DMA implementation using the STM32H7xx-Hal. This is implemented here for swapping the DMA double-buffer for the next transfer

@dnadlinger
Copy link
Contributor

Possibly related: nightly now emits warnings like this (same for other DMA/Ethernet description ring buffers), and it seems like this is going to turn into deny by default in 2024 still

warning: creating a mutable reference to mutable static is discouraged
   --> src/hardware/adc.rs:378:25
    |
378 |                           ADC_BUF.write(Default::default())
    |                           ^^^^^^^ mutable reference to mutable static
...
446 | / adc_input!(
447 | |     Adc1Input, 1, Stream3, Stream4, Stream5, SPI3, Channel2, Tim2Ch2, Channel2,
448 | |     Tim3Ch2
449 | | );
    | |_- in this macro invocation
    |
    = note: for more information, see <https://doc.rust-lang.org/nightly/edition-guide/rust-2024/static-mut-references.html>
    = note: mutable references to mutable statics are dangerous; it's undefined behavior if any other pointer to the static is used or if any other reference is created for the static while the mutable reference lives
    = note: this warning originates in the macro `adc_input` (in Nightly builds, run with -Z macro-backtrace for more info)

I am not sure I understand the documentation here – is ptr::addr_of_mut!(ADC_BUF) a valid "fix"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants