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

Fix unwinding through Reset #337

Merged
merged 4 commits into from
Nov 23, 2021

Conversation

jonas-schievink
Copy link
Contributor

Unwinders may detect the end of the program by seeing 0xFFFFFFFF in lr, which is why code to ensure that it has that value was added in #293. However, the bl main overwrites that value with the current program counter. This PR saves the old lr value on the stack, and adds debuginfo entries to allow an external unwinder to restore the value.

This fixes knurling-rs/probe-run#277

@rust-highfive
Copy link

r? @therealprof

(rust-highfive has picked a reviewer for you, use r? to override)

@@ -94,6 +94,10 @@ Reset:
#endif

4:
.cfi_def_cfa sp, 0
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be worth an explanation, as it's not something you often come across in ARM asm.

Maybe link to https://sourceware.org/binutils/docs/as/CFI-directives.html, or include their note:

.cfi_def_cfa defines a rule for computing CFA as: take address from register and add offset to it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a comment and link

@jonathanpallant
Copy link
Contributor

Can confirm it resolves the upstream issue though:

PS C:\Users\Test\Documents\knurling\app-template> cargo run --bin hello
    Blocking waiting for file lock on build directory
   Compiling cortex-m-rt v0.7.0 (https://github.com/knurling-rs/cortex-m-rt.git?branch=fix-unwind-through-reset#9c88eab9)
   Compiling stm32f4xx-hal v0.10.1
   Compiling my_project v0.1.0 (C:\Users\Test\Documents\knurling\app-template)
    Finished dev [optimized + debuginfo] target(s) in 15.88s
     Running `probe-run --chip STM32F429ZITx target\thumbv7em-none-eabi\debug\hello`
(HOST) INFO  flashing program (7.93 KiB)
(HOST) INFO  success!
────────────────────────────────────────────────────────────────────────────────
Hello, world!
└─ hello::__cortex_m_rt_main @ src\bin\hello.rs:8
────────────────────────────────────────────────────────────────────────────────
(HOST) INFO  device halted without error

@jonathanpallant
Copy link
Contributor

bors r+

@bors
Copy link
Contributor

bors bot commented Nov 23, 2021

🔒 Permission denied

Existing reviewers: click here to make jonathanpallant a reviewer

@jonathanpallant
Copy link
Contributor

Ugh. Wearing the wrong hat.

@thejpster
Copy link
Contributor

bors r+

@thejpster
Copy link
Contributor

^ have switched hats

@bors
Copy link
Contributor

bors bot commented Nov 23, 2021

@bors bors bot merged commit c37794c into rust-embedded:master Nov 23, 2021
@jonas-schievink jonas-schievink deleted the fix-unwind-through-reset branch November 23, 2021 15:57
@jonathanpallant jonathanpallant mentioned this pull request Nov 24, 2021
bors bot added a commit that referenced this pull request Nov 24, 2021
339: Release 0.7.1 r=adamgreig a=jonathanpallant

Includes stack unwinding through Reset fix #337.

Co-authored-by: Jonathan Pallant (Ferrous Systems) <jonathan.pallant@ferrous-systems.com>
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

False positive "call stack was corrupted" warning on 0.3
5 participants