GDB Stub: Fix the id of the registers returned by p and P packets #10298
Conversation
|
@dolphin-emu-bot rebuild |
|
I have mixed feelings on the magic numbers used here, but it looks like they're auto-generated magic numbers on the GDB side too. On the ghidra side, there seem to be a lot more unnamed SPRs defined, but I don't know how that ties into the GDB integration either. |
Yeah I used the GDB page to define these. The ghidra one it's because what ghidra seems to try is to mix both the registers that it knows (which is p much all of them in theory) and GDB's which is a limited set. The reason it's at least important to respect GDB is because assuming one set the correct architecture, it allows See the way the remote protocol work with g, G, p and P packets is that both sides has to agree on the registers's ids because they refer to their positions when you send the reply. Here, we just take the one GDB is ok with, but the other way would be to support XML target descriptiong which allows us to define whatever. That being said, since this format allows us to INCLUDE others, it would almost certainly end up being an extension of the existing powerpc:750 since the GC/Wii doesn't add as much. So no matter how we end up doing this, these magic numbers would probably stay (it is possible to redefine them, but I personally don't see the point if GDB already has a set that is established). As for the ones Ghidra supports, it's something to handle on the Ghidra side because I only recently realised that our loader gives access to too much registers (there's many features that the CPU just doesn't support), but what it will do is it will still show you these registers, but the value will be grayed out. |
The stub was made with the assumption that the GDB architecture is rs6000:6000, but the closest is actually powerpc:750 which features much more SPR that the gekko supports, but it also has slightly different ID. This commit now assumes the more proper powerpc:750.
aldelaro5
force-pushed
the
gdb-stub-fix-registers
branch
from
1baf11d
to
beabd56
Compare
| } | ||
| else if (id >= 88 && id < 104) | ||
| { | ||
| PowerPC::ppcState.sr[SPR_IBAT0U + id - 88] = re32hex(bufptr); |
There was a problem hiding this comment.
This is a typo, and should be spr instead of sr. (It also generates a compile warning.)
The stub was made with the assumption that the GDB architecture is rs6000:6000, but the closest is actually powerpc:750 which features much more SPR that the gekko supports, but it also has slightly different ID. This commit now assumes the more proper powerpc:750.
To note: if we would want to have p much EVERY spr we want (including the arch specific ones), we would need to add support to XML registers which allows the stub to define its own layout, but I think this commit is enough to support the basic for now. It also incidentally has large implications with regards to Ghidra support:
It wasn't possible before this pr to get the registers to show up as well as seeing the disassembly and others because Ghidra would want to know every registers gdb would be able to know, but then since we eventually would error out, it would just give up.