rtic-rs / rtic

Real-Time Interrupt-driven Concurrency (RTIC) framework for ARM Cortex-M microcontrollers
https://rtic.rs
Apache License 2.0
1.8k stars 209 forks source link

When using min-const-fn, SPSC can fail to be created #182

Closed korken89 closed 5 years ago

korken89 commented 5 years ago

We noticed at the Oxidize Workshop that there is a way to get the SPSC queue to fail to be made, see the example below. I checked the heapless implementation, and I was not able to get this to happen without RTFM, so maybe the issue is in here.

Any ideas? @japaric @TeXitoi Original code: https://github.com/korken89/rtfm_workshop

#![no_main]
#![no_std]

// panic handler
extern crate dwm1001;
extern crate panic_semihosting;

use cortex_m_semihosting::hprintln;
use dwm1001::nrf52832_hal as hal;
use hal::{DMAPool, TXQSize};

use heapless::{
    pool::singleton::Box,
    spsc::{Consumer, Producer, Queue},
};

use rtfm::app;

#[app(device = crate::hal::target)]
const APP: () = {
    static mut PRODUCER: Producer<'static, Box<DMAPool>, TXQSize> = ();
    static mut CONSUMER: Consumer<'static, Box<DMAPool>, TXQSize> = ();

    #[init(spawn = [])]
    fn init() -> init::LateResources {
        // for the producer/consumer of TX
        static mut TX_RB: Option<Queue<Box<DMAPool>, TXQSize>> = None;

        hprintln!("init").unwrap();

        *TX_RB = Some(Queue::new());
        let m = TX_RB.as_mut().take();

        // Should always be some
        match &m {
            Some(_) => hprintln!("Some ok").unwrap(),
            None => hprintln!("Danger danger").unwrap(), // <-- This happens!!!! D:
        }

        let (txp, txc) = m.unwrap().split();

        init::LateResources {
            PRODUCER: txp,
            CONSUMER: txc,
        }
    }
};
japaric commented 5 years ago

I can repro this on the stm32f103xx (blue-pill) using rustc 1.36.0-nightly (7c71bc320 2019-04-30). The problem goes away if I replace Box<DMAPool> with struct NotBox; so this may be related to destructors.

The problem also also goes away if I go from heapless' min-const-fn feature to const-fn. That change in features also changes the internal use of heapless::MaybeUninit to core::mem::MaybeUninit. So this could be UB caused by trying to implement core::mem::MaybeUninit on stable, similar to #149.

japaric commented 5 years ago

@korken89 could you try PR #187 / #188? It fixes this issue for me; I tested on the blue-pill.

korken89 commented 5 years ago

I will test it on the nRF52832

korken89 commented 5 years ago

@japaric I have now tested, and the error still persists with heapless v0.4.4 on an nRF52832, if you need to borrow hardware I can fix that

korken89 commented 5 years ago

I saw that the real MaybeUninit is on its way in, should we close this and await the beta and stable release?

perlindgren commented 5 years ago

At least to me it makes sense to close this awaiting permanent fix.

/Per

korken89 commented 5 years ago

SGTM