Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Interpreter] Unused opcode handling needs fixing #193

Open
andymcca opened this issue Apr 1, 2023 · 3 comments
Open

[Interpreter] Unused opcode handling needs fixing #193

andymcca opened this issue Apr 1, 2023 · 3 comments

Comments

@andymcca
Copy link

andymcca commented Apr 1, 2023

gpsp/cpu.c

Line 3190 in 541adc9

#ifdef HAVE_UNUSED

In the Interpreter, the handling of these ARM opcodes can only be included if HAVE_UNUSED is specified at compile time. Even then, the handling doesn't work because there is no code to move on to the next instruction. It should be something like this -

case 0xC0 ... 0xEF:
{
/* coprocessor instructions, reserved on GBA */
arm_next_instruction();
break;
}

@davidgfnet do you agree? The dynarecs don't have this issue because they don't process the instructions in a loop, and the program counter is moved forward regardless at the end of the Switch/Case.

I'm still debugging DemiKids Light at the moment and found this issue whilst working on it - unfortunately changing it doesn't fix DemiKids but it does stop an endless loop when DemiKids issues an invalid ARM instruction!

@davidgfnet
Copy link
Collaborator

Unfortunately invalid opcodes cannot be just "skipped" or ignored. Theoretically, an invalid instruction should generate an INVALID IRQ that can be handled by the BIOS. (Also note that the definition of invalid is quite open since the GBA accepts some opcodes that are theoretically forbidden by the ARM spec).
This is however pointless since the GBA BIOS blocks on said IRQ, it will just attempt to trigger some debugging stuff that is not present on the retail devices and "hang".
We could handle the illegal opcode by terminating the emulation somehow (not sure if libretro supports this, I assume it might), and perhaps printing some useful messages. For the dynarec it's a bit harder, since it's OK for the dynarec to generate a lot of invalid opcodes (say some never-taken branch to "garbage" code), so adding this print/trap would require runtime support.
Anyway not sure how valuable this is, since no "good" game should ever encounter an invalid opcode. This is just for our (developer) convenience.
On that note I checked a few games with this issue and already found a couple of bugs. Thanks!

@andymcca
Copy link
Author

andymcca commented Apr 3, 2023

No problem, and again, thanks for the explanations, it helps greatly for my understanding. My observation was that the invalid ARM opcode was issued during an Interrupt, but when I get back to it I'll actually get the PC and Opcode

@andymcca andymcca closed this as completed Apr 3, 2023
@andymcca
Copy link
Author

andymcca commented Apr 3, 2023

Didn't mean to close - clumsy phone fingers!

@andymcca andymcca reopened this Apr 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants