gdbstub icon indicating copy to clipboard operation
gdbstub copied to clipboard

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

Open rayc345 opened this issue 1 year ago β€’ 7 comments

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

rayc345 avatar Aug 18 '24 14:08 rayc345

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 πŸ˜…

daniel5151 avatar Aug 18 '24 15:08 daniel5151

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.

rayc345 avatar Aug 19 '24 01:08 rayc345

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 e9a5296c, 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

daniel5151 avatar Aug 19 '24 16:08 daniel5151

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.

rayc345 avatar Aug 20 '24 02:08 rayc345

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 :)

daniel5151 avatar Aug 20 '24 16:08 daniel5151

I am here not directly with this issue, but with vCont in general, I am having an issue with "next" in gdb, when nexting over functions with deep inner loops. Without vCont, it starts stepping and sits there spinning for extremely long periods of time (minutes). When I just want to go "next".

But if I stub in vCont, say by replying with vCont;c;C;s;S, and then I "next" in gdb, I Get vCont;s:1;c#

I can't figure out what GDB is trying to get me to do there. Some people online are saying that's a single step, but if I do single step with that, it just super spins. It doesn't make sense to me... How does gdb avoid spending minutes stepping through with other gdb servers?

If I interpret vCont;s:1;c as step once then continue, it runs for a bit then gdb breaks execution later. If I interpret vCont;s:1;c as single-step, the behavior is no different than if I make no changes, and gdb just spins...

cnlohr avatar Nov 22 '24 07:11 cnlohr

@cnlohr, lets move this side-thread over to #158

daniel5151 avatar Nov 23 '24 18:11 daniel5151