bug in master branch
Closed this issue · 2 comments
Hello everyone. Found a strange bug in the master branch. Global variables are defined incorrectly. Perhaps the addiu instruction is being handled incorrectly. In version 1.6 everything is ok. Ghidra 9.2.2, 9.2.3, 9.2.4. It seems to me that this bug appeared after commits Tupelov.
Ghidra9 2 4 _master
Just a heads up, I have seen this. Will be busy the next few days but it will be looked into once time allows.
I'm convinced that #55 is the culprit here, it affects those touched instructions. Look at this loop for example (I removed some irrelevant code):
01 00 04 24 li a0,0x1
ff ff 05 24 li a1,0xffff
loop:
ff ff 84 24 addiu a0,a0,0xffff
...
fb ff 85 14 bne a0,a1,loop
...
int iVar5;
iVar5 = 1;
do {
iVar5 = iVar5 + 0xffff;
} while (iVar5 != 0xffff);
However, this should have been:
int iVar5;
iVar5 = 1;
do {
iVar5 = iVar5 - 1;
} while (iVar5 != -1);
or:
01 00 04 24 li a0,0x1
ff ff 05 24 li a1,0xffffffff // ~signExtend(0xffff)
loop:
ff ff 84 24 addiu a0,a0,0xffffffff // ~signExtend(0xffff)
...
fb ff 85 14 bne a0,a1,loop
...
Such patterns can be found everywhere; so this breaks almost any disassembly / decompilation.
Note that the original mips.sinc came from Ghidra, which also treats this as signed: https://github.com/NationalSecurityAgency/ghidra/blob/b6ba209ed796343880327bc3337355c303b760cd/Ghidra/Processors/MIPS/data/languages/mips32Instructions.sinc#L465-L469
@Tupelov Looking at #53 I'm not sure how you concluded those values should be any different.
I think this is an issue which comes down to how Ghidra displays number, rather than how they are decoded.
If you want unsigned numbers, this should be the expected result:
addiu v0,v0,0xfffffd80
addiu v1,v1,0xffffe330
Wether you want to see signed or unsigned numbers depends on a case-by-case basis of course.
Ghidra allows you to change it: https://reverseengineering.stackexchange.com/questions/21253/how-do-i-make-a-hex-literal-a-decimal-literal-in-ghidra
If you are convinced that these should be different numbers than 0xfffffd80
and 0xffffe330
, I suggest to check if there might be a relocation table or something which is fixing these offsets.
Please revert #55 (as done in #69), I don't think any of the changes were correct.
I suggest to stick to what Ghidra does for the standard MIPS stuff (with minor fixes for the PS2): https://github.com/NationalSecurityAgency/ghidra/blob/b6ba209ed796343880327bc3337355c303b760cd/Ghidra/Processors/MIPS/data/languages/mips32Instructions.sinc#L24-L30