Support vCont packets with '0' thread-id in single-threaded operation

[rayc345]

When using probe-rs with Segger Embedded Studio to debug a MCU, gdbstub seems not able to recognize some commands.
"Client sent an unexpected packet. This should never happen! Please re-run with log trace-level logging enabled and file an issue at https://github.com/daniel5151/gdbstub/issues"
It seems the issue lines in '$vCont;s:0#22'
In resume.rs:206 seems to be the origin.
GDBstub uses the latest version from GitHub repository.
Here is the GDB comm:

write: $qSupported:vContSupported+#6c
read: $PacketSize=1000;vContSupported+;multiprocess+;QStartNoAckMode+;hwbreak+;qXfer:features:read+;qXfer:memory-map:read+#72
write: $vMustReplyEmpty#3a
read: $#00
write: $QStartNoAckMode#b0
read: $OK#9a
write: $!#21
read: $#00
write: $qXfer:memory-map:read::0,ffe#1b
read: $m<?xml version="1.0"?>[0a]<!DOCTYPE memory-map PUBLIC "+//IDN gnu.org//DTD GDB Memory Map V1.0//EN" "http://sourceware.org/gdb/gdb-memory-map.dtd">[0a]<memory-map>[0a]<memory type="ram" start="0x0" length="0x20000"/>\n<memory type="ram" start="0x80000" length="0x20000"/>\n<memory type="rom" start="0x80000000" length="0x100000"/>\n<memory type="ram" start="0xf0400000" length="0x8000"/>\n</memory-map>#55
write: $qXfer:memory-map:read::188,ffe#8c
read: $l#6c
write: $qXfer:features:read:target.xml:0,ffe#7c
read: $m<?xml version="1.0"?>[0a]        <!DOCTYPE target SYSTEM "gdb-target.dtd">[0a]        <target version="1.0">[0a]        <architecture>riscv:rv32</architecture><feature name='org.gnu.gdb.riscv.cpu'><reg name='x0' bitsize='32' type='uint32'/><reg name='x1' bitsize='32' type='uint32'/><reg name='x2' bitsize='32' type='uint32'/><reg name='x3' bitsize='32' type='uint32'/><reg name='x4' bitsize='32' type='uint32'/><reg name='x5' bitsize='32' type='uint32'/><reg name='x6' bitsize='32' type='uint32'/><reg name='x7' bitsize='32' type='uint32'/><reg name='x8' bitsize='32' type='uint32'/><reg name='x9' bitsize='32' type='uint32'/><reg name='x10' bitsize='32' type='uint32'/><reg name='x11' bitsize='32' type='uint32'/><reg name='x12' bitsize='32' type='uint32'/><reg name='x13' bitsize='32' type='uint32'/><reg name='x14' bitsize='32' type='uint32'/><reg name='x15' bitsize='32' type='uint32'/><reg name='x16' bitsize='32' type='uint32'/><reg name='x17' bitsize='32' type='uint32'/><reg name='x18' bitsize='32' type='uint32'/><reg name='x19' bitsize='32' type='uint32'/><reg name='x20' bitsize='32' type='uint32'/><reg name='x21' bitsize='32' type='uint32'/><reg name='x22' bitsize='32' type='uint32'/><reg name='x23' bitsize='32' type='uint32'/><reg name='x24' bitsize='32' type='uint32'/><reg name='x25' bitsize='32' type='uint32'/><reg name='x26' bitsize='32' type='uint32'/><reg name='x27' bitsize='32' type='uint32'/><reg name='x28' bitsize='32' type='uint32'/><reg name='x29' bitsize='32' type='uint32'/><reg name='x30' bitsize='32' type='uint32'/><reg name='x31' bitsize='32' type='uint32'/><reg name='pc' bitsize='32' type='code_ptr'/><reg name='pc' bitsize='32' type='code_ptr'/></feature></target>#c9
write: $qXfer:features:read:target.xml:69f,ffe#21
read: $l#6c
write: $g#67
read: $0000000000000000a0ffffff0000000000000000000000000000000000000000000000000000000003000000a800000000000000000000008913000002000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000#c8
write: $mfffffff8,c#fe
read: $E7a#dd
write: $m0,100#5a
read: $E7a#dd
write: $m0,4#fd
read: $E7a#dd
write: $P0=00000000#3d
read: $OK#9a
write: $P1=00000000#3e
read: $OK#9a
write: $P2=00000000#3f
read: $OK#9a
write: $P3=00000000#40
read: $OK#9a
write: $P4=00000000#41
read: $OK#9a
write: $P5=00000000#42
read: $OK#9a
write: $P6=00000000#43
read: $OK#9a
write: $P7=00000000#44
read: $OK#9a
write: $P8=00000000#45
read: $OK#9a
write: $P9=00000000#46
read: $OK#9a
write: $Pa=00000000#6e
read: $OK#9a
write: $Pb=00000000#6f
read: $OK#9a
write: $Pc=00000000#70
read: $OK#9a
write: $Pd=00000000#71
read: $OK#9a
write: $Pe=00000000#72
read: $OK#9a
write: $Pf=00000000#73
read: $OK#9a
write: $P10=00000000#6e
read: $OK#9a
write: $P11=00000000#6f
read: $OK#9a
write: $P12=00000000#70
read: $OK#9a
write: $P13=00000000#71
read: $OK#9a
write: $P14=00000000#72
read: $OK#9a
write: $P15=00000000#73
read: $OK#9a
write: $P16=00000000#74
read: $OK#9a
write: $P17=00000000#75
read: $OK#9a
write: $P18=00000000#76
read: $OK#9a
write: $P19=00000000#77
read: $OK#9a
write: $P1a=00000000#9f
read: $OK#9a
write: $P1b=00000000#a0
read: $OK#9a
write: $P1c=00000000#a1
read: $OK#9a
write: $P1d=00000000#a2
read: $OK#9a
write: $P1e=00000000#a3
read: $OK#9a
write: $P1f=00000000#a4
read: $OK#9a
write: $P20=00000000#6f
read: $OK#9a
write: $vCont;s:0#22

[daniel5151]

Wow, Segger Embedded Studio really likes to do weird stuff with the GDB RSP, eh?

This is very similar to what we talked about in #150, insofar that vCont;s;0 really shouldn't be a valid packet, as the request is to single step "some" core, which isn't a particularly reasonable request in any scenario where your device has multiple cores...

Then again - given that you're debugging an MCU, can I assume that you've only got a single CPU?

If so, maybe I can fix gdbstub to allow handling '0' thread-id requests in the case where there is only a single live thread (and therefore, there wouldn't be any ambiguity regarding which thread needs to be resumed).

Let me know if that is the case for you.

---

Also, meta-note: can you encapsulate packet transcripts in a markdown code block? I've been editing your messages manually to do that, but it'd be nice if you could do it yourself 😅

[rayc345]

Hello Daniel
Thank you for your reply.
Yeah, it's a single-core MCU. I may need to talk with segger about their implementation in the GDB client. Since there are many multi core MCUs, it would be ambiguous in this situation.

I would use markdown in the future, thanks for your reminding! I'm not familiar with it now.

[daniel5151]

Ok, in that case, here is my proposed solution:

• If gdbstub receives a vCont packet with a '0' thread-id, instead of immediately rejecting it (as it does today), it should first query the Target to see if it has more than one thread
  • If the Target is using SingleThreadBase, it is trivially true that it has a single thread
  • If the Target is using MultiThreadBase, we can query the list_active_threads method to determine if there is a single active thread (and what thread-id it has)
• If we detect the target has more than one active thread
  • Report an error (no way to handle the ambiguous request)
• If the target is confirmed to be single-threaded
  • gdbstub should be able to unambiguously handle the request

We'll also need to revert e9a5296, and moreover, convert the vCont packet parser from using SpecificThreadId, back to using a regular ThreadId (so that we can plumb the '0' thread-id to the vCont handler code).

I don't actually think this is a big lift, and if I find some time, I could probably hack this together myself.
...unfortunately, I'll be going on vacation in a few days, and will be gone for a few weeks.

Let me know if you'd like to take a stab at sending in a PR to fix this issue, or if you'd prefer to wait a bit until I find a moment to address this issue myself

[rayc345]

Hello, Daniel
I have proposed an issue thread in Segger user forum, so their engineers would read it and may add it to their 'to do's.
I think the best option is both GDB client and server should be more robust, your proposal is very nice, it would work if other users use a similar GDB client. But it would be better if client does not send 0 thread id.
I'm new to rust so I'm afraid I cannot provide much help, I'm still a freshman in this field, but I would try.

[daniel5151]

No worries if you can't lend a hand with the implementation!
Maybe I'll have a chance to hack away at this during some downtime.

---

And thanks for reporting it to Segger! At this point, I think I've convinced myself that gdbstub should be able to properly respond to this packet in the single-threaded case (if only for compat with existing clients), but that it should still continue to be an error in the multithreaded case.

The only possible counter-point I see is that one could support this packet in multi-threaded mode by using the "currently selected resume thread" set by the 'H' packet, but even so, the docs for the packet clearly state (emphasis mine):

Set thread for subsequent operations (‘m’, ‘M’, ‘g’, ‘G’, et.al.). Depending on the operation to be performed, op should be ‘c’ for step and continue operations (note that this is deprecated, supporting the ‘vCont’ command is a better option), and ‘g’ for other operations. The thread designator thread-id has the format and interpretation described in thread-id syntax.

...which implies to me that if a client chooses to use vCont, it can't rely on the '0' thread-id corresponding the set thread from the 'H' packet.

Anyways - I'll be interested to see if the folks at Segger think here :)