RISCV debug improvements - Part 1

[noppej]

This PR was meant to focus on RISCV debug improvements. However, in the course of fixing RISCV, the extent of changes ballooned, because I discovered some things that needed to be fixed first (my apologies for the large PR)

• The current probe_rs::debug::registers API needed changes:
  • Needed to adapt to the new probe_rs::RegisterValue to allow support of 32-bit and 64-bit values.
  • More importantly, the current implementation is focussed on 'platform_registers', and with RISCV, the program counter register lives outside of of the 32 core platform registers.
• Fixed the RISCV disassembly - it was broken because it didn't take the 'compressed' instruction set into account. I needed to fix this to help with debugging the stack unwind.

This brings RISCV debug functionality that works up to the following:

• Stacktrace correctly unwinds functions and inlined functions.
• Until this riscv-rt PR is released, the name of the main function appears mangled in the stacktrace.
• TODO: Implement ability to unwind into ROM functions that have no debug information see comment here
• Peripheral registers work
• CPU registers work
• Program variables are unwound (names and datatypes appear correctly).
• Program variables data values are not working correctly.
• Breakpoints (hardware) can be set, and will correctly halt execution.
• Stepping during halt does not work yet.
• Disassembly for RV32C (compressed instruction set) works.
• TODO: Need to implement instruction set variants to track compressed vs. non-compressed variants of RISCV

@MabezDev I would appreciate if you can test some of the 'working' features above on your RISCV use cases.
@thefaxman I would appreciate if you can test to make sure I didn't break any of the ARMv8 (32 and 64-bit) stuff.

[thefaxman]

Overall this is good stuff. I added a few comments here and there, but most of them are in type conversion cleanup and figuring out where to place certain logic. The direction is rock solid.

I did some testing on ARMv8-a / v7-a and not only did it not regress things, it actually got functionality working that wasn't before:

>> bt
Frame 0: cpu_do_idle @ 0xffffffc008b66f2c
       /home/rfairfax/linux/arch/arm64/kernel/idle.c:32:2
Frame 0: cpu_do_idle () @ 0xffffffc008b66f2c

This unwind never worked at all previously. It's still not walking the full stack as here's what GDB is getting (one more frame):

#0  cpu_do_idle () at arch/arm64/kernel/idle.c:32
#1  0xffffffc008b66f44 in arch_cpu_idle () at arch/arm64/kernel/idle.c:44
#2  0x0000000100000001 in ?? ()

All that being said this is a clear improvement and I can find no regressions in my testing. It's not surprising to me that there's further work to get ARMv8-a fully working. Ship it.

[thefaxman]

Should this be Platform to match the other usage?

[noppej]

I'm open to votes on that. I personally think 'platform' has a broader meaning, and the DWARF spec calls them base registers, so I went with that here, since the debug module is so closely linked with dwarf stuff.

[thefaxman]

Fair enough. If we're matching terms from the DWARF spec, which makes sense, a comment may help.

[noppej]

@thefaxman

All that being said this is a clear improvement and I can find no regressions in my testing. It's not surprising to me that there's further work to get ARMv8-a fully working. Ship it.

Thanks you for your review and some very valid suggestions. I've added TODO's for things that need work, and I will tackle those as Part 2 ( a follow-up PR) of working on this part of the code. I will be away (off grid) from tomorrow thru Fri, so will synch with you on the the FP support for ARMv8-a you are working on when I get back.

[noppej]

@thefaxman If you are inspired, would you please look at this code

I can't explain why it would be necessary to read the N-2 frame's debug info, to get the correct LR value for N-1. If I don't do it, the unwind on Arm doesn't work at all ... and on RISCV it works the way you would expect (only need to read N-1 frame's debug info to the the correct RegisterRule and value for the LR).

I'm sure it is a gap in my knowledge somewhere, but have not found any documentation as to why it works like that on Arm.

PS. The reason I bring this up here, is because I think there is a risk that it might impact the ARMv8 work you are doing eventually.

[thefaxman]

@thefaxman If you are inspired, would you please look at this code

Nothing obvious jumps out to me from a read nor can I think of a reason why you'd need to do this. I do know ARM has a ton of quirks in unwinding, especially for a general purpose unwinder. I'm glad to help spend some cycles debugging this after I finish up the FP work I have in my queue.

[noppej]

bors r+

[bors]

Build succeeded:

• Check (macos-latest)
• Check (ubuntu-latest)
• Check (windows-latest)
• Rustfmt
• Test Suite (ubuntu-latest)
• Test Suite (windows-latest)