Skip to content
This repository has been archived by the owner on Jan 24, 2022. It is now read-only.

compile on stable #69

Merged
merged 18 commits into from
May 12, 2018
Merged

compile on stable #69

merged 18 commits into from
May 12, 2018

Conversation

japaric
Copy link
Member

@japaric japaric commented Apr 24, 2018

with these changes this crate compiles on stable

@japaric
Copy link
Member Author

japaric commented Apr 24, 2018

It also appears to fix all the LLD issues reported in #53. cc @jcsoo

src/lib.rs Outdated
#[doc(hidden)]
pub enum Exception {
NMI,
MenManage,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be MemManage?

@jcsoo
Copy link
Contributor

jcsoo commented Apr 24, 2018

Tried it out and it works for me.

@japaric
Copy link
Member Author

japaric commented Apr 26, 2018

Changes from cortex-m-rt v0.4.x and stable-embedded-rust that warrant some discussion:

  • The user must define both DefaultHandler and HardFault.

  • The HardFault handler is now a diverging function (fn(..) -> !). The rationale is that
    returning from the hard fault handler can be (is usually?) UB. A diverging function prevents that
    scenario.

  • Now, only the HardFault handler has access to the previous stack frame. In cortex-m-rt
    DefaultHandler had access to the previous stack frame.

  • DefaultHandler now takes a u8 argument. This integer is the exception number and can be used
    to identify which exception / interrupt is being serviced.

  • interrupts! can be used to bind all the potential interrupt handlers (240) to DefaultHandler.
    This can be used to write device agnostic programs. In cortex-m-rt one had to explicitly declare
    a static variable and place it in the .vector_table.interrupts section; interrupts! is (less
    error prone) sugar for that.

Unresolved questions:

  • Do we want to apply the divergent function trick to the other fault handlers; that is to
    UsageFault, BusFault and MemManage (MPU fault?)? Is it safe (not UB) to return from those
    fault handlers? An idea here could be to make it safe to use a divergent function but unsafe to
    use a non divergent function. See below:
// You can pick
exception!(BusFault, bf);

fn bf() -> ! {
    // ..
}

// OR this
exception!(unsafe BusFault, bf);

fn bf()  {
    // ..
    return;
}
  • Do we want one default handler for Cortex-M exceptions and a different default handler for
    interrupts? That is DefaultExceptionHandler and DefaultInterruptHandler. (I don't see much
    gain from such split)

Bikeshedding:

  • should main! be renamed to entry!? The macro defines the user program entry point, which doesn't need to be named main.

TODO:

  • Add back exception / interrupt local data (locals in the old exception! macro)

  • Document the overridable symbols (_foo) and the private symbols (__foo). The later will
    be documented for inspection / diagnostic purposes (objdump, nm, etc.).

  • Document that exception! and interrupts! must be called in the root of the crate, and if possible add (reachability) checks for that.

cc @thejpster

@japaric
Copy link
Member Author

japaric commented Apr 26, 2018

Bikeshedding: should main! be renamed to entry!? The macro defines the user program entry point, which doesn't need to be named main.

@japaric
Copy link
Member Author

japaric commented Apr 26, 2018

Future improvements:

Macros 1.2 is planned for the 1.28 release and will allow custom attributes (i.e. #[foo]; today we only allow custom derive on stable: #[derive(Foo)]), so we could in the future replace the exception! macro for an #[exception] attribute which would slightly reduce boilerplate / look more natural:

// Today
#[macro_use(exception)]
extern crate cortex_m_rt;

exception!(SysTick, sys_tick);

fn sys_tick() { .. }

// Macros 1.2
extern crate cortex_m_rt;

use cortex_m_rt::exception;

#[exception(SysTick)]
fn sys_tick() { .. }

// or even
#[exception]
fn SysTick() { .. }

We could do the same with main! macro; we could replace it with an #[entry] attribute.

@jcsoo
Copy link
Contributor

jcsoo commented Apr 26, 2018

Do we want one default handler for Cortex-M exceptions and a different default handler for
interrupts? That is DefaultExceptionHandler and DefaultInterruptHandler. (I don't see much
gain from such split)

In my use case, I want all interrupts to optionally go to a shared handler, but I'd prefer to have individual exception handlers as well as a DefaultHandler for everything else.

Is it possible to have the interrupts! macro bind to a specific handler rather than DefaultHandler?

@japaric
Copy link
Member Author

japaric commented Apr 26, 2018

Is it possible to have the interrupts! macro bind to a specific handler rather than DefaultHandler?

It could but most applications are not going to use the interrupts! macro; they are going to use the __INTERRUPTS variable provided by a device crate (generated using svd2rust) or an equivalent to it.

So, I guess my question actually is: should svd2rust bind the device specific interrupts to DefaultHandler or to some other handler (DefaultInterruptHandler)? Though if we go down that route we'll probably want to add EXTERN(DefaultInterruptHandler) to link.x to prevent the linker from dropping the symbol.

If you are using something other than svd2rust you can do whatever you want; you don't need to use interrupts! (svd2rust doesn't use interrupts! either)

…` ..

- remove some variants from `Exception` when targeting ARMv6-M

- future proof the ABI of `__EXCEPTIONS` by using a `union` instead of `Option<fn()>` (the `None`
  variant is not guaranteed to always be represented as the value `0`)

- tweak exception names to match CMSIS and the names used in cortex-m v0.5.0

- add an opt-in "device" feature to opt into a device specific build (interrupts must be supplied by
a different crate, or manually by the user).

- drop the `interrupts!` macro. If you don't enable the "device" feature `cortex-m-rt` will bind all
possible interrupts to the default exception handler.
- document the `main` symbol as an alternative to `entry!`
- document `ResetTrampoline`
- fix: `PendSV` is available on ARMv6-M
- document that `entry!` and `exception!` must be called from accessible modules.
- add a deny lint to `entry!` and `exception!` to prevent them from being invoked from inaccessible
  modules.
@japaric
Copy link
Member Author

japaric commented May 11, 2018

bors try

bors bot added a commit that referenced this pull request May 11, 2018
@bors
Copy link
Contributor

bors bot commented May 11, 2018

try

Build succeeded

@japaric japaric changed the title [WIP] compile on stable compile on stable May 12, 2018
@japaric
Copy link
Member Author

japaric commented May 12, 2018

bors r+

bors bot added a commit that referenced this pull request May 12, 2018
69: compile on stable r=japaric a=japaric

with these changes this crate compiles on stable

Co-authored-by: Jorge Aparicio <[email protected]>
@bors
Copy link
Contributor

bors bot commented May 12, 2018

Build succeeded

@bors bors bot merged commit 2f410f2 into master May 12, 2018
@japaric japaric deleted the stable branch May 12, 2018 16:06
bors bot added a commit to rust-embedded/cortex-m-quickstart that referenced this pull request May 12, 2018
29: use less unstable dependencies r=japaric a=japaric

This PR and the ones at the bottom reduce the number of unstable features needed for Cortex-M development to a single one: `lang = "panic_fmt"`, which already has a path towards stabilization and which we hope to get on stable by 1.28.

[Check out the temporary documentation](https://japaric.github.io/cortex-m-quickstart/cortex_m_quickstart/index.html) (we still need more docs) to try out this preview. 

We would love your input on [these unresolved questions](rust-embedded/cortex-m-rt#69 (comment))

This PR depends on:

- rust-embedded/cortex-m-rt#69
- rust-embedded/cortex-m#88
- rust-embedded/panic-semihosting#2
- rust-embedded/svd2rust#203
- japaric/stm32f103xx#24

Co-authored-by: Jorge Aparicio <[email protected]>
danc86 added a commit to danc86/riscv-rt that referenced this pull request Aug 10, 2018
dvc94ch pushed a commit to rust-embedded/riscv-rt that referenced this pull request Aug 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants