search

daniel5151 / gdbstub

An ergonomic, featureful, and easy-to-integrate implementation of the GDB Remote Serial Protocol in Rust (with no-compromises #![no_std] support)
Other
291 stars 45 forks source link

Issue about a GDB command handling #150

Closed rayc345 closed 1 month ago

rayc345 commented 1 month ago

Hello everyone. I used a GDB client to debug a chip using 'probe-rs' which relies on this library for GDB command processing. I found the GDB client not able to run, and issue lies in this library. When GDB client sends out '$vCont;c:0#12', gdbstub library would regard this as a malformed command. But OpenOCD correctly handles this commands and replies '$T05#b9'.

Here is the full GDB communication:

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="rom" start="0x800*!" length="0x20* "/>\n<memory type="ram" start="0x1000*!" length="0x2800"/>\n<memory type="ram" start="0x2000*!" length="0x4000"/>\n<memory type="ram" start="0x20004000" length="0x1800"/>\n<memory type="ram" start="0x20005800" length="0x2800"/>\n</memory-map>#ed
write: $qXfer:memory-map:read::1c0,ffe#af
read: $mry-map>#01
write: $qXfer:memory-map:read::1c7,ffe#b6
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>armv7</architecture><feature name='org.gnu.gdb.arm.m-profile'><reg name='R0' bitsize='32' type='uint32'/><reg name='R1' bitsize='32' type='uint32'/><reg name='R2' bitsize='32' type='uint32'/><reg name='R3' bitsize='32' type='uint32'/><reg name='R4' bitsize='32' type='uint32'/><reg name='R5' bitsize='32' type='uint32'/><reg name='R6' bitsize='32' type='uint32'/><reg name='R7' bitsize='32' type='uint32'/><reg name='R8' bitsize='32' type='uint32'/><reg name='R9' bitsize='32' type='uint32'/><reg name='R10' bitsize='32' type='uint32'/><reg name='R11' bitsize='32' type='uint32'/><reg name='R12' bitsize='32' type='uint32'/><reg name='SP' bitsize='32' type='data_ptr'/><reg name='LR' bitsize='32' type='uint32'/><reg name='PC' bitsize='32' type='code_ptr'/><reg name='MSP' bitsize='32' type='uint32'/><reg name='PSP' bitsize='32' type='uint32'/><reg name='XPSR' bitsize='32' type='uint32'/><reg name='EXTRA' bitsize='32' type='uint32'/><reg name='XPSR' bitsize='32' type='uint32'/></feature><feature name='org.gnu.gdb.arm.m-system'><reg name='MSP' bitsize='32' type='uint32'/><reg name='PSP' bitsize='32' type='uint32'/></feature><feature name='org.gnu.gdb.arm.vfp'><reg name='d0' bitsize='64' type='ieee_double'/><reg name='d1' bitsize='64' type='ieee_double'/><reg name='d2' bitsize='64' type='ieee_double'/><reg name='d3' bitsize='64' type='ieee_double'/><reg name='d4' bitsize='64' type='ieee_double'/><reg name='d5' bitsize='64' type='ieee_double'/><reg name='d6' bitsize='64' type='ieee_double'/><reg name='d7' bitsize='64' type='ieee_double'/><reg name='d8' bitsize='64' type='ieee_double'/><reg name='d9' bitsize='64' type='ieee_double'/><reg name='d10' bitsize='64' type='ieee_double'/><reg name='d11' bitsize='64' type='ieee_double'/><reg name='d12' bitsize='64' type='ieee_double'/><reg name='d13' bitsize='64' type='ieee_double'/><reg name='d14' bitsize='64' type='ieee_double'/><reg name='d15' bitsize='64' type='ieee_double'/><reg name='FPSCR' bitsize='32' type='uint32'/></feature></target>#f1
write: $qXfer:features:read:target.xml:843,ffe#eb
read: $mature></target>#fc
write: $qXfer:features:read:target.xml:852,ffe#eb
read: $l#6c
write: $me000ed08,4#f3
read: $000*!8#13
write: $m10000010,4#4f
read: $0300*!#0e
write: $m10000014,4#53
read: $0300*!#0e
write: $m10000018,18#8c
read: $170f0008b80* 10* 40* f6030*0#bc
write: $Z1,800021a,2#a1
read: $OK#9a
write: $Z1,80001d8,2#aa
read: $OK#9a
write: $G00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000fa400000fffffffffa40000000000000f77f000000008e65f77f00006e020000b702000040379e0663010000200d5e06630100009011e30663010000010cbb0a6301000040379e066301000010db870b6301000040379e0663010000d0c9cd0e63010000f13dd165f77f000040379e0600000000#7d
read: $OK#9a
write: $vCont;c:0#12
write: $m10000010,4#4f
daniel5151 commented 1 month ago

Huh, that's very interesting indeed! And thank you for including a full packet log - that is very useful!

Can you please also share what version of the GDB client you are using? Also, can you include a transcript of any GDB client commands you sent (if any) to generate this packet log?

Poking around gdbstub's current implementation, it seems that past-me assumed that a '0' ThreadId (AKA: "pick any thread") wouldn't actually be valid in the context of vCont. I assumed this would be reasonable, as why would GDB send over an "ambiguous" thread id, when it could query what threads are live itself, and pick a specific TID (or, at the very least, pass '-1' to resume all threads)?

As such - the current code in the vCont packet parser discards these packets as malformed.

...but if GDB itself is sending over these sorts of packets, then it does seem that gdbstub will need to be updated to properly handle this.

rayc345 commented 1 month ago

Hello Daniel: Thank you for your reply! The GDB client I use is not a standalone GDB client, it's part of an IDE named 'Segger Embedded Studio', produced by Segger GmbH. This is not an open-source implementation of GDB client. The software is used for MCU development, debugging etc. The example I provided above was performed on a STM32F1 MCU which has an Arm Cortex-M3 CPU core inside, which runs bare-metal firmware with only one hardware thread. I attached the debugger to the MCU successfully, and I got the error message right after I clicked the 'start run' button.

rayc345 commented 1 month ago

I currently mitigate this issue by change the source code of 'IdKind::Any => return Err(()),' to 'IdKind::Any => SpecificIdKind::All,' in thread_id.rs line 110 of version 0.7.1

daniel5151 commented 1 month ago

This is not an open-source implementation of GDB client

That certainly raises an eyebrow for me, and makes me think gdbstub might not be so wrong here after all...

That said - I'm OK with being a bit pragmatic with the implementation, and tweaking gdbstub to handle this somewhat ambiguous packet in a reasonable manner.

I currently mitigate this issue by change the source code of 'IdKind::Any => return Err(()),' to 'IdKind::Any => SpecificIdKind::All,' in thread_id.rs line 110 of version 0.7.1

This was precisely the workaround I was going to propose, so if this works for you, I'll go ahead and land that in-tree, and push out the fix as part of 0.7.2

daniel5151 commented 1 month ago

@rayc345, I've pushed up a slight variant on the proposed fix to master.

Could you test that this new version works for you? Once you've confirmed the fix, I'll go ahead and publish 0.7.2 to crates.io :)

rayc345 commented 1 month ago

Thanks Danial. With this patch, the connect problem with Segger Embedded Studio is fixed, thanks. I still have some questions about the implementation of gdbstub, I will open new issues since they are not relevant to this issue.

daniel5151 commented 1 month ago

Thanks for testing!

Marking as closed, via e9a5296

daniel5151 commented 1 month ago

gdbstub 0.7.2 has been published to crates.io, which includes this fix :)