diff --git a/sys/kern/src/arch/arm_m.rs b/sys/kern/src/arch/arm_m.rs index 21443f1c3..8019d1a43 100644 --- a/sys/kern/src/arch/arm_m.rs +++ b/sys/kern/src/arch/arm_m.rs @@ -1185,10 +1185,14 @@ unsafe extern "C" fn pendsv_entry() { crate::profiling::event_secondary_syscall_enter(); let current = CURRENT_TASK_PTR.load(Ordering::Relaxed); + // We're being slightly pedantic about this, since it's straightforward (if + // weird) for pre-kernel main to trigger a PendSV, and it's not possible to + // disable this interrupt source. uassert!(!current.is_null()); // irq before kernel started? // Safety: we're dereferencing the current task pointer, which we're - // trusting the rest of this module to maintain correctly. + // trusting the rest of this module to maintain correctly (and we've just + // checked that the kernel has started). let current = usize::from(unsafe { (*current).descriptor().index }); with_task_table(|tasks| { @@ -1222,10 +1226,19 @@ pub unsafe extern "C" fn DefaultHandler() { }; let current = CURRENT_TASK_PTR.load(Ordering::Relaxed); - uassert!(!current.is_null()); // irq before kernel started? - { - let t = unsafe { &mut *current }; + // Because this handler is reachable in response to an NMI, it's possible to + // get here before the kernel has started. So before dereferencing the task + // pointer, check for NULL (the state at reset). If we manage to get here + // before the kernel has started, we'll skip the stack watermark update and + // hit a panic below. + + // Safety: this dereferences the pointer only if it isn't NULL. If it + // isn't NULL, that means we've initialized it elsewhere in this module + // and it's a valid (but aliased) pointer to a task. We can safely + // dereference it as long as we throw it away before hitting + // `with_task_table` below. + if let Some(t) = unsafe { current.as_mut() } { t.update_stack_watermark(); } @@ -1251,6 +1264,8 @@ pub unsafe extern "C" fn DefaultHandler() { .get(abi::InterruptNum(irq_num)) .unwrap_or_else(|| panic!("unhandled IRQ {irq_num}")); + // with_task_table will panic if the kernel has not yet been + // started. This is good. let switch = with_task_table(|tasks| { disable_irq(irq_num, false); diff --git a/sys/kern/src/task.rs b/sys/kern/src/task.rs index b151adbe9..77b45ef5a 100644 --- a/sys/kern/src/task.rs +++ b/sys/kern/src/task.rs @@ -50,19 +50,12 @@ pub struct Task { /// restarted. descriptor: &'static TaskDesc, - /// Tracks the lowest stack pointer value (e.g. fullest stack) observed on - /// any kernel entry for this instance of this task. + /// Stack watermark tracking support. /// - /// Initialized to `u32::MAX` if the task has not yet run. + /// This field is completely missing if the feature is disabled to make that + /// clear to debug tools. #[cfg(feature = "stack-watermark")] - stack_pointer_low: u32, - - /// Tracks the lowest stack pointer value (e.g. fullest stack) observed on - /// any kernel entry across *any* instance of this task. - /// - /// Initialized to `u32::MAX` if the task has not yet run. - #[cfg(feature = "stack-watermark")] - past_stack_pointer_low: u32, + stack_watermark: StackWatermark, } impl Task { @@ -84,9 +77,7 @@ impl Task { save: crate::arch::SavedState::default(), timer: crate::task::TimerState::default(), #[cfg(feature = "stack-watermark")] - stack_pointer_low: u32::MAX, - #[cfg(feature = "stack-watermark")] - past_stack_pointer_low: u32::MAX, + stack_watermark: StackWatermark::default(), } } @@ -341,9 +332,11 @@ impl Task { #[cfg(feature = "stack-watermark")] { - self.past_stack_pointer_low = - u32::min(self.past_stack_pointer_low, self.stack_pointer_low); - self.stack_pointer_low = u32::MAX; + self.stack_watermark.past_low = u32::min( + self.stack_watermark.past_low, + self.stack_watermark.current_low, + ); + self.stack_watermark.current_low = u32::MAX; } crate::arch::reinitialize(self); @@ -356,8 +349,10 @@ impl Task { pub fn update_stack_watermark(&mut self) { #[cfg(feature = "stack-watermark")] { - self.stack_pointer_low = - u32::min(self.stack_pointer_low, self.save().stack_pointer()); + self.stack_watermark.current_low = u32::min( + self.stack_watermark.current_low, + self.save().stack_pointer(), + ); } } @@ -418,6 +413,32 @@ impl Task { } } +#[cfg(feature = "stack-watermark")] +#[derive(Copy, Clone, Debug)] +struct StackWatermark { + /// Tracks the lowest stack pointer value (e.g. fullest stack) observed on + /// any kernel entry for this instance of this task. + /// + /// Initialized to `u32::MAX` if the task has not yet run. + current_low: u32, + + /// Tracks the lowest stack pointer value (e.g. fullest stack) observed on + /// any kernel entry across *any* instance of this task. + /// + /// Initialized to `u32::MAX` if the task has not yet run. + past_low: u32, +} + +#[cfg(feature = "stack-watermark")] +impl Default for StackWatermark { + fn default() -> Self { + Self { + current_low: u32::MAX, + past_low: u32::MAX, + } + } +} + /// Interface that must be implemented by the `arch::SavedState` type. This /// gives architecture-independent access to task state for the rest of the /// kernel.