NationalSecurityAgency / ghidra

Ghidra is a software reverse engineering (SRE) framework
https://www.nsa.gov/ghidra
Apache License 2.0
51.36k stars 5.85k forks source link

Z80 and Z180 incorrect CPIR instruction decompile #2343

Closed depili closed 7 months ago

depili commented 4 years ago

Describe the bug The CPIR instruction in z80.slaspec file has a logic error that inverts the end condition of the compare loop. https://github.com/NationalSecurityAgency/ghidra/blob/master/Ghidra/Processors/Z80/data/languages/z80.slaspec#L656 The implemented instruction returns to the start of the compare loop if the compare result is 0, when it should only do so if the result is nonzero.

This screenshot shows the incorrect decompilation with senseless end condition for the while loop:

Screenshot 2020-10-08 at 11 16 06

Expected behavior The while loop should end with while (some_searched_byte != byte_under_test || i != 0);

Environment (please complete the following information):

esaulenka commented 4 years ago

@depili, can you check CPDR? Looks like same issue in it.

depili commented 4 years ago

Yes, CPDR has the exact same issue on https://github.com/NationalSecurityAgency/ghidra/blob/master/Ghidra/Processors/Z80/data/languages/z80.slaspec#L686 it should be cmp != 0

The rom I'm currently working with doesn't use that instruction so didn't check that one.

I feel that the whole z80 slaspec could use an audit, for example the flags in F need a looking into (and the P flag might be outright wrong), as decompilation with conditionals tends to get extra messy, but that is stuff for another issue / pull request.

neuromancer commented 7 months ago

Can this issue be closed or it is still affecting the decompilation?

depili commented 7 months ago

The fix in #2437 looks good to me.