Monitor the main stack's stack guard #4207

bjoernQ · 2025-09-29T13:45:29Z

Thank you for your contribution!

We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:

Submission Checklist 📝

I have updated existing examples or added new ones (if applicable).
I have used cargo xtask fmt-packages command to ensure that all changed code is formatted correctly.
My changes were added to the CHANGELOG.md in the proper section.
I have added necessary changes to user code to the latest Migration Guide.
My changes are in accordance to the esp-rs developer guidelines

Extra:

I have read the CONTRIBUTING.md guide and followed its instructions.

Pull Request Details 📖

Description

This gives us the stack-overflow part from the (unstable) stack-protector feature for free.

We could even make the "save zone" a bit larger - up to 64 bytes on Xtensa and "as much as we like" on RISC-V - but it would create constraints on the alignment - it seems this might already help.

e.g. this example on C6 does funny things without the protection (using rustc 1.91.0-nightly (5eda692e7 2025-09-11))
(make sure to set the guard-offset to approx. 4096)

//% CHIPS: esp32 esp32c3 esp32c6 esp32h2 esp32s2 esp32s3 #![no_std] #![no_main] use esp_backtrace as _; use esp_hal::{delay::Delay, main}; esp_bootloader_esp_idf::esp_app_desc!(); #[main] fn main() -> ! { let _peripherals = esp_hal::init(esp_hal::Config::default()); let _delay = Delay::new(); smash_it(); loop {} } const FOO: usize = 1000; fn smash_it() { let f = [0u8; FOO]; smash(0, &f); } #[allow(unconditional_recursion)] #[inline(never)] fn smash(i: usize, _foo: &[u8; FOO]) { esp_println::println!("{i}"); let f = [0u8; FOO]; smash(i + 1, &f); }

If we see this working "well enough" we can consider expanding it to esp-rtos tasks.

skip-changelog for the change in esp-preempt

Testing

Manual testing, running existing examples

bugadani · 2025-09-29T13:48:27Z

esp-hal/esp_config.yml

 - '8'
 active: 'chip == "esp32s3"'
+
+# TODO somni doesn't yet support `%` and evalexpr doesn't support `&`


should be available with somni-expr 0.2.0

esp-rtos/src/task/mod.rs

bugadani · 2025-09-29T13:56:09Z

esp-hal/src/exception_handler/mod.rs


+#[cfg(xtensa)]
+#[unsafe(no_mangle)]
+#[unsafe(link_section = ".rwtext")]


Does this have to be in RAM? We should be able to load this from flash, and the performance isn't critical as this will just die miserably if called.

I thought the same - for now this was just copied, but in this case since we know "the world is about to end" a cache miss is not the worst thing that could happen 👍

esp-hal/src/soc/mod.rs

bugadani · 2025-09-30T08:48:22Z

esp-hal/src/soc/esp32/cpu_control.rs

 debug_assert!(!entry.is_null());

+ #[cfg(stack_guard_monitoring)]
+ crate::soc::enable_main_stack_guard_monitoring();


I don't think this will work as is, at least it's not watching the second core's stack.

it's not intended for now to monitor the second core's stack - just detect the second core trashing the first core's stack - maybe not too useful/likely - t.b.h. I'm not sure if observing the other core's stack is really helpful 🤔

Observing the second core's stack (that is, the second core guarding its own stack) makes a lot more sense to me, especially since it's much more limited than the first one's. Cross-checking like this is probably not very useful, randomly writing to that one address is very unlikely.

thinking about it - trying to do that will probably open a can of worms - I changed my mind - let's not monitor the other cores stack

esp-hal/src/interrupt/xtensa.rs

esp-rtos/src/lib.rs

bugadani · 2025-09-30T12:43:23Z

That recursive assert failure doesn't look too pretty 😅

bugadani · 2025-09-30T12:44:09Z

esp-hal/src/soc/esp32/cpu_control.rs

 // is copied to the core's stack.
 static START_CORE1_FUNCTION: AtomicPtr<()> = AtomicPtr::new(core::ptr::null_mut());
 static APP_CORE_STACK_TOP: AtomicPtr<u32> = AtomicPtr::new(core::ptr::null_mut());
+static APP_CORE_STACK_GUARD: AtomicPtr<u32> = AtomicPtr::new(core::ptr::null_mut());


This should be cfg-gated to not generate warnings - or maybe the whole functionality shouldn't 🤔

ah it's not unused because of the always present start_app_core_with_stack_guard_offset - but I wasn't honoring stack_guard_monitoring in start_app_core 👍

bugadani

S3 tests are broken. Please avoid pushing to this PR without fixing it, the HIL runner can't recover by itself.

bjoernQ · 2025-09-30T14:46:36Z

S3 tests are broken. Please avoid pushing to this PR without fixing it, the HIL runner can't recover by itself.

~~This is an interesting thing - the offending test works perfectly well locally - something to figure out later probably~~

Ok - not having an env var set to disable it helps a lot

esp-hal/src/debugger.rs

bugadani · 2025-09-30T16:03:05Z

esp-hal/src/soc/esp32s3/cpu_control.rs

+ {
+ let stack_guard = APP_CORE_STACK_GUARD.load(Ordering::Acquire);
+ // setting 0 effectively disables the functionality
+ crate::debugger::set_stack_watchpoint(stack_guard as usize);


The stack guard value should be written before enabling the watchpoint on the second core

we don't write the stack guard yet - but why not might help if someone is manually inspecting the stack

This is currently incidental, esp-rtos may decide to place it's stack canary to the watched address

Co-authored-by: Dániel Buga <bugadani@gmail.com>

* Allow setting a custom idle hook (esp-rs#4209) * Document esp-rtos just a bit (esp-rs#4208) * Slight MG touchup * Document esp-rtos in migration guide * Document things * Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Improve esp-rtos docs --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * refactor(ledc): introduce freq unset error and mode conversion (esp-rs#4214) * feat: introduce freq unset error and mode conversion * chore: format * refactor * refactor!: rename api * fix * fix docs * Disable unusable pins for various chips (esp-rs#4202) * ESP32: disable integrated SPI-connected pins * remove unavailable/non-existent pins for esp32c2 and esp32h2 * address reviews, drop more user-unusable pins * edit the changelog * fix psram (esp32) oi * RTOS: Use simpler sync object for executor (esp-rs#4215) * Use simpler sync object internally * Update esp-rtos/src/task/mod.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Monitor the main stack's stack guard (esp-rs#4207) * Monitor the main stack's stack guard * CHANGELOG.md * Clippy * Save a bit of RAM * Some some IRAM * Avoid warning * Hint on register names * Protect the main stack on the second core, too * Fix `rt` w/o main stack protection * Use correct cfg gates * Refactor * Improvements * More rt gating * Stack-guard for second core * Adapt examples and tests * Bump somni-expr * CHANGELOG and migration guide * Fix doc-tests * fmt * Use ESP_HAL_CONFIG_STACK_GUARD_OFFSET for the second core * Fix test * Honor stack_guard_monitoring everywhere * Fix rebased migration guide * Fix doc-tests again * Update esp-hal/src/debugger.rs Co-authored-by: Dániel Buga <bugadani@gmail.com> * Write the stack guard value on the second core --------- Co-authored-by: Dániel Buga <bugadani@gmail.com> * Move `multicore` from `esp-storage` to `esp-hal` (esp-rs#4188) * Move multicore from esp-storage to esp-hal * changelog * reviews * address review comments and clean up * remove duplicated park_core() and split is_running() per chip * cleanup * Clean up blocking until deadline (esp-rs#4217) * Hw-based RTOS stack overflow detection (esp-rs#4218) * Respect the stack guard offset in thread stacks * Use the hardware watchpoint to protect rtos thread stacks * Use watchpoints for stack overflow detection * Do not rely on the value of the stack guard (esp-rs#4220) * Make BLE task configurable (esp-rs#4223) * Redo config options (esp-rs#4224) * Remove unused deps (esp-rs#4230) * `esp-phy` the second (esp-rs#4228) * Hopefully fixes it * fmt * Fix documentation link in Cargo.toml --------- Co-authored-by: Dániel Buga <bugadani@gmail.com> * twai: do not abort transmissions on recoverable errors (esp-rs#4227) * twai: do not abort transmissions on recoverable errors The TWAI driver's interrupt handler currently aborts transmissions if certain (error) interrupts were triggered. This causes the transmission of frames to become unreliable, because the `transmit_async` operation does not return any error codes if a transmission was aborted. Additionally, the TWAI controller is designed to be a reliable and, with the exception of the bus-off state, normally would simply try to rentransmit a message on its own. Therefore, this commit removes transmission abortion on errors and, instead, only aborts a transmission if the controller changes into the bus-off state. Fixes esp-rs#4222 * Update esp-hal/CHANGELOG.md Co-authored-by: Juraj Sadel <jurajsadel@gmail.com> --------- Co-authored-by: Juraj Sadel <jurajsadel@gmail.com> * Change default wifi_max_burst_size (esp-rs#4231) * Remove outdated limitation (esp-rs#4232) * feat: Use github actions artifacts to store semver baseline --------- Co-authored-by: Dániel Buga <bugadani@gmail.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Ethan Wu <7086cmd@gmail.com> Co-authored-by: Kirill Mikhailov <62840029+playfulFence@users.noreply.github.com> Co-authored-by: Björn Quentin <bjoernQ@users.noreply.github.com> Co-authored-by: Juraj Sadel <juraj.sadel@espressif.com> Co-authored-by: Simon Neuenhausen <frostie.neuenhausen@gmail.com> Co-authored-by: Harald Böhm <harald@boehm.dev> Co-authored-by: Juraj Sadel <jurajsadel@gmail.com>

bugadani reviewed Sep 29, 2025

View reviewed changes

esp-rtos/src/task/mod.rs Show resolved Hide resolved

bugadani reviewed Sep 29, 2025

View reviewed changes

bjoernQ added the skip-changelog No changelog modification needed label Sep 29, 2025

bugadani reviewed Sep 29, 2025

View reviewed changes

esp-hal/src/soc/mod.rs Outdated Show resolved Hide resolved

bjoernQ force-pushed the main-stack-overflow-check branch from 7f1c63a to 7aa0d62 Compare September 30, 2025 08:43

bugadani reviewed Sep 30, 2025

View reviewed changes

bjoernQ force-pushed the main-stack-overflow-check branch 2 times, most recently from 89a7f26 to 71b9f54 Compare September 30, 2025 10:32

bugadani reviewed Sep 30, 2025

View reviewed changes

esp-hal/src/interrupt/xtensa.rs Show resolved Hide resolved

bugadani reviewed Sep 30, 2025

View reviewed changes

esp-rtos/src/lib.rs Show resolved Hide resolved

bugadani approved these changes Sep 30, 2025

View reviewed changes

bugadani reviewed Sep 30, 2025

View reviewed changes

bjoernQ added 16 commits September 30, 2025 14:59

Monitor the main stack's stack guard

2b97db0

CHANGELOG.md

4c0e61a

Clippy

5b5a0f5

Save a bit of RAM

c9ae891

Some some IRAM

dc45972

Avoid warning

073a07a

Hint on register names

05f5702

Protect the main stack on the second core, too

01e5f00

Fix rt w/o main stack protection

78f46a0

Use correct cfg gates

fec6684

Refactor

5e79cde

Improvements

dda64b5

More rt gating

66db820

Stack-guard for second core

c61b96d

Adapt examples and tests

2a0054d

Bump somni-expr

1bcc9cb

bjoernQ added 7 commits September 30, 2025 14:59

CHANGELOG and migration guide

2bd07a3

Fix doc-tests

0b4e5e5

fmt

c8dc227

Use ESP_HAL_CONFIG_STACK_GUARD_OFFSET for the second core

dc8a110

Fix test

96627e9

Honor stack_guard_monitoring everywhere

ccd46e2

Fix rebased migration guide

c259809

bjoernQ force-pushed the main-stack-overflow-check branch from efcb57f to c259809 Compare September 30, 2025 13:05

Fix doc-tests again

86eb5da

bugadani requested changes Sep 30, 2025

View reviewed changes

bugadani reviewed Sep 30, 2025

View reviewed changes

esp-hal/src/debugger.rs Outdated Show resolved Hide resolved

bugadani reviewed Sep 30, 2025

View reviewed changes

bjoernQ and others added 2 commits September 30, 2025 18:07

Update esp-hal/src/debugger.rs

0b58113

Co-authored-by: Dániel Buga <bugadani@gmail.com>

Write the stack guard value on the second core

e299d44

bugadani approved these changes Sep 30, 2025

View reviewed changes

bugadani enabled auto-merge September 30, 2025 16:26

bugadani added this pull request to the merge queue Sep 30, 2025

Merged via the queue into esp-rs:main with commit c66e4cf Sep 30, 2025
28 checks passed

bugadani mentioned this pull request Oct 1, 2025

Hw-based RTOS stack overflow detection #4218

Merged

bjoernQ mentioned this pull request Oct 7, 2025

Update deps to latest esp-hal crates esp-rs/no_std-training#82

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Monitor the main stack's stack guard #4207

Monitor the main stack's stack guard #4207

Uh oh!

bjoernQ commented Sep 29, 2025 •

edited

Loading

bugadani Sep 29, 2025

bugadani Sep 29, 2025

Uh oh!

bugadani Sep 29, 2025

bjoernQ Sep 29, 2025

Uh oh!

bugadani Sep 30, 2025

bjoernQ Sep 30, 2025

bugadani Sep 30, 2025 •

edited

Loading

bjoernQ Sep 30, 2025

Uh oh!

Uh oh!

bugadani commented Sep 30, 2025

bugadani Sep 30, 2025 •

edited

Loading

bjoernQ Sep 30, 2025

bugadani left a comment

bjoernQ commented Sep 30, 2025 •

edited

Loading

Uh oh!

bugadani Sep 30, 2025

bjoernQ Sep 30, 2025

bugadani Sep 30, 2025

Uh oh!

Labels

2 participants

Monitor the main stack's stack guard #4207

Monitor the main stack's stack guard #4207

Uh oh!

Conversation

bjoernQ commented Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Thank you for your contribution!

Submission Checklist 📝

Extra:

Pull Request Details 📖

Description

Testing

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bugadani Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bugadani commented Sep 30, 2025

bugadani Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bugadani left a comment

Choose a reason for hiding this comment

bjoernQ commented Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Uh oh!

Labels

2 participants

bjoernQ commented Sep 29, 2025 •

edited

Loading

bugadani Sep 30, 2025 •

edited

Loading

bugadani Sep 30, 2025 •

edited

Loading

bjoernQ commented Sep 30, 2025 •

edited

Loading