Allow initialization of resources in `init`. #43

jonas-schievink · 2017-09-03T16:19:30Z

Closes #37

TODO:

Open dependent PRs
- Get them merged
Add an example ~~(I cannot currently run any of the examples since I "only" have an stm32f401)~~
Add a cfail test ensuring that init cannot access uninitialized resources (and possibly other tests)
- Figure out why I cannot run these tests
Simplify code, it's quite ugly

perlindgren · 2017-09-03T21:49:37Z

Hi Jonas We are running on stm32f401re (uncleos), what exactly is the problem? (I mean the examples should be possible run with quite small modifications…) Best Per On 03 Sep 2017, at 18:19, Jonas Schievink <notifications@github.com<mailto:notifications@github.com>> wrote: Closes #37<https://github.com/japaric/cortex-m-rtfm/issues/37> TODO: * Open dependent PRs and get them merged * Add an example (I cannot currently run any of the examples since I "only" have an stm32f401) * Add a cfail test ensuring that init cannot access uninitialized resources (and possibly other tests) * Figure out why I also cannot run these tests * Simplify code, it's quite ugly

…

jonas-schievink · 2017-09-03T22:15:23Z

I'm hitting #42, but managed to work around that by compiling in release mode (all examples compile, haven't run any yet).

However, I still can't run the cfail tests on the host (x86-64 Linux) because there is still ARM inline assembly being used by stm32f103xx:

error: <inline asm>:2:17: error: unknown directive
                .thumb_func
                ^


error: <inline asm>:4:21: error: invalid instruction mnemonic 'b'
                    b DEFAULT_HANDLER
                    ^

Travis seems to run these tests, too, so I don't understand why that works.

japaric

Thanks for working on this, @jonas-schievink.

The implementation looks good to me but I left some nit comments. I agree that having a cfail test for this is a good idea.

japaric · 2017-09-21T21:34:36Z

macros/src/trans.rs

    let mut mod_items = vec![];

-    if !app.resources.is_empty() {
+    // Write resources usable by `init`, if any


stray comment?

japaric · 2017-09-21T21:35:55Z

macros/src/trans.rs

+
+
+    // Are there any resources that have an initializer? Those can be used by `init`.
+    let has_initialized_resources = app.resources.iter()


I think this can be expressed as app.resource.keys().any(|res| res.expr.is_some()).

japaric · 2017-09-21T21:40:31Z

macros/src/trans.rs

        let mut fields = vec![];
        let mut lifetime = None;
        let mut rexprs = vec![];

-        for (name, resource) in &app.resources {
+        for (name, resource) in app.resources.iter()
+                .filter(|&(_, res)| res.expr.is_some()) {


hmm, I wonder if this would be more "correct" as app.resources.iter().filter_map(|(name, res)| res.map(|res| (name, rers))). It does look more complicated though. Basically what I'm trying to do is avoid having resource be an "Option" that it's always Some. In any case, we won't unwrap the Option in this branch so maybe I'm just being pendantic ...

japaric · 2017-09-21T21:41:08Z

macros/src/trans.rs

@@ -204,6 +213,46 @@ fn init(app: &App, main: &mut Vec<Tokens>, root: &mut Vec<Tokens>) {
        exprs.push(quote!(init::Resources::new()));
    }

+    let mut late_resources = vec![];
+    let has_late_resources = app.resources.iter()


Same as above: this could use any

Actually it just occurred to me that this could be done in a single pass over the resources by collecting the "early" resources and the "late" resources in vectors and then doing the appropriate codegen depending on whether the vector is empty or not.

Not sure which one would be easier to read.

japaric · 2017-09-21T21:45:52Z

macros/src/trans.rs

+            });
+
+            late_resources.push(quote! {
+                #_name = #krate::UntaggedOption { some: _late_resources.#name };


nit: I think UntaggedOption::some(..) would be shorter. Would that require the const_fn feature in user code though? Maybe it's already required by other generated code ...

Actually, #_name.some = _late_resources.#name is even shorter. I'm not 100% if that has the exact semantics wrt to destructors. I think they are never run for any assignment.

Would that require the const_fn feature in user code though?

Yes. That's the reason why I made those fields public in the first place.

Actually, #_name.some = _late_resources.#name is even shorter.

Yeah, I'll do this

I'm not 100% if that has the exact semantics wrt to destructors. I think they are never run for any assignment.

Running them would be unsafe (how does the drop glue know which variant was active before the assignment?), so it can't really happen automatically (dropping or assignment is always safe). At least that's how I understand it.

japaric · 2017-09-21T21:46:19Z

macros/src/trans.rs

+                },
+                None => quote! {
+                    // Resource initialized in `init`
+                    static mut #_name: #krate::UntaggedOption<#ty> = #krate::UntaggedOption { none: () };


nit: as above this would be shorter as UntaggedOption::none()

japaric · 2017-09-21T21:47:43Z

src/lib.rs


 use core::u8;

 pub use rtfm_core::{Resource, Static, Threshold};
 pub use cortex_m::asm::{bkpt, wfi};
 pub use cortex_m_rtfm_macros::app;
+pub use untagged_option::UntaggedOption;


This re-export should be #[doc(hidden)] as it is a not user facing implementation detail.

Fixes compilation on x86-64

The extended example tests that this actually works this time.

jonas-schievink · 2017-09-22T16:22:23Z

Got rid of all iterators except a single call to partition.

japaric · 2017-09-22T16:40:54Z

I had forgotten about the partition method. Nice.

@homunkulus r+

homunkulus · 2017-09-22T16:40:54Z

📌 Commit be1a27c has been approved by japaric

homunkulus · 2017-09-22T16:40:57Z

⌛ Testing commit be1a27c with merge be1a27c...

jonas-schievink · 2017-09-22T17:29:08Z

This needs a new release of rtfm-syntax before landing. I've redirected the dependency to my fork to work on this, the branch is now deleted. The other failures look like something else caught fire...

homunkulus · 2017-09-22T17:32:34Z

💔 Test failed - status-travis

japaric · 2017-09-22T17:33:43Z

The other failures look like something else caught fire...

Xargo v0.3.9 can't compile compiler-builtins. Could you pin the version to v0.3.8? That version works fine.

japaric · 2017-09-22T17:37:01Z

rtfm-syntax v0.2.0 has been released

japaric · 2017-09-22T18:35:27Z

@homunkulus r+

homunkulus · 2017-09-22T18:35:27Z

📌 Commit df85298 has been approved by japaric

homunkulus · 2017-09-22T18:35:30Z

⌛ Testing commit df85298 with merge df85298...

homunkulus · 2017-09-22T18:40:54Z

💔 Test failed - status-travis

japaric · 2017-09-22T18:57:51Z

The thumbv6m build failure is due to issue #42, which has already been fixed. We are seeing the issue here because we are cross compiling the stm32f103xx crate, which is a Cortex-M3 device crate, for thumbv6m. I guess we should use a Cortex-M0 device crate for the example when cross compiling to thumbv6m.

I thought that xargo check --examples may work around the issue but it still does the linking step and errors.

As a horrible hack we could set the thumbv6m linker to the true command then linking will always work ...

Horrible hack until we switch to a Cortex-M0 device crate that works with armv6.

jonas-schievink · 2017-09-22T20:45:11Z

I changed the linker to true. I'll open an issue to switch to some Cortex-M0 device crate when this lands.

japaric · 2017-09-22T22:59:47Z

@homunkulus r+

homunkulus · 2017-09-22T22:59:47Z

📌 Commit a190da3 has been approved by japaric

homunkulus · 2017-09-22T22:59:50Z

⌛ Testing commit a190da3 with merge a190da3...

homunkulus · 2017-09-22T23:03:48Z

☀️ Test successful - status-travis
Approved by: japaric
Pushing a190da3 to master...

43: Add AfoHT to devs in README r=korken89 a=AfoHT Co-authored-by: Henrik Tjäder <henrik@tjaders.com>

Zero cost stack overflow protection, take 2

this commit adds LLD support by removing all INFO sections. LLD kind of supports INFO in the form of NOLOAD but when the linker script contains NOLOAD sections LLD emits a binary with false `size` information: for example, it reported a fake increase of 20KB in .text and a fake increase of 1KB in .bss when compiling a program that only allocates a single Box. As the INFO sections are gone we can no longer support the stack overflow protection added in rtic-rs#43 so all the other related changes, like making _stack_start overridable, have been removed as well. If you want to continue using stack overflow protection you can stick to v0.3.x. As the .debug_gdb_scripts output section has been removed from the linker script these changes will only reliably support both LD and LLD if/when rust-lang/rust#49728 lands. closes rtic-rs#53

Allow initialization of resources in init.

7ebba49

This was referenced Sep 3, 2017

Make the resource initializer optional rtic-rs/rtic-syntax#1

Merged

Add LateResource union rtic-rs/rtic-core#1

Closed

jonas-schievink added 2 commits September 4, 2017 23:31

Add late resources example

c8bdb73

Use untagged_option crate

57ed096

jonas-schievink mentioned this pull request Sep 6, 2017

[WIP] travis: disable cache #44

Closed

japaric reviewed Sep 21, 2017

View reviewed changes

jonas-schievink added 7 commits September 22, 2017 13:44

Bump stm32f103xx version

ee59e87

Fixes compilation on x86-64

Hide UntaggedOption reexport

bbc8736

Don't use deprecated method to create compiletest config

21d1f6b

Fix warning in wrong-threshold cfail test

b1777eb

Simplify iterator usage

70e2436

Fix shared resource handling and extend example.

1556948

The extended example tests that this actually works this time.

Add cfail test for late resources

be1a27c

jonas-schievink changed the title ~~[WIP] Allow initialization of resources in init.~~ Allow initialization of resources in init. Sep 22, 2017

jonas-schievink added 2 commits September 22, 2017 20:11

Use rtfm-syntax 0.2.0

b50c909

Pin Travis to Xargo 0.3.8

df85298

Use true as the armv6 linker

a190da3

Horrible hack until we switch to a Cortex-M0 device crate that works with armv6.

homunkulus merged commit a190da3 into rtic-rs:master Sep 22, 2017

jonas-schievink deleted the init-resources branch September 23, 2017 14:49

jonas-schievink mentioned this pull request Sep 23, 2017

Switch to some Cortex-M0 device crate #45

Closed

japaric mentioned this pull request Sep 27, 2017

[RFC] "rooting" memory (&'static mut- references in init) #47

Closed

japaric mentioned this pull request Oct 7, 2017

Do the current semantics for referencing registers interfere with composability? rust-embedded/svd2rust#151

Closed

japaric pushed a commit that referenced this pull request Sep 10, 2021

Merge #43

edada7c

43: Add AfoHT to devs in README r=korken89 a=AfoHT Co-authored-by: Henrik Tjäder <henrik@tjaders.com>

andrewgazelka pushed a commit to andrewgazelka/cortex-m-rtic that referenced this pull request Nov 3, 2021

Merge pull request rtic-rs#43 from japaric/swap-ld

eb9970f

Zero cost stack overflow protection, take 2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow initialization of resources in `init`. #43

Allow initialization of resources in `init`. #43

jonas-schievink commented Sep 3, 2017 •

edited

Loading

perlindgren commented Sep 3, 2017 via email

jonas-schievink commented Sep 3, 2017 •

edited

Loading

japaric left a comment

japaric Sep 21, 2017

japaric Sep 21, 2017

japaric Sep 21, 2017

japaric Sep 21, 2017

japaric Sep 21, 2017

japaric Sep 21, 2017

japaric Sep 21, 2017

jonas-schievink Sep 21, 2017

japaric Sep 21, 2017

japaric Sep 21, 2017

jonas-schievink commented Sep 22, 2017

japaric commented Sep 22, 2017

homunkulus commented Sep 22, 2017

homunkulus commented Sep 22, 2017

jonas-schievink commented Sep 22, 2017

homunkulus commented Sep 22, 2017

japaric commented Sep 22, 2017

japaric commented Sep 22, 2017

japaric commented Sep 22, 2017

homunkulus commented Sep 22, 2017

homunkulus commented Sep 22, 2017

homunkulus commented Sep 22, 2017

japaric commented Sep 22, 2017

jonas-schievink commented Sep 22, 2017

japaric commented Sep 22, 2017

homunkulus commented Sep 22, 2017

homunkulus commented Sep 22, 2017

homunkulus commented Sep 22, 2017



		// Are there any resources that have an initializer? Those can be used by `init`.
		let has_initialized_resources = app.resources.iter()

Allow initialization of resources in init. #43

Allow initialization of resources in init. #43

Conversation

jonas-schievink commented Sep 3, 2017 • edited Loading

perlindgren commented Sep 3, 2017 via email

jonas-schievink commented Sep 3, 2017 • edited Loading

japaric left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonas-schievink commented Sep 22, 2017

japaric commented Sep 22, 2017

homunkulus commented Sep 22, 2017

homunkulus commented Sep 22, 2017

jonas-schievink commented Sep 22, 2017

homunkulus commented Sep 22, 2017

japaric commented Sep 22, 2017

japaric commented Sep 22, 2017

japaric commented Sep 22, 2017

homunkulus commented Sep 22, 2017

homunkulus commented Sep 22, 2017

homunkulus commented Sep 22, 2017

japaric commented Sep 22, 2017

jonas-schievink commented Sep 22, 2017

japaric commented Sep 22, 2017

homunkulus commented Sep 22, 2017

homunkulus commented Sep 22, 2017

homunkulus commented Sep 22, 2017

Allow initialization of resources in `init`. #43

Allow initialization of resources in `init`. #43

jonas-schievink commented Sep 3, 2017 •

edited

Loading

jonas-schievink commented Sep 3, 2017 •

edited

Loading