diff options
author | 2022-01-04 16:42:26 +0000 | |
---|---|---|
committer | 2022-01-04 16:42:26 +0000 | |
commit | e0b93a022d3288e57f4743bf0d548baf78d01b02 (patch) | |
tree | 0652ce14dd57cb9918e98b4f8aa0977bb1be8d1c | |
parent | 50c7fd34901eb9a2a9aec79c4c5a5574140046ba (diff) | |
parent | 632af2e78241416420b8004820d6f386cfabeae8 (diff) | |
download | cortex-m-e0b93a022d3288e57f4743bf0d548baf78d01b02.tar.gz cortex-m-e0b93a022d3288e57f4743bf0d548baf78d01b02.tar.zst cortex-m-e0b93a022d3288e57f4743bf0d548baf78d01b02.zip |
Merge #380
380: Improve singleton!() macro r=adamgreig a=Rahix
This PR addresses two shortcomings of the `cortex_m::singleton!()` macro, which I raised in #364. For review, I think it is best to look at the two commits implementing these changes individually.
I think this changeset should also be backported to `0.7.x` where it applies cleanly and which is also the place where I tested it.
### 1. Fix `singleton!()` sometimes ending up in `.data`
The static is always initialized to a "zero" value with `Option::None` which means it should end up in `.bss`. However, if the enclosed type has a niche, `Option::None` can become a non-zero bitpattern which moves the whole singleton from `.bss` to `.data`. This is especially problematic when storing large buffers in the `singleton!()` as this starts eating lots of flash space unnecessarily.
To prevent this, I switched to using an explicit boolean flag instead. This is not quite as nice but at least there is no chance for the `singleton!()` to end up in `.data`...
For reference and as an example, the faulty behavior can be triggered with
```rust
cortex_m::singleton!(: Option<u32> = None)
```
(the inner option has a non-zero niche which the outer option will occupy)
### 2. Allow naming the `static`
Due to the static always being named `VAR` right now, all `singleton!()` instances end up having non-descriptive symbol names like `__cortex_m_rt_main::{{closure}}::VAR` which makes them hard to tell apart in a debugger or when looking at an objdump.
I added the ability to set an explicit name which end up becoming part of the symbol name. This does not affect Rust code at all - the new name is not visible anywhere. It works like this:
```rust
let value = singleton!(FOO_BUFFER: [u8; 1024] = [0u8; 1024]);
```
Of course the old syntax also still works and keeps the old behavior of calling the static `VAR`.
Fixes #364.
Co-authored-by: Rahix <rahix@rahix.de>
-rw-r--r-- | CHANGELOG.md | 4 | ||||
-rw-r--r-- | src/macros.rs | 32 |
2 files changed, 26 insertions, 10 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index c1e61b6..04bf9d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - ITM: add `configure` API (#342). - TPIU: add API for *Formatter and Flush Control* (FFCR) and *Selected Pin Control* (SPPR) registers (#342). - Add `std` and `serde` crate features for improved host-side ITM decode functionality when working with the downstream `itm`, `cargo-rtic-scope` crates (#363, #366). +- Added the ability to name the statics generated by `singleton!()` for better debuggability (#364, #380). + +### Fixed +- Fixed `singleton!()` statics sometimes ending up in `.data` instead of `.bss` (#364, #380). ## [v0.7.4] - 2021-12-31 diff --git a/src/macros.rs b/src/macros.rs index 66b75b1..512c932 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -30,9 +30,12 @@ macro_rules! iprintln { /// `None` variant the caller must ensure that the macro is called from a function that's executed /// at most once in the whole lifetime of the program. /// -/// # Note +/// # Notes /// This macro is unsound on multi core systems. /// +/// For debuggability, you can set an explicit name for a singleton. This name only shows up the +/// the debugger and is not referencable from other code. See example below. +/// /// # Example /// /// ``` no_run @@ -50,15 +53,24 @@ macro_rules! iprintln { /// fn alias() -> &'static mut bool { /// singleton!(: bool = false).unwrap() /// } +/// +/// fn singleton_with_name() { +/// // A name only for debugging purposes +/// singleton!(FOO_BUFFER: [u8; 1024] = [0u8; 1024]); +/// } /// ``` #[macro_export] macro_rules! singleton { - (: $ty:ty = $expr:expr) => { + ($name:ident: $ty:ty = $expr:expr) => { $crate::interrupt::free(|_| { - static mut VAR: Option<$ty> = None; + // this is a tuple of a MaybeUninit and a bool because using an Option here is + // problematic: Due to niche-optimization, an Option could end up producing a non-zero + // initializer value which would move the entire static from `.bss` into `.data`... + static mut $name: (::core::mem::MaybeUninit<$ty>, bool) = + (::core::mem::MaybeUninit::uninit(), false); #[allow(unsafe_code)] - let used = unsafe { VAR.is_some() }; + let used = unsafe { $name.1 }; if used { None } else { @@ -66,16 +78,16 @@ macro_rules! singleton { #[allow(unsafe_code)] unsafe { - VAR = Some(expr) - } - - #[allow(unsafe_code)] - unsafe { - VAR.as_mut() + $name.1 = true; + $name.0 = ::core::mem::MaybeUninit::new(expr); + Some(&mut *$name.0.as_mut_ptr()) } } }) }; + (: $ty:ty = $expr:expr) => { + $crate::singleton!(VAR: $ty = $expr) + }; } /// ``` compile_fail |