MultiThreadSchedulerLocking support #178

[Satar07]

Description

This PR implements a new Protocol Extension IDET: MultiThreadSchedulerLocking.

Closes #178

Changes

• Introduced MultiThreadSchedulerLocking IDET under MultiThreadResume.
• Updated do_vcont_multi_thread to track the presence of wildcard/default continue actions.
• If no wildcard continue is found (indicating GDB expects scheduler locking), gdbstub now invokes set_resume_action_scheduler_lock.
• If the target does not implement this IDET but GDB requests locking, gdbstub returns a fatal PacketUnexpected error to avoid silent race conditions (as discussed in issue Expose additional vCont semantics to support scheduler-locking modes #178).
• Updated the armv4t_multicore example to demonstrate a working scheduler-locking implementation.

API Stability

• This PR does not require a breaking API change.

Checklist

• Documentation
  • Ensured any public-facing rustdoc formatting looks good
• Validation
  • Included output of running examples/armv4t_multicore
  • Included output of running ./example_no_std/check_size.sh
• If implementing a new protocol extension IDET
  • Included a basic sample implementation in examples/armv4t_multicore
  • IDET can be optimized out (confirmed via ./example_no_std/check_size.sh)

---

Validation

1. Functional Validation (armv4t_multicore)

Tested with GDB 15.0. Verified that when set scheduler-locking on is used, only the selected thread steps, while the other remain frozen at its last PC.

GDB Session Output

Code snippet

0x55550000 in ?? ()
(gdb) info threads
  Id   Target Id            Frame
* 1    Thread 1.1 (CPU Cpu) 0x55550000 in ?? ()
  2    Thread 1.2 (CPU Cop) 0x55550000 in ?? ()
(gdb) b*0x55550000+0x30
Breakpoint 1 at 0x55550030
(gdb) c
Continuing.

Thread 1 hit Breakpoint 1, 0x55550030 in ?? ()
(gdb) info threads
  Id   Target Id            Frame
* 1    Thread 1.1 (CPU Cpu) 0x55550030 in ?? ()
  2    Thread 1.2 (CPU Cop) 0x55550044 in ?? ()
(gdb) set scheduler-locking on
(gdb) x/10i $pc
=> 0x55550030:  ldr     r3, [r11, #-16]
   0x55550034:  cmp     r3, #0
   0x55550038:  beq     0x55550030
   0x5555003c:  ldr     r3, [r11, #-8]
   0x55550040:  b       0x55550080
   0x55550044:  mov     r3, #0
   0x55550048:  str     r3, [r11, #-12]
   0x5555004c:  b       0x55550068
   0x55550050:  ldr     r3, [r11, #-8]
   0x55550054:  add     r3, r3, #1
(gdb) b*0x55550038
Breakpoint 3 at 0x55550038
(gdb) c
Continuing.

Thread 1 hit Breakpoint 3, 0x55550038 in ?? ()
(gdb) info threads
  Id   Target Id            Frame
* 1    Thread 1.1 (CPU Cpu) 0x55550038 in ?? ()
  2    Thread 1.2 (CPU Cop) 0x55550044 in ?? ()

2. Binary Size Check (./example_no_std/check_size.sh)

Section	master	fix/resume_multi	Diff
.text	14128	14156	+28 bytes
Total	18586	18614	+28 bytes

check_size Output

before:

File  .text    Size          Crate Name
2.1%  62.1%  8.6KiB      [Unknown] main
0.2%   5.4%    764B        gdbstub gdbstub::stub::state_machine::GdbStubStateMachineInner<gdbstub::stub::state_machine:...
0.1%   2.4%    336B        gdbstub gdbstub::protocol::commands::breakpoint::BasicBreakpoint::from_slice
0.1%   2.3%    320B        gdbstub <gdbstub::protocol::common::thread_id::ThreadId as core::convert::TryFrom<&[u8]>>::t...
0.1%   2.2%    308B        gdbstub gdbstub::protocol::common::hex::decode_hex_buf
0.1%   2.0%    284B        gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::write
0.1%   1.7%    236B        gdbstub gdbstub::stub::core_impl::resume::<impl gdbstub::stub::core_impl::GdbStubImpl<T,C>>:...
0.1%   1.6%    232B        gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::write_specific_thread_id
0.1%   1.5%    216B           core core::iter::traits::iterator::Iterator::nth
0.1%   1.5%    216B           core core::iter::traits::iterator::Iterator::nth
0.0%   1.1%    160B        gdbstub gdbstub::protocol::common::hex::decode_hex
0.0%   1.1%    156B        gdbstub gdbstub::protocol::common::hex::decode_hex
0.0%   1.1%    156B        gdbstub gdbstub::protocol::common::hex::decode_hex
0.0%   1.1%    156B        gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::write_num
0.0%   1.1%    152B        gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::inner_write
0.0%   1.0%    144B        gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::write_num
0.0%   1.0%    140B        gdbstub <gdbstub::protocol::common::thread_id::IdKind as core::convert::TryFrom<&[u8]>>::try...
0.0%   0.9%    132B        gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::flush
0.0%   0.9%    128B        gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::write_hex
0.0%   0.8%    116B gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiTh...
0.0%   0.8%    108B           core <core::iter::adapters::skip::Skip<I> as core::iter::traits::iterator::Iterator>::next
0.0%   0.7%    104B           core <core::slice::iter::SplitMut<T,P> as core::iter::traits::iterator::Iterator>::next
0.0%   0.5%     76B gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiTh...
0.0%   0.5%     76B gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiTh...
0.0%   0.5%     76B gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiTh...
0.0%   0.5%     68B   gdbstub_arch <gdbstub_arch::arm::reg::arm_core::ArmCoreRegs as gdbstub::arch::Registers>::gdb_des...
0.0%   0.4%     52B gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiTh...
0.0%   0.4%     52B gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiTh...
0.0%   0.4%     52B gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiTh...
0.0%   0.4%     52B      [Unknown] _start
0.0%   0.3%     48B  gdbstub_nostd gdbstub_nostd::print_str::print_str
0.0%   0.1%     20B      [Unknown] call_weak_fn
0.0%   0.1%     12B gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::breakpoints::SwBreakpoint>...
3.3% 100.0% 13.8KiB                .text section size, the file size is 413.5KiB
target/release/gdbstub-nostd  :
section               size     addr
.interp                 27      568
.note.gnu.build-id      36      596
.note.ABI-tag           32      632
.gnu.hash               28      664
.dynsym                432      696
.dynstr                198     1128
.gnu.version            36     1326
.gnu.version_r          48     1368
.rela.dyn              192     1416
.rela.plt              312     1608
.init                   24     1920
.plt                   240     1952
.text                14128     2240
.fini                   20    16368
.rodata                670    16388
.eh_frame_hdr          300    17060
.eh_frame             1072    17360
.init_array              8   130384
.fini_array              8   130392
.dynamic               496   130400
.got                   176   130896
.data                    8   131072
.bss                     8   131080
.comment                87        0
Total                18586

after:

File  .text    Size          Crate Name
2.1%  62.2%  8.6KiB      [Unknown] main
0.2%   5.4%    764B        gdbstub gdbstub::stub::state_machine::GdbStubStateMachineInner<gdbstub::stub::state_machine:...
0.1%   2.4%    336B        gdbstub gdbstub::protocol::commands::breakpoint::BasicBreakpoint::from_slice
0.1%   2.3%    320B        gdbstub <gdbstub::protocol::common::thread_id::ThreadId as core::convert::TryFrom<&[u8]>>::t...
0.1%   2.2%    308B        gdbstub gdbstub::protocol::common::hex::decode_hex_buf
0.1%   2.0%    284B        gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::write
0.1%   1.7%    236B        gdbstub gdbstub::stub::core_impl::resume::<impl gdbstub::stub::core_impl::GdbStubImpl<T,C>>:...
0.1%   1.6%    232B        gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::write_specific_thread_id
0.1%   1.5%    216B           core core::iter::traits::iterator::Iterator::nth
0.1%   1.5%    216B           core core::iter::traits::iterator::Iterator::nth
0.0%   1.1%    160B        gdbstub gdbstub::protocol::common::hex::decode_hex
0.0%   1.1%    156B        gdbstub gdbstub::protocol::common::hex::decode_hex
0.0%   1.1%    156B        gdbstub gdbstub::protocol::common::hex::decode_hex
0.0%   1.1%    156B        gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::write_num
0.0%   1.1%    152B        gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::inner_write
0.0%   1.0%    144B        gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::write_num
0.0%   1.0%    140B        gdbstub <gdbstub::protocol::common::thread_id::IdKind as core::convert::TryFrom<&[u8]>>::try...
0.0%   0.9%    132B        gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::flush
0.0%   0.9%    128B        gdbstub gdbstub::protocol::response_writer::ResponseWriter<C>::write_hex
0.0%   0.8%    116B gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiTh...
0.0%   0.8%    108B           core <core::iter::adapters::skip::Skip<I> as core::iter::traits::iterator::Iterator>::next
0.0%   0.7%    104B           core <core::slice::iter::SplitMut<T,P> as core::iter::traits::iterator::Iterator>::next
0.0%   0.5%     76B gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiTh...
0.0%   0.5%     76B gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiTh...
0.0%   0.5%     76B gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiTh...
0.0%   0.5%     68B   gdbstub_arch <gdbstub_arch::arm::reg::arm_core::ArmCoreRegs as gdbstub::arch::Registers>::gdb_des...
0.0%   0.4%     52B gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiTh...
0.0%   0.4%     52B gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiTh...
0.0%   0.4%     52B gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::base::multithread::MultiTh...
0.0%   0.4%     52B      [Unknown] _start
0.0%   0.3%     48B  gdbstub_nostd gdbstub_nostd::print_str::print_str
0.0%   0.1%     20B      [Unknown] call_weak_fn
0.0%   0.1%     12B gdbstub_nostd? <gdbstub_nostd::gdb::DummyTarget as gdbstub::target::ext::breakpoints::SwBreakpoint>...
3.3% 100.0% 13.8KiB                .text section size, the file size is 414.2KiB
target/release/gdbstub-nostd  :
section               size     addr
.interp                 27      568
.note.gnu.build-id      36      596
.note.ABI-tag           32      632
.gnu.hash               28      664
.dynsym                432      696
.dynstr                198     1128
.gnu.version            36     1326
.gnu.version_r          48     1368
.rela.dyn              192     1416
.rela.plt              312     1608
.init                   24     1920
.plt                   240     1952
.text                14156     2240
.fini                   20    16396
.rodata                670    16416
.eh_frame_hdr          300    17088
.eh_frame             1072    17392
.init_array              8   130384
.fini_array              8   130392
.dynamic               496   130400
.got                   176   130896
.data                    8   131072
.bss                     8   131080
.comment                87        0
Total                18614

[daniel5151]

Let me know if you'd like me to take a look here. I saw you opened it as a draft PR, so thought I'd ask before leaving comments

[Satar07]

@daniel5151 Please go ahead, I'd appreciate the feedback. It's currently running well in my project. I'll mark this as ready.

(Apologies for any slow replies—I'm currently busy with final exams.)

[daniel5151]

Great work! Overall design looks good, so after we fix up a few minor doc and error-handling related things, we should be gtg here.

Also, like I mentioned in #178 (comment), it'd be good to double-check that the approach we're talking here lines up with how the GDB client (and maybe gdbserver) handle scheduler-locking.

I'll see if I can find some time to poke around https://github.com/bminor/binutils-gdb myself, but would def appreciate it if you could poke around a bit as part of this diff.

---

Best of luck with exams! I'm also somewhat busy this week (home for the holidays and whatnot), so no need to rush or anything.

[daniel5151]

Also, like I mentioned in #178 (comment), it'd be good to double-check that the approach we're talking here lines up with how the GDB client (and maybe gdbserver) handle scheduler-locking.

I nerd sniped myself, so I did some digging here lol.

https://github.com/bminor/binutils-gdb/blob/b0196f0/gdb/remote.c#L960-L961

It looks like GDB unconditionally assumes that the remote target supports scheduler-locking, which seems... not ideal. Interestingly enough - I chased the git blame chain to see when this default was introduced, and it looks like it stretches waaaaaaaaaay back into the past (I'm talking before 1999, and even before the initial commit that migrated GDB over to git). Check it out: https://github.com/bminor/binutils-gdb/blob/c906108c21474dfb4ed285bcc0ac6fe02cd400cc/gdb/remote.c#L3389

So, if you want some bonus points - you might want to consider opening an issue (or even sending in a PR?) to GDB that points out this behavior, and that it might be more user-friendly if GDB negotiated support for scheduler-locking on remote targets via a qSupported feature.

[Satar07]

@daniel5151 First off, thanks for the deep dive! Reading that massive amount of ancient C code is honestly painful to look at 😂.

I’ve been thinking about this, and I’m actually not sure if opening an issue or PR against GDB is the right move here. Since the Remote Serial Protocol uses wildcards in its data packets to convey information, there seems to be an implicit assumption that the remote server should possess the capability to control threads/processes to that degree. In that sense, the hardcoded behavior (defaulting to assuming support) actually seems reasonable.

I noticed that for local targets (non-remote), there is indeed code to handle TARGET_DEFAULT_RETURN (tc_none): https://github.com/bminor/binutils-gdb/blob/47049598bd189cd504cb12eaec3b3736f452490a/gdb/target.h#L736

However, regarding remote targets, it seems tied to how packets like c:-1 are transmitted. If we were to change this, it feels like it would require a massive architectural shift. If the target doesn't support locking, GDB would effectively be forced to default to sending -1 (all threads) requests and forbidden from sending packets describing specific threads. That feels a bit odd and seems deeply rooted in the protocol's original design.

That said, I do think the current redesign of gdbstub is great. I originally planned to use it in my project to enable multi-thread debugging, but I've hit a wall. I realized my implementation lacks the ability to perform fine-grained thread control.

I tried to control execution purely by sending signals to specific threads/processes to stop and continue them. However, after researching, I learned that even with tgkill (the syscall to target a specific thread), SIGSTOP and SIGCONT still affect the entire process on Linux. Relying on signals alone isn't enough to achieve scheduler locking (it really does require ptrace). Given this, I might just have to identify this scenario and issue a warning, or simply not implement the trait to indicate that the feature isn't supported.

Also, regarding extern bool target_can_lock_scheduler ();: I traced the code and found that while this function is configured, it doesn't seem to be called in a way that checks the backend. It looks like scheduler-locking is treated purely as a client-side state in GDB, with the unconditional assumption that the debugger backend has the means to handle it.

[daniel5151]

Code looks good! I think we are good to land it. Thank you for your contribution!

Let me know if you'd like me to cut a new gdbstub release to crates.io that includes this feature. If there's no rush - I might sit on it a while, and bundle it alongside some future version bump.

---

(this section is me musing about whether upstream GDB is doing things in a reasonable way or not - nothing below blocks landing this PR)

Also, regarding extern bool target_can_lock_scheduler ();: I traced the code and found that while this function is configured, it doesn't seem to be called in a way that checks the backend. It looks like scheduler-locking is treated purely as a client-side state in GDB, with the unconditional assumption that the debugger backend has the means to handle it.

Indeed, I also saw that this was a purely client-side concept.

If the target doesn't support locking, GDB would effectively be forced to default to sending -1 (all threads) requests and forbidden from sending packets describing specific threads. That feels a bit odd and seems deeply rooted in the protocol's original design.

I'm not sure I follow here - the behavior you claim is "forbidden" is exactly what GDB does today, no? i.e: sending a vCont packet that includes thread-specific instructions alongside a c:pPID.-1 trailing wildcard (when locking is off)?

there seems to be an implicit assumption that the remote server should possess the capability to control threads/processes to that degree.

Yeah, my point is that this is a bogus assumption.

My (conjectured) read on the situation is this: Given the relatively "low-stakes" failure mode of trying to enable scheduler locking on a remote target that doesn't support it (i.e: "oh, weird, my threads didn't all stop - I guess this remote doesn't support scheduler locking. oh well"), there wasn't ever much reason to invest in extra infrastructure to do proper client/remote negotiation of target_can_lock_scheduler.

An interesting data-point to confirm this theory would be to look into what the upstream gdbserver does when a client GDB enables scheduler locking on a remote target that doesn't support it (either mapping out the code, or empirically testing the behavior on a particular target). My guess is that it just "does something reasonable" instead of erroring out (which, much to my chagrin, is typically the case with these older C codebases).

And yet another point: look at this commit that landed which tweaked the vCont packet language back in 2016: bminor/binutils-gdb@ca6eff5

This is once again a bit hard to interpret... but if you squint, it seems like it used to say that threads "should" remain stopped in all-stop mode. This could be a weasel-word that implies "but it might not always happen". This is just conjecture of course.

The new phrasing seems a bit too aggressive IMO, since there's no way for the GDB client to actually force all remote targets to support scheduler-locking on correctly

^and really, that last sentence is the crux of my annoyance here.

The fact GDB unconditionally assumes all remote targets support fine-grained scheduling is silly, and the failure mode is - as you've experienced first hand - silent, and subtly wrong behavior. Ideally, there should be some checks/validation (i.e: feature negotiation) between the client and server that allows the GDB client to gate the use of scheduler-locking on the client-side, instead of forcing gdb stubs (like us) to infer this mismatch and error out server-side.