beardypig / ghidra-emotionengine

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

Instruction Parsing Problems #38

Open Kneesnap opened 3 years ago

Kneesnap commented 3 years ago

It seems at least one instruction is not parsed properly by Ghidra. I have built a .S file. When I open the resulting ELF file in Ghidra, it misidentifies some of the instructions.

The .S file: image

Ghidra image

NOTE: The reason the .S file looks funky is because it's auto-generated.

Kneesnap commented 3 years ago

Another Instruction: image image

astrelsky commented 3 years ago

These are all correct. They are simplified. For example daddu v0, zero, zero is v0 = 0 + 0 which is the same as dmove v0, 0 or v0 = 0

Kneesnap commented 3 years ago

dmove is not a valid instruction in the emotion engine though. That's the problem.

astrelsky commented 3 years ago

Capture

Kneesnap commented 3 years ago

Just because it shows up in an emulator doesn't mean it's valid. Neither the official PS2 instruction set documentation, nor the PS2SDK recognize either qmove or dmove as valid instructions.

image

astrelsky commented 3 years ago

@beardypig

asmblur commented 3 years ago

I think it would be nice to have an option to disable those macros.

beardypig commented 3 years ago

I don't think it would be very easy to add an option to enable/disable these instructions because of how sleigh works, but I might be wrong. They were borrowed from other instruction sets to simplify the decompilation results, similar to what pcsx2 has in their debugger. I never intended for the decompiled output from this extension to be reused, but if people will find it more useful to use the strict instruction set then it can be changed.

We could remove them as they may no longer be required - but, my guess is that it would be a breaking change and might break existing projects.

Edit: upon further investigation, we could make this pseudo instructions optional using a flag.

Mc-muffin commented 3 years ago

Just because it shows up in an emulator doesn't mean it's valid. Neither the official PS2 instruction set documentation, nor the PS2SDK recognize either qmove or dmove as valid instructions.

image

Just as a reference, PCSX2's debugger was implemented by the same person that made armips, and as such it has some macros defined, but you can still view the un-macro'd instrunctions (and the bytes they are decoded from) by pressing tab in the assembly view

astrelsky commented 3 years ago

I don't think it would be very easy to add an option to enable/disable these instructions because of how sleigh works, but I might be wrong. They were borrowed from other instruction sets to simplify the decompilation results, similar to what pcsx2 has in their debugger. I never intended for the decompiled output from this extension to be reused, but if people will find it more useful to use the strict instruction set then it can be changed.

We could remove them as they may no longer be required - but, my guess is that it would be a breaking change and might break existing projects.

Edit: upon further investigation, we could make this pseudo instructions optional using a flag.

We could. Not sure if we mean the same thing, but we could also use the context register. You would need to redisassemble to see the changes though.