Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue about a GDB command handling #150

Closed
rayc345 opened this issue Aug 10, 2024 · 8 comments
Closed

Issue about a GDB command handling #150

rayc345 opened this issue Aug 10, 2024 · 8 comments

Comments

@rayc345
Copy link

rayc345 commented Aug 10, 2024

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
Copy link
Owner

daniel5151 commented Aug 10, 2024

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
Copy link
Author

rayc345 commented Aug 11, 2024

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
Copy link
Author

rayc345 commented Aug 11, 2024

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
Copy link
Owner

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 added a commit that referenced this issue Aug 11, 2024
Related to issue #150

See inline comment in _vCont.rs for more details on this change.
@daniel5151
Copy link
Owner

@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
Copy link
Author

rayc345 commented Aug 12, 2024

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
Copy link
Owner

Thanks for testing!

Marking as closed, via e9a5296

@daniel5151
Copy link
Owner

daniel5151 commented Aug 17, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants