-
Notifications
You must be signed in to change notification settings - Fork 214
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rtfm based stm32f030 debug build causes relcation truncated error #42
Comments
I have seen this before though haven't nailed down the exact cause. From what I have seen:
Out of curiosity, what happens when you compile without cc @therealprof, who may know more about this problem |
Hm, I thought I opened a bug report for this before or at least mentioned it somewhere... The problem is not debug related, I can easily reproduce it with I've no idea how (and where) this could be addressed but monomorphisation, heavy inlining, and LTO (as well as lack of optimisation in debug builds) are the source of the issue here because they all lead to few but huge functions. |
Looking into the instruction set it seems that Cortex-M should happily support the larger version of the branch as well. Maybe it would suffice to tell that to the linker somehow... |
@therealprof a bit disheartening that you get this problem with @japaric error: linking with BTW, if you want me to move this over to cortex-m-rt I can do that.. I suspected that I might be posting to the wrong specific location in the correct ecosystem.. |
@japaric I figured out what the problem is... Seemingly the default CPU model for the armv6-m architecture is broken. I tried various options including using the -mcpu=cortex-m0 and -mcpu=cortex-m3 options on the generated assembly and the latter automatically changes branches where the target doesn't fit into the available 2 bytes into the 4 bytes form of the branch while the default model and -mcpu=cortex-m0 (which actually might be the default model) keeps it as-is causing the linker to barf. However, if I explicitly change the short branch in the code emitted by
@x37v Can you with the above change on your code? |
@therealprof yes, that did solve my problem! THANKS SO MUCH! |
This can't be right, using branch with link will clobber the link register, so the interrupt handlers will fail to return. To make it work with Also, the documentation here says that |
@pftbest Whoops, you're right. I picked the wrong mnemonic, b.w is what I wanted to say. And you're also right that it can't be used due to the compiler/assembler complaining. :( |
@pftbest Okay, I checked around and there's nothing that would the assembly accept that function, BUT: why do we do that manual jump in the first place? DH_TRAMPOLINE doesn't do anything useful so we might as well just get rid of that...
That removes one unnecessary indirection from code that actually does compile and fixes this particular problem since we're jumping to the correct function right from our exception/interrupt table where we don't have any address limitations... It'll likely not fix the problem I had before wrt. functions becoming too big to be jumped to (which I've addressed in the code) and should be addressed by the compiler, but this seems like a win-win to. NB: I have no hardware here so I can't very it but it sure looks good to me. |
No, we can't remove this trampoline, because it will silently break non-lto builds. Weak references can only point to symbols defined in the same object file, but default handler is defined in another crate, so it will end up in different object file. This bug was reported here: https://github.com/japaric/cortex-m-rtfm/issues/39 |
I think the only working solution here is to make It may affect a stack trace when debugging, not sure if it counts as a breaking change. I don't have a board atm so I can't test it. |
Hm, non-lto builds... Those still exist? ;) I'll have to look a bit closer at this in a non-lto context. I'm still not exactly sure why the trampoline needs to exist at all, my preference would be to fix the visibility of the symbols. As I said before this will most likely not fix the compiler issue at hand (refusing wo accept the b.w for armv6m) so it's very likely that we will run into the same problem sooner or later again... Not sure how to properly report this though. |
interesting, @pftbest, |
@therealprof and @pftbest I could try to get an stm32f0 based discovery board to you if you want some hardware to test on.. |
@pftbest is right that |
@x37v, the issue here is that processor relies on This does not break the provided default_handler, since it goes into infinite loop and never returns, but the user may override it using |
@pftbest I'm still a bit confused... my understanding is that interrupt handlers get executed after an interrupt arrives and execution jumps out of your main loop [in the rtfm case a loop waiting for interrupts] execute some code and then jump back. Are you saying that the default handler, before being overridden, normally goes into an infinite loop and never returns to the main loop?.. or is this simply an effect of the |
@x37v The default handler is only used if the system fires an exception or an interrupt and you haven't provided your own exception or interrupt handler. You can override the default_handler, too if you want to do anything specific in this case however the default implementation is more or less the only sane implementation one can have in this situation: Set a breakpoint and do nothing more. |
@therealprof AHH, that makes sense. So, beyond the potential override, is it problematic as is, with no way to return? |
@x37v At that point the MCU is pretty much in a dead end, so other than saying goodbye I don't think there's much you can do to re-enter the program in orderly fashion other than a reset... Even if you have the link register; who say's it points to a place where you can actually reenter? |
@therealprof, why is MCU in a dead end? Nothing serious would happen if we just return from some unhandled GPIO interrupt. |
@pftbest Why would you enable an interrupt you're not willing to handle? And if your willing to handle it, why not have a specific handler for that? Using the default handler has a number of drawbacks; sure with enough effort you might be able to figure out why ended in there but all the exceptions you're not willing to deal with also end up in there, i.e. the really bad stuff from which a useful recovery is typically not possible. There's a reason that in 99.99% of all cases the default handler is used to
or any combination thereof. |
@pftbest You're right. The easiest way to make that work seems to be a proper Rust function; I tried all kinds of tricks with assembly but the simplest solution is the obvious one:
The binary code grows by 4 bytes. It also adds the additional benefit of properly naming the function, but here's the kicker; it also uses the
🤔 |
Hi
A comment +/- related.
A sensible thing to do in the default handler is to unwind the stack, that makes it easier to trace the cause of the error. (I made some tests earlier and it works…)
The question is just what to do with the “trace info”. I sent it over the ITM, but as there is no flow control for the ITM, the debugger buffer (on the nucleo/st-link) may overflow, so you may loose characters. One can also inspect the stack directly from gdb, perhaps there is some scripting possible to facilitate this (Japaric might know…)
Best,
Per
On 05 Sep 2017, at 00:58, therealprof <notifications@github.com<mailto:notifications@github.com>> wrote:
@pftbest<https://github.com/pftbest> You're right. The easiest way to make that work seems to be a proper Rust function; I tried all kinds of tricks with assembly but the simplest solution is the obvious one:
extern "C" {
fn DEFAULT_HANDLER();
}
#[allow(non_snake_case)]
#[naked]
#[no_mangle]
pub unsafe fn DH_TRAMPOLINE() {
DEFAULT_HANDLER();
}
The binary code grows by 4 bytes.
It also adds the additional benefit of properly naming the function, but here's the kicker; it also uses the bl instruction:
│ -08000480 <ADC_COMP>:
│ +08000480 <DH_TRAMPOLINE>:
│ - 8000480: e059 b.n 8000536 <BUS_FAULT>
│ + 8000480: f000 f85b bl 800053a <BUS_FAULT>
│ + 8000484: 4770 bx lr
🤔
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub<https://github.com/japaric/cortex-m-rtfm/issues/42#issuecomment-327036730>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AD5naDUQ9VcMi_3hjpGJvYp8OhziHoYjks5sfICigaJpZM4PIR-X>.
{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/japaric/cortex-m-rtfm","title":"japaric/cortex-m-rtfm","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/japaric/cortex-m-rtfm"}},"updates":{"snippets":[{"icon":"PERSON","message":"@therealprof in #42: @pftbest You're right. The easiest way to make that work seems to be a proper Rust function; I tried all kinds of tricks with assembly but the simplest solution is the obvious one:\r\n```\r\n extern \"C\" {\r\n fn DEFAULT_HANDLER();\r\n }\r\n\r\n #[allow(non_snake_case)]\r\n #[naked]\r\n #[no_mangle]\r\n pub unsafe fn DH_TRAMPOLINE() {\r\n DEFAULT_HANDLER();\r\n }\r\n```\r\n\r\nThe binary code grows by 4 bytes.\r\n\r\nIt also adds the additional benefit of properly naming the function, but here's the kicker; it also uses the `bl` instruction:\r\n```\r\n│ -08000480 \u003cADC_COMP\u003e:\r\n│ +08000480 \u003cDH_TRAMPOLINE\u003e:\r\n│ - 8000480: e059 b.n 8000536 \u003cBUS_FAULT\u003e\r\n│ + 8000480: f000 f85b bl 800053a \u003cBUS_FAULT\u003e\r\n│ + 8000484: 4770 bx lr\r\n```\r\n\r\n🤔"}],"action":{"name":"View Issue","url":"https://github.com/japaric/cortex-m-rtfm/issues/42#issuecomment-327036730"}}}
|
@therealprof I think you forgot to remove the Maybe we can do better, by having 2 functions behind a |
@pftbest Hm, right again... this is becoming uncanny. ;) However now we have the same function twice with two different labels:
Duh, well.
There's no reason for that. Both actually support the very same |
I believe LLVM is correct in this case, quote from the docs:
Yes, DEFAULT_HANDLER gets inlined, that is unfortunate, but it may be fixed by this patches when they will be merged. |
Hm, I can't find the reference at the moment but some site said that |
Sorry for jumping in the discussion! Actually the link @pftbest posted earlier clears this up perfecrly: See Table 12. |
@Samonitari Right, however this wouldn't be the first time that the official documentation turns out to be incorrect. 😉 Really the only difference it makes is whether to report a bug to LLVM or not. |
@Samonitari Yes, thumb1 has |
@pftbest's idea, namely:
Sounds good to me. I'd be happy to merge a PR implementing that. |
I'm already working on that :) |
This turned out to be pretty complicated as the mentioned #[cfg] would have to be put into every crate generated by svd2rust, so they all need a build.rs setting some |
I guess this is still fine, you just have to opt-in to get armv6 support... |
Note that the issue isn't truly fixed until the stm32f030 crate is regenerated with an up-to-date svd2rust. |
I figure this is worth a patch version update? |
the crate: example project: Builds and debugs in dev! 👍 Thanks All! |
Fixes japaric/cortex-m-rtfm#42 Note that this will make all generated crates that target an armv6 device fail to compile unless they add a build script enabling the added `cfg`.
Just stumbled onto this so I might of missed something, but about about just always using
then you have unlimited range. |
@parched but you will loose the value in r0 register. and you may want to know the value for debugging purposes. |
This implements the "rooting" mechanism proposed in #47. However, it implements a `root` constructor function instead of list of `roots` values as originally proposed. In a nutshell: - There's a new field, `root`, which takes a path to the "root" constructor function. - This constructor has signature `fn() -> T` - When the `root` field is used the signature of `init` changes to accommodate a `&'static mut T` argument at the end. The `T` in that argument type matches the type returned by the "root" constructor. - The "root"-ed value is stack allocated. This enables the safe creation of `&'static mut` references. Example below: ``` rust //#![feature(proc_macro)] //#![no_std] extern crate blue_pill; extern crate cortex_m_rt; extern crate cortex_m_rtfm as rtfm; extern crate heapless; use blue_pill::stm32f103xx; use heapless::RingBuffer; use heapless::ring_buffer::{Consumer, Producer}; use rtfm::{app, Threshold}; use stm32f103xx::Interrupt; app! { device: stm32f103xx, resources: { static CONSUMER: Consumer<'static, u32, [u32; 8]>; static PRODUCER: Producer<'static, u32, [u32; 8]>; }, root: root, idle: { resources: [CONSUMER], }, tasks: { EXTI0: { path: exti0, resources: [PRODUCER], }, } } struct Root { rb: RingBuffer<u32, [u32; 8]>, } fn root() -> Root { Root { rb: RingBuffer::new(), } } fn init(_p: init::Peripherals, root: &'static mut Root) -> init::LateResourceValues { let (p, c) = root.rb.split(); init::LateResourceValues { CONSUMER: c, PRODUCER: p, } } fn idle(_t: &mut Threshold, r: idle::Resources) -> ! { rtfm::set_pending(Interrupt::EXTI0); loop { if r.CONSUMER.dequeue().is_some() { rtfm::bkpt(); } else { rtfm::wfi(); } } } fn exti0(_t: &mut Threshold, r: EXTI0::Resources) { r.PRODUCER.enqueue(42).ok(); rtfm::bkpt(); } ``` This produces the following machine code: ``` armasm 0800019c <EXTI0>: 800019c: f240 0000 movw r0, #0 80001a0: f2c2 0000 movt r0, #8192 ; 0x2000 80001a4: 6800 ldr r0, [r0, #0] 80001a6: 6803 ldr r3, [r0, #0] 80001a8: 6842 ldr r2, [r0, #4] 80001aa: 1c51 adds r1, r2, #1 80001ac: f001 0107 and.w r1, r1, #7 80001b0: 4299 cmp r1, r3 80001b2: d006 beq.n 80001c2 <EXTI0+0x26> 80001b4: eb00 0282 add.w r2, r0, r2, lsl #2 80001b8: 232a movs r3, #42 ; 0x2a 80001ba: 6093 str r3, [r2, #8] 80001bc: f3bf 8f5f dmb sy 80001c0: 6041 str r1, [r0, #4] 80001c2: be00 bkpt 0x0000 80001c4: 4770 bx lr 080001c6 <main>: 80001c6: b08a sub sp, #40 ; 0x28 ; Root allocation 80001c8: f240 1030 movw r0, #304 ; 0x130 80001cc: 4669 mov r1, sp 80001ce: 22f0 movs r2, #240 ; 0xf0 80001d0: f6c0 0000 movt r0, #2048 ; 0x800 80001d4: 7800 ldrb r0, [r0, #0] 80001d6: 2000 movs r0, #0 80001d8: e9cd 0000 strd r0, r0, [sp] 80001dc: f240 0000 movw r0, #0 80001e0: f2c2 0000 movt r0, #8192 ; 0x2000 80001e4: b672 cpsid i 80001e6: 6001 str r1, [r0, #0] ; PRODUCER = .. 80001e8: f240 0004 movw r0, #4 80001ec: f2c2 0000 movt r0, #8192 ; 0x2000 80001f0: 6001 str r1, [r0, #0] ; CONSUMER = .. 80001f2: f24e 4106 movw r1, #58374 ; 0xe406 80001f6: f2ce 0100 movt r1, #57344 ; 0xe000 80001fa: 700a strb r2, [r1, #0] 80001fc: f24e 1100 movw r1, #57600 ; 0xe100 8000200: 2240 movs r2, #64 ; 0x40 8000202: f2ce 0100 movt r1, #57344 ; 0xe000 8000206: 600a str r2, [r1, #0] 8000208: b662 cpsie i 800020a: f8c1 2100 str.w r2, [r1, #256] ; 0x100 800020e: e006 b.n 800021e <main+0x58> 8000210: f3bf 8f5f dmb sy 8000214: 3201 adds r2, #1 8000216: f002 0207 and.w r2, r2, #7 800021a: 600a str r2, [r1, #0] 800021c: be00 bkpt 0x0000 800021e: 6801 ldr r1, [r0, #0] 8000220: 684b ldr r3, [r1, #4] 8000222: 680a ldr r2, [r1, #0] 8000224: 429a cmp r2, r3 8000226: d1f3 bne.n 8000210 <main+0x4a> 8000228: bf30 wfi 800022a: e7f8 b.n 800021e <main+0x58> ``` Unresolved questions: - Is this mechanism memory safe in presence of `panic!` unwinding? - If not, can we generate a compile error if `panic = abort` is *not* used? - How does this affect the DMA API proposed in rust-embedded/embedded-hal#14 cc @pftbest
here is an example project:
https://gitlab.com/xnor/stm32f0308-disco-rust
If I build it without --release I get
The text was updated successfully, but these errors were encountered: