z80 icon indicating copy to clipboard operation
z80 copied to clipboard

Bug in opcode decoder for redundant DD/FD prefixes

Open nickd4 opened this issue 3 years ago • 1 comments

I noticed that it does not seem to pass the ZEXALL tests for me, I haven't tried to run the built-in tests because I have my own tests reliant on ZEXALL obtained from another source but I would guess that the built-in tests also fail and that this is a regression from refactoring that code, that hasn't been noticed.

The problem report with ZEXALL is as follows

ld <bcdexya>,<bcdexya>........  ERROR **** crc expected:478ba36b found:7c37fe10

and I found a corresponding bug in the opcode decoder for redundant DD/FD prefixes, fixed as follows

diff --git a/z80.c b/z80.c
index f309f1b..395c67b 100644
--- a/z80.c
+++ b/z80.c
@@ -1698,8 +1698,8 @@ loop: // for invalid DD/FD opcodes we return here to process normal opcode
 
   case 0xCB: exec_opcode_cb(z, nextb(z)); break;
   case 0xED: exec_opcode_ed(z, nextb(z)); break;
-  case 0xDD: if (!exec_opcode_ddfd(z, nextb(z), &z->ix)) goto loop; break;
-  case 0xFD: if (!exec_opcode_ddfd(z, nextb(z), &z->iy)) goto loop; break;
+  case 0xDD: if (!exec_opcode_ddfd(z, opcode = nextb(z), &z->ix)) goto loop; break;
+  case 0xFD: if (!exec_opcode_ddfd(z, opcode = nextb(z), &z->iy)) goto loop; break;
 
   default: fprintf(stderr, "unknown opcode %02X\n", opcode); break;
   }

nickd4 avatar Jul 22 '22 03:07 nickd4

Ah no it's OK this turned out to be because of a change I made.

However I am leaving the issue open because I think the recursion in this line

    // any other FD/DD opcode behaves as a non-prefixed opcode:
    exec_opcode(z, opcode);

is not a good design choice. So when I have time I will PR my solution that uses a loop instead. (The reason why I had to change things was because I made the exec opcodes be inline in my version, and the compiler complained that it could not inline things due to the recursion).

nickd4 avatar Jul 26 '22 03:07 nickd4