Interoperation with cortex_m::peripheral::Peripheral.borrow is unsound

[japaric]

Background

The cortex-m crate defines a Peripheral type which is a new type around the address block address of the peripheral. We use that Peripheral in our Peripheral constructor. svd2rust generates a bunch of cortex_m::Peripheral "instances" (as pub consts) which the user later uses to create a rtfm::Peripheral static.

Problem

The problem is that cortex_m::Peripheral provides a borrow method that grants access to the peripheral within a cortex_m::interrupt::free critical section but that borrow doesn't know about the SRP or task priorities. Problematic code looks like this:

use rtfm::{C1, P1, P2, Peripheral}
use stm32f30x::interrupt;

// RESOURCES
static GPIOA: Peripheral<stm32f30x::Gpioa, C1> = unsafe { Peripheral::new(stm32f30x::GPIOA) };

// TASKS
tasks!(stm32f30x, {
    t1: Task {
        interrupt: Exti0,
        priority: P1,
        enabled: true,
    },
    t2: Task {
        interrupt: Exti0,
        priority: P2,
        enabled: true,
    },
});

fn t1(_task: interrupt::Gpioa, prio: P1) {
    let ceil = prio.as_ceiling();

    // NOTE GPIOA has type `cortex_m_rtfm::Peripheral`
    let gpioa = GPIOA.access(&prio, ceil);

    // Immediately preempts
    rtfm::request(t2);
}

fn t2(_task: interrupt::Gpiob, _prio: P2) {
    cortex_m::interrupt::free(|cs| {
        // this GPIOA has type `cortex_m::peripheral::Peripheral`
        // this breaks SRP because GPIOA has ceiling C1 which is less than this task priority (P2)
        let gpioa = stm32f30x::GPIOA.borrow(cs);
    });
}

Possible solutions

• Remove cortex_m::interrupt::free or have svd2rust generated code not create cortex_m::peripheral::Peripheral instances. Instead svd2rust should generate some other type for peripheral instances but that doesn't have a borrow method that works with cortex_m::interrupt::free

• Make cortex_m::Peripheral::borrow unsafe if and only if the cortex-m-rtfm crate is used with the cortex-m crate. I don't know if this is possible at all: Perhaps add a "rtfm" Cargo feature to cortex-m that makes cortex_m::Peripheral::borrow unsafe and have cortex-m-rtfm depend on cortex-m with that feature enabled. Because Cargo features are additive if one cortex-m instance in the dependency graph has the "rtfm" feature enabled then the cortex-m crate will always be compiled with that feature enabled. If this does work, I think it would probably be considered a misuse of Cargo features...

[awelkie]

Other possible solutions:

• Write a lint plugin to warn when using the cortex_m::Peripheral::borrow method

• Instead of exposing the Peripherals as pub consts, expose them as non-Sync objects. That way, svd2rust doesn't give a synchronization method and the user is responsible for synchronization if they need it. One way could be to have pub const Option<Peripheral>, and then any code (rtfm in this case) could take ownership of the peripheral by using Option::take.

Not sure if these are workable, just throwing them out there.

[japaric]

This was actually not fixed by #34. Peripheral.borrow can bypass all the scope / ceiling machinery.

[japaric]

This has been (finally!) fixed in #50. 🎉