From f9b2cf0e76a525b33884945cfd80000dd6b26dce Mon Sep 17 00:00:00 2001 From: Cyril Date: Wed, 7 Jan 2026 16:36:00 +0800 Subject: [PATCH] implement MultiThreadSchedulerLocking support - Protocol: Add `MultiThreadSchedulerLocking` IDET to track and enforce GDB's scheduler-locking mode during `vCont` handling. - Error Handling: Introduce `MissingMultiThreadSchedulerLocking` internal error to provide clear feedback when the IDET is missing. - Documentation: Refactor IDET docs to focus on high-level GDB user behavior (`set scheduler-locking`) instead of RSP packet details. - Examples: Update `armv4t_multicore` to support core freezing via `ExecMode::Stop`, providing a reference implementation for lock-step multi-core targets. - Examples: Revert unnecessary cycle stall changes in the emulator. Closes #178 --- examples/armv4t_multicore/emu.rs | 35 +++++++++++--------- examples/armv4t_multicore/gdb.rs | 16 +++++++++ src/stub/core_impl/resume.rs | 17 ++++++++++ src/stub/error.rs | 2 ++ src/target/ext/base/multithread.rs | 52 ++++++++++++++++++++++++++++-- 5 files changed, 104 insertions(+), 18 deletions(-) diff --git a/examples/armv4t_multicore/emu.rs b/examples/armv4t_multicore/emu.rs index 1b9dac40..7e8595fc 100644 --- a/examples/armv4t_multicore/emu.rs +++ b/examples/armv4t_multicore/emu.rs @@ -36,9 +36,11 @@ pub enum Event { WatchRead(u32), } +#[derive(PartialEq)] pub enum ExecMode { Step, Continue, + Stop, } /// incredibly barebones armv4t-based emulator @@ -187,6 +189,10 @@ impl Emu { let mut evt = None; for id in [CpuId::Cpu, CpuId::Cop].iter().copied() { + if matches!(self.exec_mode.get(&id), Some(ExecMode::Stop)) { + continue; + } + if let Some(event) = self.step_core(id) { if evt.is_none() { evt = Some((event, id)); @@ -198,26 +204,25 @@ impl Emu { } pub fn run(&mut self, mut poll_incoming_data: impl FnMut() -> bool) -> RunEvent { - // the underlying armv4t_multicore emulator runs both cores in lock step, so - // when GDB requests a specific core to single-step, all we need to do is jot - // down that we want to single-step the system, as there is no way to - // single-step a single core while the other runs. + // The underlying armv4t_multicore emulator cycles all cores in lock-step. // - // In more complex emulators / implementations, this simplification is _not_ - // valid, and you should track which specific TID the GDB client requested to be - // single-stepped, and run them appropriately. - - let should_single_step = matches!( - self.exec_mode - .get(&CpuId::Cpu) - .or_else(|| self.exec_mode.get(&CpuId::Cop)), - Some(&ExecMode::Step) - ); + // Inside `self.step()`, we iterate through all cores and only invoke + // `step_core` if that core's `ExecMode` is not `Stop`. + + let should_single_step = self.exec_mode.values().any(|mode| mode == &ExecMode::Step); match should_single_step { true => match self.step() { Some((event, id)) => RunEvent::Event(event, id), - None => RunEvent::Event(Event::DoneStep, CpuId::Cpu), + None => { + let stepping_core = self + .exec_mode + .iter() + .find(|&(_, mode)| mode == &ExecMode::Step) + .map(|(id, _)| *id) + .unwrap_or(CpuId::Cpu); + RunEvent::Event(Event::DoneStep, stepping_core) + } }, false => { let mut cycles = 0; diff --git a/examples/armv4t_multicore/gdb.rs b/examples/armv4t_multicore/gdb.rs index 11ed1e3f..e6fb556c 100644 --- a/examples/armv4t_multicore/gdb.rs +++ b/examples/armv4t_multicore/gdb.rs @@ -185,6 +185,13 @@ impl MultiThreadResume for Emu { Ok(()) } + + #[inline(always)] + fn support_scheduler_locking( + &mut self, + ) -> Option> { + Some(self) + } } impl target::ext::base::multithread::MultiThreadSingleStep for Emu { @@ -294,6 +301,15 @@ impl target::ext::thread_extra_info::ThreadExtraInfo for Emu { } } +impl target::ext::base::multithread::MultiThreadSchedulerLocking for Emu { + fn set_resume_action_scheduler_lock(&mut self) -> Result<(), Self::Error> { + for id in [CpuId::Cpu, CpuId::Cop] { + self.exec_mode.entry(id).or_insert(ExecMode::Stop); + } + Ok(()) + } +} + /// Copy all bytes of `data` to `buf`. /// Return the size of data copied. pub fn copy_to_buf(data: &[u8], buf: &mut [u8]) -> usize { diff --git a/src/stub/core_impl/resume.rs b/src/stub/core_impl/resume.rs index 38a046c4..8c78a43e 100644 --- a/src/stub/core_impl/resume.rs +++ b/src/stub/core_impl/resume.rs @@ -166,6 +166,13 @@ impl GdbStubImpl { ) -> Result<(), Error> { ops.clear_resume_actions().map_err(Error::TargetError)?; + // Track whether the packet contains a wildcard/default continue action + // (e.g., `c` or `c:-1`). + // + // Presence of this action implies "Scheduler Locking" is OFF. + // Absence implies "Scheduler Locking" is ON. + let mut has_wildcard_continue = false; + for action in actions.iter() { use crate::protocol::commands::_vCont::VContKind; @@ -185,6 +192,7 @@ impl GdbStubImpl { None | Some(SpecificIdKind::All) => { // Target API contract specifies that the default // resume action for all threads is continue. + has_wildcard_continue = true; } Some(SpecificIdKind::WithId(tid)) => ops .set_resume_action_continue(tid, signal) @@ -251,6 +259,15 @@ impl GdbStubImpl { } } + if !has_wildcard_continue { + let Some(locking_ops) = ops.support_scheduler_locking() else { + return Err(Error::MissingMultiThreadSchedulerLocking); + }; + locking_ops + .set_resume_action_scheduler_lock() + .map_err(Error::TargetError)?; + } + ops.resume().map_err(Error::TargetError) } diff --git a/src/stub/error.rs b/src/stub/error.rs index 5b311dea..ec1ef931 100644 --- a/src/stub/error.rs +++ b/src/stub/error.rs @@ -43,6 +43,7 @@ pub(crate) enum InternalError { MissingCurrentActivePidImpl, TracepointFeatureUnimplemented(u8), TracepointUnsupportedSourceEnumeration, + MissingMultiThreadSchedulerLocking, // Internal - A non-fatal error occurred (with errno-style error code) // @@ -147,6 +148,7 @@ where MissingCurrentActivePidImpl => write!(f, "GDB client attempted to attach to a new process, but the target has not implemented support for `ExtendedMode::support_current_active_pid`"), TracepointFeatureUnimplemented(feat) => write!(f, "GDB client sent us a tracepoint packet using feature {}, but `gdbstub` doesn't implement it. If this is something you require, please file an issue at https://github.com/daniel5151/gdbstub/issues", *feat as char), TracepointUnsupportedSourceEnumeration => write!(f, "The target doesn't support the gdbstub TracepointSource extension, but attempted to transition to enumerating tracepoint sources"), + MissingMultiThreadSchedulerLocking => write!(f, "GDB requested Scheduler Locking, but the Target does not implement the `MultiThreadSchedulerLocking` IDET"), NonFatalError(_) => write!(f, "Internal non-fatal error. You should never see this! Please file an issue if you do!"), } diff --git a/src/target/ext/base/multithread.rs b/src/target/ext/base/multithread.rs index e2293fd8..640986bd 100644 --- a/src/target/ext/base/multithread.rs +++ b/src/target/ext/base/multithread.rs @@ -134,9 +134,21 @@ pub trait MultiThreadResume: Target { /// GDB client had specified using any of the `set_resume_action_XXX` /// methods. /// - /// Any thread that wasn't explicitly resumed by a `set_resume_action_XXX` - /// method should be resumed as though it was resumed with - /// `set_resume_action_continue`. + /// # Default Resume Behavior + /// + /// By default, any thread that wasn't explicitly resumed by a + /// `set_resume_action_XXX` method should be resumed as though it was + /// resumed with `set_resume_action_continue`. + /// + /// **However**, if [`support_scheduler_locking`] is implemented and + /// [`set_resume_action_scheduler_lock`] has been called for the current + /// resume cycle, this default changes: **unmentioned threads must remain + /// stopped.** + /// + /// [`support_scheduler_locking`]: Self::support_scheduler_locking + /// [`set_resume_action_scheduler_lock`]: MultiThreadSchedulerLocking::set_resume_action_scheduler_lock + /// + /// # Protocol Extensions /// /// A basic target implementation only needs to implement support for /// `set_resume_action_continue`, with all other resume actions requiring @@ -146,14 +158,17 @@ pub trait MultiThreadResume: Target { /// ----------------------------|------------------------------ /// Optimized [Single Stepping] | See [`support_single_step()`] /// Optimized [Range Stepping] | See [`support_range_step()`] + /// [Scheduler Locking] | See [`support_scheduler_locking()`] /// "Stop" | Used in "Non-Stop" mode \* /// /// \* "Non-Stop" mode is currently unimplemented in `gdbstub` /// /// [Single stepping]: https://sourceware.org/gdb/current/onlinedocs/gdb/Continuing-and-Stepping.html#index-stepi /// [Range Stepping]: https://sourceware.org/gdb/current/onlinedocs/gdb/Continuing-and-Stepping.html#range-stepping + /// [Scheduler Locking]: https://sourceware.org/gdb/current/onlinedocs/gdb#index-scheduler-locking-mode /// [`support_single_step()`]: Self::support_single_step /// [`support_range_step()`]: Self::support_range_step + /// [`support_scheduler_locking()`]: Self::support_scheduler_locking /// /// # Additional Considerations /// @@ -233,6 +248,14 @@ pub trait MultiThreadResume: Target { ) -> Option> { None } + + /// Support for [scheduler locking]. + /// + /// [scheduler locking]: https://sourceware.org/gdb/current/onlinedocs/gdb#index-scheduler-locking-mode + #[inline(always)] + fn support_scheduler_locking(&mut self) -> Option> { + None + } } define_ext!(MultiThreadResumeOps, MultiThreadResume); @@ -290,3 +313,26 @@ pub trait MultiThreadRangeStepping: Target + MultiThreadResume { } define_ext!(MultiThreadRangeSteppingOps, MultiThreadRangeStepping); + +/// Target Extension - support for GDB's "Scheduler Locking" mode. +/// See [`MultiThreadResume::support_scheduler_locking`]. +pub trait MultiThreadSchedulerLocking: Target + MultiThreadResume { + /// Configure the target to enable "Scheduler Locking" for the upcoming + /// resume. + /// + /// This method is invoked when the GDB client expects only a specific set + /// of threads to run, while all other threads remain frozen. This behavior + /// is typically toggled in GDB using the `set scheduler-locking on` + /// command. + /// + /// When this method is called, the implementation must ensure that any + /// threads not explicitly resumed via previous `set_resume_action_...` + /// calls **remain stopped**. + /// + /// This prevents any "implicit continue" behavior for unmentioned threads, + /// satisfying GDB's expectation that only the designated threads will + /// advance during the next resume. + fn set_resume_action_scheduler_lock(&mut self) -> Result<(), Self::Error>; +} + +define_ext!(MultiThreadSchedulerLockingOps, MultiThreadSchedulerLocking);