beardypig / ghidra-emotionengine

Ghidra Processor for the Play Station 2's Emotion Engine MIPS based CPU
Apache License 2.0
198 stars 35 forks source link

Mangled decompilation for pairs of ldl/ldr and sdl/sdr #54

Open mdecicco opened 3 years ago

mdecicco commented 3 years ago

image Pairs of ldl/ldr and sdl/sdr are decompiled strangely. I may be wrong about this, but I believe that when the sdl and sdr instructions come in pairs like this and both have the same register as the first operand (and when the second operands together refer to the space that a doubleword would fit in) it is essentially just a store of a double word. I found this code in the MIPS language in the ghidra repo which seems to deal with just this: here ... and here.

This seems to rely on PAIR_INSTRUCTION_FLAG which as far as I was able to understand might be referring to a single bit in the data that encodes the instruction. I did some research and even looked through the PCSX2 source code but wasn't able to determine if such a pair instruction flag exists for R5900 instructions. Here's how it's defined in mips.sinc ... and in mips64.pspec

Is there any way to add behavior like this to ghidra-emotionengine? I am more than willing to write an analyzer or modify the language files and submit a pull request, I'd just need a few hints about what needs to be done in general as I'm still new to reverse engineering and don't fully understand things as low level as this.

EDIT: I used the disassembly from IDA in the screenshot just because the disassembly in ghidra is cluttered with local variable names and I wanted it to be more clear what the intent of the instructions is.

The ldl/ldr/lw instructions don't seem to correspond to any of the code in the decompilation view. I'm not sure what that's about.

Mc-muffin commented 3 years ago

Sorry, I'm not here to answer your question, but just so you know, if by clutter you meant the fact that Ghidra changes registers to the variables in the assembly view, like so: image

you can disable that via Edit>Tool options... then expand Listing Fields select Operand Fields and disable the "Markup" options, that way it'll always show the actual registers: image

mdecicco commented 3 years ago

Ah, thank you! I actually have been wishing I could remove the variable names from the listing, I didn't even realize it was an option.

beardypig commented 3 years ago

@mdecicco that can definitely be improved. I ignored it when I initially wrote the extension as I was not very familiar with SLEIGH nor Ghidra when it was first released.

mdecicco commented 3 years ago

@beardypig Is there anything that I can do to help with this?

astrelsky commented 3 years ago

The pair instruction flag is defined in the mips processor spec files. You can test it by setting it up the same way. It allows the user to override this value in the listing via set register value.

Changes may need to be made to the constant reference analyzer to add back the handling of this flag but you can test it without that for now.

mdecicco commented 3 years ago

I tried that before opening this issue. I probably didn't do it correctly, I kept getting exceptions while loading the extension. I'll try again when I get the chance and post the results here when I get as far as I think I can go.

bigianb commented 3 years ago

Some more context in case it helps. The default structure copy operation seems to use this. For example (pseudo C code):

struct vec3 {float x, float y, float z}

passing a vec3 by value or doing a value assignment such as:

vec3 a;
vec3 b;

b = a;

will be implemented by the compiler as:

ldl
ldr
lw
sdl
sdr
sw

So it's using ldl/ldr sdl/sdr as a quicker alternative to 2 lw/sw pairs. You see a similar thing with bigger matrix copies ... which are quite common in PS2 code and it makes simple C decompile to a nightmare.

bigianb commented 3 years ago

I implemented it in my fork and it seems to work okay (still ugly but much better than previously). I based it on @astrelsky 's v10 changes so probably a bit noisy for a PR but feel free to cherry pick or whatever - I just moved the code across from the ghidra mips source.

https://github.com/bigianb/ghidra-emotionengine/tree/v10-support