Skip to content

Interrupt handlers from app macro don't appear to work in debug mode #39

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

Closed
cs2dsb opened this issue Jul 30, 2017 · 9 comments
Closed

Comments

@cs2dsb
Copy link

cs2dsb commented Jul 30, 2017

I've converted a project I'm tinkering with to v2 but a basic example of a timer interrupt doesn't work in debug mode.

In release mode it works as expected, never hits the default_handler and a counter the timer interrupt is incrementing goes up every 10 seconds as expected.

In debug mode it goes straight to the default handler. The exception is 55, which is the correct interrupt.

I've double checked the "rt" feature flag stuff is getting generated in my device crate and the xargo expand output contains what looks like an override for the weak symbol defined in the device support crate:

#[allow(non_snake_case)]
#[allow(unsafe_code)]
#[export_name = "TIM7"]
pub unsafe extern "C" fn _TIM7() {
    let f: fn(&mut rtfm::Threshold, TIM7::Resources) = tim7_int;
    f(
        &mut if 1u8 == 1 << stm32f7x6::NVIC_PRIO_BITS {
            rtfm::Threshold::new(::core::u8::MAX)
        } else {
            rtfm::Threshold::new(1u8)
        },
        TIM7::Resources::new(),
    )
}

I'm a bit hazy on the details of how linking/weak symbols/etc works so please let me know if there are any commands i can run on the ELF files to get more info. This seemed relevant:

Release:
arm-none-eabi-objdump -t target/thumbv7em-none-eabihf/release/examples/adc | grep TIM7
08000222 g     F .text	0000002a TIM7

Then in gdb:
x 0x8000222
0x8000222 <adc::_TIM7>
info functions
<snip>
File examples/adc.rs:
fn adc::_TIM7();
  
Debug: 
arm-none-eabi-objdump -t target/thumbv7em-none-eabihf/debug/examples/adc | grep TIM7
0800539e g     F .text	00000012 _ZN59_$LT$stm32f7x6..TIM7$u20$as$u20$core..ops..deref..Deref$GT$5deref17h4b245b5fe55c3366E

Then in gdb:
x 0x0800539e
0x800539e <stm32f7x6::{{impl}}::deref>:	0x4601b083
info functions
<snip>
File examples/adc.rs:
static fn adc::_initResources::new() -> adc::_initResources;
static fn adc::idle() -> !;
static fn adc::init(stm32f7x6::Peripherals, adc::_initResources);
static fn adc::main();
static fn adc::main::{{closure}}(rtfm_core::Threshold *);

I've updated my arm and rust toolchains and double checked the various xargo and cargo config files for all projects and everything seems to be in line with your blue-pill repo.

Thanks

@cs2dsb
Copy link
Author

cs2dsb commented Jul 30, 2017

Ok, I worked out what was going on.
The release profile has LTO turned on but debug/dev doesn't.
Turning it on for [profile.dev] allowed the interrupts to work as expected.

Although I've worked out the issue, I still think it's a bug that different optimisations result in different behaviour. I can see that LTO is required for the weak symbols to be resolved. Changing the way that the vector table is defined so that it's pure rust rather than using weak symbols and extern c functions sounds great to me (if possible). Alternatively there could be a build.rs that checks the rustflags for the current profile that fails if required options aren't set correctly.

Let me know what you think, I'm pretty new to rust to feel free to shoot me down ;)

@cs2dsb cs2dsb closed this as completed Jul 30, 2017
@cs2dsb cs2dsb reopened this Jul 30, 2017
@pftbest
Copy link

pftbest commented Jul 30, 2017

That's interesting. I wonder if linker is not overriding the weak symbol properly. LTO should not be required for this to work.

@japaric
Copy link
Collaborator

japaric commented Jul 31, 2017

My hypothesis (I haven't looked at anything yet) is that the linker is dropping the TIM7 symbol defined in the application. It's likely doing that because of how Rust objects are being passed to it (the order matters) in the command line; you don't see this because in LTO there's only one object file passed to the linker in that case.

Will take a look.

@cs2dsb
Copy link
Author

cs2dsb commented Jul 31, 2017

Thanks, let me know if there is anything I can do to help.

@japaric
Copy link
Collaborator

japaric commented Jul 31, 2017

This

.weak FOO
FOO = BAR

which cortex-m-rt and device crates are using doesn't produce any symbol; neither clang or rustc produce symbols for it. It seems that the symbol resolution for that code is entirely tracked in LLVM. For this reason there's e.g. no undefined TIM7 symbol in the device crate so the linker won't keep around the TIM7 symbol defined in the top crate.

How to deal with this:

  • In short term we can revert Fix duplicated exception handlers. rust-embedded/svd2rust#130 and define handlers as weak aliases of DEFAULT_HANDLER rust-embedded/cortex-m-rt#27 to fix this in the current releases. This is not a breaking change.

  • In the longer term we can consider using linker scripts (with lines like PROVIDE(TIM7 = DEFAULT_HANDLER)) to avoid using global_asm!. This would be a breaking change (in cortex-m-rt and cortex-m-quickstart) because application crates would need to change their linker invocation to include -Texceptions.x -Tinterrupts.x; in this new version the device crate would include a bunch of undefined symbols to the interrupt handlers (U TIM7) which would be "provided" by the linker script (without the linker script you would get a bunch of linker errors).

@pftbest
Copy link

pftbest commented Jul 31, 2017

@japaric

How did you test for that?

For example, foo.rs:

#![feature(global_asm)]
#![feature(linkage)]

#[no_mangle]
#[linkage = "weak"]
pub extern "C" fn BAR() {
}

global_asm!(r#"
.weak FOO
FOO = BAR
"#);

Compiling:

$ rustc --crate-type rlib -C opt-level=3 foo.rs

Both symbols are present in the object file:

$ nm libfoo.rlib 

foo.0.o:
0000000000000000 W BAR
0000000000000000 W FOO

@pftbest
Copy link

pftbest commented Jul 31, 2017

@japaric
I think I got it!
DEFAULT_HANDLER is undefined in device crate, that is why LLVM fails to create an alias for it.
We just need to move it from rt crate into device crate.

clang will emit a compilation error for such case:

4 : <source>:4:22: error: alias must point to a defined variable or function
__attribute__((weak, alias("bar")))
                     ^
1 error generated.
Compiler exited with result code 1

@japaric
Copy link
Collaborator

japaric commented Aug 2, 2017

Fixed in svd2rust v0.11.3. Note that you'll have to regenerate device crates that were created using svd2rust v0.11.2 using the latest svd2rust version to fix this problem in your application.

@japaric japaric closed this as completed Aug 2, 2017
@cs2dsb
Copy link
Author

cs2dsb commented Aug 2, 2017

Can confirm it's fixed it for me. Thanks for the quick fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants