Msp430 fixes

[mchesser]

Description

The previous MSP430 register definition in gdbstub_arch was missing a GPR.

This PR adds the missing register as well as adding support for 20-bit MSP430 variant (CPUX) using the paramaterization strategy from the MIPS implementation, note that gdb treats the 20-bit registers as 32-bit registers internally.

I tested both the 16-bit variant and the 20-bit variant with my emulator, and things seemed to work. Backtraces seem to be broken but I don't think that is related gdbstub (The TI compiler was used to compile the binary I was testing, I think it could be an upstream GDB issue with the TI calling convention).

Technically this is breaking API change, however, since the previous implementation would have resulted in R15 being dropped, it seems unlikely it is being used.

Checklist

• Implementation
  • cargo build compiles without errors or warnings
  • cargo clippy runs without errors or warnings
  • cargo fmt was run
  • All tests pass
• Documentation
  • rustdoc + appropriate inline code comments
  • Updated CHANGELOG.md

Debug output

Gdb output

Remote debugging using :9999
warning: Can not parse XML target description; XML support was disabled at compile time
_c_int00_noargs () at boot.c:134
134     boot.c: No such file or directory.
(gdb) break main
Breakpoint 1 at 0x10bc4: file ../main.c, line 72.
(gdb) c
Continuing.

Breakpoint 1, main () at ../main.c:72
72      {
(gdb) info r
pc             0x10bc4  0x10bc4 <main>
sp             0x43fc   17404
sr             0x3      3
cg             <unavailable>
r4             0x0      0
r5             0x0      0
r6             0x0      0
r7             0x0      0
r8             0x0      0
r9             0x0      0
r10            0x0      0
r11            0xfff    4095
r12            0x0      0
r13            0x24f7   9463
r14            0x0      0
r15            0x0      0
(gdb) bt
#0  main () at ../main.c:72
(gdb) q
A debugging session is active.

        Inferior 1 [process 1] will be detached.

Quit anyway? (y or n) y
Detaching from program: D:\src\fuzzing\sysroots\msp430\H3_EchoToHost.out, process 1
Ending remote debugging.

Protocol output

w +$qSupported:multiprocess+;swbreak+;hwbreak+;qRelocInsn+;fork-events+;vfork-events+;exec-events+;vContSupported+;QThreadEvents+;no-resumed+#df
r +$PacketSize=1000;vContSupported+;multiprocess+;QStartNoAckMode+;ReverseStep+;swbreak+;qXfer:features:read+#db
w +$vMustReplyEmpty#3a
r +$#00
w +$QStartNoAckMode#b0
r +$OK#9a
w +$Hgp0.0#ad
r $OK#9a
w $qXfer:features:read:target.xml:0,ffb#79
r $l<target version="1.0"><architecture>msp430x</architecture></target>#5b
w $qTStatus#49
r $#00
w $?#3f
r $S05#b8
w $qfThreadInfo#bb
r $mp01.01#cd
w $qsThreadInfo#c8
r $l#6c
w $qAttached:1#fa
r $1#31
w $Hc-1#09
r $OK#9a
w $qC#b4
r $#00
w $qOffsets#4b
r $#00
w $g#67
r $5c450*0x*$0*|#27
w $qfThreadInfo#bb
r $mp01.01#cd
w $qsThreadInfo#c8
r $l#6c
w $qSymbol::#5b
r $#00
c break main
w $m10bc4,1#24
r $f1#97
w $m10bc5,1#25
r $03#63
w $m10bc4,1#24
r $f1#97
w $m10bc5,1#25
r $03#63
w $m10bc4,2#25
r $f103#fa
c c
w $Z0,10bc4,2#6e
r $OK#9a
w $vCont?#49
r $vCont;c;C;s;S#62
w $vCont;c:p1.-1#0f
r <Timeout: 0 seconds>$S05#b8
w $g#67
r $c40b0100fc430*!300*!x*$0*Tff0f0*(f7240*0#58
w $qfThreadInfo#bb
r $mp01.01#cd
w $qsThreadInfo#c8
r $l#6c
w $z0,10bc4,2#8e
r $OK#9a
c info r
c bt
c q
w $D;1#b0
r $OK#9a
End of log

[daniel5151]

Thanks for the PR!

After you remove the RegIdImpl bits, we can get this merged in 👍

[daniel5151]

Then again, after taking a closer look, can you confirm that the RegId implementation is actually correct? If the CG register isn't actually sent as part of the main registers payload, then there's a chance that it's also not assigned a regid value either. If so, then that could explain some weirdness you're seeing, as GDB could potentially be reading/writing to the wrong registers.

[mchesser]

Updated to used fixed RegId. I'm fairly sure that the IDs are actually correct, e.g. after running this:

	MOVX #0x1,SP
	MOVX #0x4,R4
	MOVX #0x5,R5
	MOVX #0x6,R6
	MOVX #0x7,R7
	MOVX #0x8,R8
	MOVX #0x9,R9
	MOVX #0xa,R10
	MOVX #0xb,R11
	MOVX #0xc,R12
	MOVX #0xd,R13
	MOVX #0xe,R14
	MOVX #0xf,R15

GDB reports:

(gdb) info r
pc             0x4464   0x4464 <test+72>
sp             0x1      1
sr             0x0      0
cg             <unavailable>
r4             0x4      4
r5             0x5      5
r6             0x6      6
r7             0x7      7
r8             0x8      8
r9             0x9      9
r10            0xa      10
r11            0xb      11
r12            0xc      12
r13            0xd      13
r14            0xe      14
r15            0xf      15

... That said, maybe it makes sense to just return 0 for the CG register -- I've only ever use the TI debugger for MSP430 in the past (which I don't think is GDB based), but that typically treats CG as being 0 through the debugger (e.g. watch expressions).

[daniel5151]

... That said, maybe it makes sense to just return 0 for the CG register -- I've only ever use the TI debugger for MSP430 in the past (which I don't think is GDB based), but that typically treats CG as being 0 through the debugger (e.g. watch expressions).

Up to you ¯\_(ツ)_/¯

Feel free to switch it, or if you're happy as is, let me know and we can merge the PR in as-is.

[daniel5151]

I think I'm going to go ahead and merge this PR as-is by EOD.
Feel free to push up any tweaks / submit a follow-up PR if you want to make any changes.

[mchesser]

Ah I was meaning to get back to this. It works fine for me as is so I happy for it to be merged.

Only other thing I noticed is that the RISC-V implementation, where x0 is hardcoded to zero in hardware, is just treated the same as any other register.

[daniel5151]

Only other thing I noticed is that the RISC-V implementation, where x0 is hardcoded to zero in hardware, is just treated the same as any other register.

Would you mind popping open an issue to track that? That way I can address it at some point. Of course, a follow-up PR would also be more than welcome.

In any case though, I'll go ahead and merge this PR 😄