From 71d7f2815f25d5387e467d58dec658e1a6dc6b8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Mon, 3 Aug 2026 09:37:22 +0200 Subject: [PATCH 1/4] Fix window spilling in esp-backtrace --- esp-backtrace/src/xtensa.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/esp-backtrace/src/xtensa.rs b/esp-backtrace/src/xtensa.rs index 802fbc761e6..8aa6b6697dd 100644 --- a/esp-backtrace/src/xtensa.rs +++ b/esp-backtrace/src/xtensa.rs @@ -20,15 +20,15 @@ fn sp() -> u32 { asm!( "mov {0}, a1", // current stack pointer // Spill registers, otherwise `sp - 12` will not contain the previous stack pointer - "add a12,a12,a12", + "and a12,a12,a12", "rotw 3", - "add a12,a12,a12", + "and a12,a12,a12", "rotw 3", - "add a12,a12,a12", + "and a12,a12,a12", "rotw 3", - "add a12,a12,a12", + "and a12,a12,a12", "rotw 3", - "add a12,a12,a12", + "and a12,a12,a12", "rotw 4", out(reg) sp ); From 6cc318ef2f525fac24a894bf4616ffd11235448c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Mon, 3 Aug 2026 09:49:07 +0200 Subject: [PATCH 2/4] Explicitly set the UM bit in handlers --- esp-rtos/src/task/xtensa.rs | 3 ++- xtensa-lx-rt/src/exception/asm.rs | 12 +++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/esp-rtos/src/task/xtensa.rs b/esp-rtos/src/task/xtensa.rs index d6732ae5de9..f6771697ac2 100644 --- a/esp-rtos/src/task/xtensa.rs +++ b/esp-rtos/src/task/xtensa.rs @@ -50,7 +50,8 @@ extern "C" fn idle_entry() -> ! { // Exception mode. Setting this bit prevents interrupts below EXCMLEVEL. Cleared by `rfe` at the end // of the Level 1 interrupt handler. const PS_EXCM: u32 = 1 << 4; -// User mode. This bit doesn't matter for us yet, we don't have separate kernel mode exceptions. +// User mode. Selects the user exception vector, instead of the kernel one. Both vectors point at +// the same handler, but tasks must run with this bit set, because the interrupt handlers do, too. const PS_UM: u32 = 1 << 5; // Windowed mode. const PS_WOE: u32 = 1 << 18; diff --git a/xtensa-lx-rt/src/exception/asm.rs b/xtensa-lx-rt/src/exception/asm.rs index 634e11567ed..731b26ad673 100644 --- a/xtensa-lx-rt/src/exception/asm.rs +++ b/xtensa-lx-rt/src/exception/asm.rs @@ -87,7 +87,7 @@ global_asm!( .set PS_INTLEVEL_EXCM, 3 // interrupt handlers above this level shouldn't be written in high level languages .set PS_INTLEVEL_MASK, 0x0000000f .set PS_EXCM, 0x00000010 - .set PS_UM, 0x00000020 + .set PS_UM, 0x00000020 // user mode: general exceptions use the user, not the kernel vector .set PS_WOE, 0x00040000 .set EXCCAUSE_LEVEL1_INTERRUPT, 4 @@ -193,7 +193,7 @@ global_asm!( .macro HANDLE_INTERRUPT_LEVEL level SAVE_CONTEXT \\level - movi a0, (\\level | PS_WOE) + movi a0, (\\level | PS_WOE | PS_UM) wsr a0, PS rsync @@ -343,6 +343,8 @@ save_context: // SPILL_REGISTERS macro requires window overflow exceptions to be enabled, // i.e. PS.EXCM cleared and PS.WOE set. + // We also set PS.UM, so that an exception during the spill uses the same + // vector as the rest of the handler. // Since we are going to clear PS.EXCM, we also need to increase INTLEVEL // at least to XCHAL_EXCM_LEVEL. This matches that value of effective INTLEVEL // at entry (CINTLEVEL=max(PS.INTLEVEL, XCHAL_EXCM_LEVEL) when PS.EXCM is set. @@ -362,7 +364,7 @@ save_context: bgeui a3, +PS_INTLEVEL_EXCM, 1f // calculate max(INTLEVEL, XCHAL_EXCM_LEVEL) - 3 = XCHAL_EXCM_LEVEL movi a3, PS_INTLEVEL_EXCM 1: - movi a0, PS_WOE // clear EXCM, enable window overflow, set new INTLEVEL + movi a0, (PS_WOE | PS_UM) // clear EXCM, enable window overflow, set user mode and new INTLEVEL or a3, a3, a0 wsr a3, ps rsr a0, EPC1 @@ -518,7 +520,7 @@ __default_naked_exception: bnei a6, EXCCAUSE_LEVEL1_INTERRUPT, .HandleException // Handle exception elsewhere - movi a0, (1 | PS_WOE) // set PS.INTLEVEL accordingly + movi a0, (1 | PS_WOE | PS_UM) // set PS.INTLEVEL, and run the handler in user mode wsr a0, PS rsync mov a6, sp // put address of save frame in a6=a2 in callee @@ -533,7 +535,7 @@ __default_naked_exception: .HandleException: mov a7, sp // put address of save frame in a7=a3 in callee - movi a0, (PS_INTLEVEL_EXCM | PS_WOE) + movi a0, (PS_INTLEVEL_EXCM | PS_WOE | PS_UM) wsr a0, PS rsync From aeae204df77c93b48163e0c8864c68dfce06b4a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Mon, 3 Aug 2026 09:58:31 +0200 Subject: [PATCH 3/4] Fix stack sizes --- esp-rtos/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/esp-rtos/src/lib.rs b/esp-rtos/src/lib.rs index 0c610f39e6d..8c1ad2af31b 100644 --- a/esp-rtos/src/lib.rs +++ b/esp-rtos/src/lib.rs @@ -412,7 +412,7 @@ pub fn start_with_idle_hook( let stack_bottom = (&raw const _stack_end_cpu0).cast::>(); let stack_slice = core::ptr::slice_from_raw_parts_mut( stack_bottom.cast_mut(), - stack_top as usize - stack_bottom as usize, + (stack_top as usize - stack_bottom as usize) / 4, ); task::allocate_main_task( @@ -482,7 +482,7 @@ pub fn start_second_core_with_stack_guard_offset( let stack_ptrs = SecondCoreStack { stack: core::ptr::slice_from_raw_parts_mut( stack.bottom().cast::>(), - STACK_SIZE, + STACK_SIZE / 4, ), }; From 0e32a7757c42f4eebacac492b905cdfc18852377 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Mon, 3 Aug 2026 09:58:53 +0200 Subject: [PATCH 4/4] Check for overflow in the idle task --- esp-rtos/src/scheduler.rs | 44 +++++++++++++++++++++++++++++++-------- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/esp-rtos/src/scheduler.rs b/esp-rtos/src/scheduler.rs index ee295ebb702..35737068f89 100644 --- a/esp-rtos/src/scheduler.rs +++ b/esp-rtos/src/scheduler.rs @@ -65,6 +65,13 @@ pub(crate) struct CpuState { /// Pointer to the task that is scheduled for deletion. pub(crate) to_delete: TaskList, + /// Set while the CPU executes the idle context. + /// + /// The idle context has no `Task`, so the thread pointer is null while it runs. A task that + /// has deleted itself also has a null thread pointer, so the flag is needed to tell the two + /// apart. + idle: bool, + // This context will be filled out by the first context switch. // We allocate the main task statically, because there is always a main task. If deleted, we // simply don't deallocate this. @@ -77,6 +84,7 @@ impl CpuState { initialized: false, idle_context: CpuContext::new(), to_delete: TaskList::new(), + idle: false, #[cfg(multi_core)] current_task: core::ptr::null_mut(), @@ -227,19 +235,34 @@ impl SchedulerState { } let current_task = NonNull::new(read_thread_pointer()); - if let Some(current_task) = current_task { + + // The idle task has no Task structure, and it has no stack of its own - it runs on the + // main task's stack. Check the main task in that case, so that a deep idle hook cannot + // overflow the main stack unnoticed. Before the main task is set up, there is no stack + // guard to check. A task that deleted itself also has no thread pointer, but it still runs + // on its own stack, which is about to be freed - there is nothing to check for it. + let stack_owner = match current_task { + Some(current_task) => Some(current_task), + None if self.per_cpu[current_cpu].idle => { + Some(NonNull::from(&self.per_cpu[current_cpu].main_task)) + } + None => None, + }; + if let Some(stack_owner) = stack_owner { unsafe { - current_task + stack_owner .as_ref() .ensure_no_stack_overflow(current_sp as usize) }; + } - if current_task.state() == TaskState::Ready { - // Current task is still ready, mark it as such. - debug!("re-queueing current task: {:?}", current_task); - self.run_queue.mark_task_ready(&self.per_cpu, current_task); - } - }; + if let Some(current_task) = current_task + && current_task.state() == TaskState::Ready + { + // Current task is still ready, mark it as such. + debug!("re-queueing current task: {:?}", current_task); + self.run_queue.mark_task_ready(&self.per_cpu, current_task); + } let mut arm_next_timeslice_tick = false; let next_task = self.run_queue.pop(); @@ -311,6 +334,8 @@ impl SchedulerState { &raw mut self.per_cpu[current_cpu].idle_context }; + self.per_cpu[current_cpu].idle = next_task.is_none(); + task_switch(current_context, next_context); // If we went to idle, this will be None and we won't mess up the main task's stack. @@ -454,7 +479,8 @@ impl SchedulerState { #[cfg(all(multi_core, sleep_light_sleep))] pub(crate) fn cpu_idle(&self, cpu: Cpu) -> bool { let per_cpu = &self.per_cpu[cpu as usize]; - !per_cpu.initialized || per_cpu.current_task.is_null() + // A CPU that never started the scheduler has no work to do, so it counts as idle. + !per_cpu.initialized || per_cpu.idle } }