From 2c319399d26f7bd1790ecd91741fec7a5c174acd Mon Sep 17 00:00:00 2001 From: Peter Evans Date: Fri, 19 Jan 2018 00:34:57 -0600 Subject: [PATCH] Change disassembly to add more info --- include/mos6502.dis.h | 6 +-- src/mos6502.dis.c | 94 ++++++++++++++++++++++++------------------- tests/mos6502.dis.c | 51 +++++++++++------------ 3 files changed, 81 insertions(+), 70 deletions(-) diff --git a/include/mos6502.dis.h b/include/mos6502.dis.h index 9f3e6c2..c60562f 100644 --- a/include/mos6502.dis.h +++ b/include/mos6502.dis.h @@ -7,11 +7,11 @@ extern bool mos6502_dis_is_jump_label(int); extern int mos6502_dis_expected_bytes(int); extern int mos6502_dis_opcode(mos6502 *, FILE *, int); -extern void mos6502_dis_instruction(FILE *, int); +extern void mos6502_dis_instruction(char *, int, int); extern void mos6502_dis_jump_label(mos6502 *, vm_16bit, int, int); extern void mos6502_dis_jump_unlabel(int); -extern void mos6502_dis_label(FILE *, int); -extern void mos6502_dis_operand(mos6502 *, FILE *, int, int, vm_16bit); +extern void mos6502_dis_label(char *, int, int); +extern void mos6502_dis_operand(mos6502 *, char *, int, int, int, vm_16bit); extern void mos6502_dis_scan(mos6502 *, FILE *, int, int); #endif diff --git a/src/mos6502.dis.c b/src/mos6502.dis.c index f1ee186..f57e19e 100644 --- a/src/mos6502.dis.c +++ b/src/mos6502.dis.c @@ -10,6 +10,12 @@ #include "mos6502.dis.h" #include "mos6502.enums.h" +static char s_inst[4], + s_oper[10], + s_label[13], + s_state[51], + s_bytes[12]; + static vm_8bit jump_table[MOS6502_MEMSIZE]; static char *instruction_strings[] = { @@ -80,7 +86,8 @@ static char *instruction_strings[] = { */ void mos6502_dis_operand(mos6502 *cpu, - FILE *stream, + char *str, + int len, int address, int addr_mode, vm_16bit value) @@ -92,16 +99,16 @@ mos6502_dis_operand(mos6502 *cpu, case ACC: break; case ABS: - fprintf(stream, "$%04X", value); + snprintf(str, len, "$%04X", value); break; case ABX: - fprintf(stream, "$%04X,X", value); + snprintf(str, len, "$%04X,X", value); break; case ABY: - fprintf(stream, "$%04X,Y", value); + snprintf(str, len, "$%04X,Y", value); break; case IMM: - fprintf(stream, "#$%02X", value); + snprintf(str, len, "#$%02X", value); break; case IMP: break; @@ -109,16 +116,16 @@ mos6502_dis_operand(mos6502 *cpu, ind_address = mos6502_get(cpu, value + 1) << 8; ind_address |= mos6502_get(cpu, value); if (jump_table[ind_address]) { - mos6502_dis_label(stream, ind_address); + mos6502_dis_label(str, len, ind_address); } else { - fprintf(stream, "($%04X)", value); + snprintf(str, len, "($%04X)", value); } break; case IDX: - fprintf(stream, "($%02X,X)", value); + snprintf(str, len, "($%02X,X)", value); break; case IDY: - fprintf(stream, "($%02X),Y", value); + snprintf(str, len, "($%02X),Y", value); break; case REL: rel_address = address + value; @@ -126,18 +133,18 @@ mos6502_dis_operand(mos6502 *cpu, rel_address -= 256; } - mos6502_dis_label(stream, rel_address); + mos6502_dis_label(str, len, rel_address); break; case ZPG: // We add a couple of spaces here to help our output // comments line up. - fprintf(stream, "$%02X ", value); + snprintf(str, len, "$%02X", value); break; case ZPX: - fprintf(stream, "$%02X,X", value); + snprintf(str, len, "$%02X,X", value); break; case ZPY: - fprintf(stream, "$%02X,Y", value); + snprintf(str, len, "$%02X,Y", value); break; } } @@ -147,14 +154,14 @@ mos6502_dis_operand(mos6502 *cpu, * opcode maps to. */ void -mos6502_dis_instruction(FILE *stream, int inst_code) +mos6502_dis_instruction(char *str, int len, int inst_code) { // Arguably this could or should be done as fputs(), which is - // presumably a simpler output method. But, since we use fprintf() + // presumably a simpler output method. But, since we use snprintf() // in other places, I think we should continue to do so. Further, we // use a simple format string (%s) to avoid the linter's complaints // about potential security issues. - fprintf(stream, "%s", instruction_strings[inst_code]); + snprintf(str, len, "%s", instruction_strings[inst_code]); } /* @@ -267,57 +274,56 @@ mos6502_dis_opcode(mos6502 *cpu, FILE *stream, int address) // contents of our inspection of the opcode. (For example, we may // just want to set the jump table in a lookahead operation.) if (stream) { + s_label[0] = '\0'; + s_oper[0] = '\0'; + // Hey! We might have a label at this position in the code. If // so, let's print out the label. if (jump_table[address]) { // This will print out just the label itself. - mos6502_dis_label(stream, address); + mos6502_dis_label(s_label, sizeof(s_label) - 3, address); // But to actually define the label, we need a colon to // complete the notation. (We don't _need_ a newline, but it // looks nicer to my arbitrary sensibilities. Don't @ me!) - fprintf(stream, ":\n"); + snprintf(s_label + 9, sizeof(s_label) - 9, ":\n"); } - // Let's print out to the stream what we have so far. First, we - // indent by four spaces. - fprintf(stream, " "); - // Print out the instruction code that our opcode represents. - mos6502_dis_instruction(stream, inst_code); - - // Let's "tab" over; each instruction code is 3 characters, so let's - // move over 5 spaces (4 spaces indent + 1, just to keep everything - // aligned by 4-character boundaries). - fprintf(stream, " "); + mos6502_dis_instruction(s_inst, sizeof(s_inst), inst_code); if (expected) { // Print out the operand given the proper address mode. - mos6502_dis_operand(cpu, stream, address, addr_mode, operand); - } else { - // Print out a tab to get a consistent look in our - // disassembled code (e.g. to take up the space that an - // operand would otherwise occupy). - fprintf(stream, "\t"); + mos6502_dis_operand(cpu, s_oper, sizeof(s_oper), address, addr_mode, operand); } // Here we just want to show a few pieces of information; one, // what the PC was at the point of this opcode sequence; two, // the opcode; - fprintf(stream, "\t; pc=$%02x%02x cy=%02d: %02x", + snprintf(s_state, sizeof(s_state) - 1, + "pc:%02x%02x cy:%02d val:%04x a:%02x x:%02x y:%02x p:%02x s:%02x", cpu->PC >> 8, cpu->PC & 0xff, - mos6502_cycles(cpu, opcode), opcode); + mos6502_cycles(cpu, opcode), operand, + cpu->A, cpu->X, cpu->Y, cpu->P, cpu->S); // And three, the operand, if any. Remembering that the operand // should be shown in little-endian order. if (expected == 2) { - fprintf(stream, " %02x %02x", operand & 0xff, operand >> 8); + snprintf(s_bytes, sizeof(s_bytes) - 1, "%02x %02x %02x", + opcode, operand & 0xff, operand >> 8); } else if (expected == 1) { - fprintf(stream, " %02x", operand & 0xff); + snprintf(s_bytes, sizeof(s_bytes) - 1, "%02x %02x", + opcode, operand & 0xff); + } else { + snprintf(s_bytes, sizeof(s_bytes) - 1, "%02x", opcode); } + } - // And let's terminate the line. - fprintf(stream, "\n"); + fprintf(stream, "%s %4s %-9s ; %-51s | %s\n", + s_label, s_inst, s_oper, s_state, s_bytes); + + if (mos6502_would_jump(inst_code)) { + fprintf(stream, ";;;\n"); } // The expected number of bytes here is for the operand, but we need @@ -353,6 +359,10 @@ mos6502_dis_jump_label(mos6502 *cpu, int jump_loc; switch (addr_mode) { + case ABS: + jump_loc = operand; + break; + // With indirect address mode, the address we want to jump to is // not the literal operand, but a 16-bit address that is // _pointed to_ by the address represented by the operand. Think @@ -387,9 +397,9 @@ mos6502_dis_jump_label(mos6502 *cpu, * fairly dumb; it'll print out whatever address you give to it. */ inline void -mos6502_dis_label(FILE *stream, int address) +mos6502_dis_label(char *str, int len, int address) { - fprintf(stream, "ADDR_%x", address); + snprintf(str, len, "ADDR_%04x", address); } /* diff --git a/tests/mos6502.dis.c b/tests/mos6502.dis.c index b06c7b5..b733149 100644 --- a/tests/mos6502.dis.c +++ b/tests/mos6502.dis.c @@ -74,13 +74,13 @@ TestSuite(mos6502_dis, .init = setup, .fini = teardown); Test(mos6502_dis, operand) { - mos6502_dis_operand(cpu, stream, 0, ABS, 0x1234); + mos6502_dis_operand(cpu, buf, sizeof(buf), 0, ABS, 0x1234); assert_buf("$1234"); - mos6502_dis_operand(cpu, stream, 0, ABX, 0x1234); + mos6502_dis_operand(cpu, buf, sizeof(buf), 0, ABX, 0x1234); assert_buf("$1234,X"); - mos6502_dis_operand(cpu, stream, 0, ABY, 0x1234); + mos6502_dis_operand(cpu, buf, sizeof(buf), 0, ABY, 0x1234); assert_buf("$1234,Y"); - mos6502_dis_operand(cpu, stream, 0, IMM, 0x12); + mos6502_dis_operand(cpu, buf, sizeof(buf), 0, IMM, 0x12); assert_buf("#$12"); mos6502_set(cpu, 0x1234, 0x48); @@ -89,45 +89,45 @@ Test(mos6502_dis, operand) // For JMPs and JSRs (and BRKs), this should be a label and not a // literal value. So we need to test both the literal and // jump-labeled version. - mos6502_dis_operand(cpu, stream, 0, IND, 0x1234); + mos6502_dis_operand(cpu, buf, sizeof(buf), 0, IND, 0x1234); assert_buf("($1234)"); mos6502_dis_jump_label(cpu, 0x1234, 0, IND); - mos6502_dis_operand(cpu, stream, 0, IND, 0x1234); + mos6502_dis_operand(cpu, buf, sizeof(buf), 0, IND, 0x1234); assert_buf("ADDR_3448"); // Let's undo our label above... mos6502_dis_jump_unlabel(0x1234); - mos6502_dis_operand(cpu, stream, 0, IDX, 0x12); + mos6502_dis_operand(cpu, buf, sizeof(buf), 0, IDX, 0x12); assert_buf("($12,X)"); - mos6502_dis_operand(cpu, stream, 0, IDY, 0x34); + mos6502_dis_operand(cpu, buf, sizeof(buf), 0, IDY, 0x34); assert_buf("($34),Y"); - mos6502_dis_operand(cpu, stream, 0, ZPG, 0x34); - assert_buf("$34 "); - mos6502_dis_operand(cpu, stream, 0, ZPX, 0x34); + mos6502_dis_operand(cpu, buf, sizeof(buf), 0, ZPG, 0x34); + assert_buf("$34"); + mos6502_dis_operand(cpu, buf, sizeof(buf), 0, ZPX, 0x34); assert_buf("$34,X"); - mos6502_dis_operand(cpu, stream, 0, ZPY, 0x34); + mos6502_dis_operand(cpu, buf, sizeof(buf), 0, ZPY, 0x34); assert_buf("$34,Y"); // These should both end up printing nothing - mos6502_dis_operand(cpu, stream, 0, ACC, 0); + mos6502_dis_operand(cpu, buf, sizeof(buf), 0, ACC, 0); assert_buf(""); - mos6502_dis_operand(cpu, stream, 0, IMP, 0); + mos6502_dis_operand(cpu, buf, sizeof(buf), 0, IMP, 0); assert_buf(""); // Test a forward jump (operand < 128) - mos6502_dis_operand(cpu, stream, 500, REL, 52); - assert_buf("ADDR_228"); + mos6502_dis_operand(cpu, buf, sizeof(buf), 500, REL, 52); + assert_buf("ADDR_0228"); // Test a backward jump (operand >= 128) - mos6502_dis_operand(cpu, stream, 500, REL, 152); - assert_buf("ADDR_18c"); + mos6502_dis_operand(cpu, buf, sizeof(buf), 500, REL, 152); + assert_buf("ADDR_018c"); } Test(mos6502_dis, instruction) { #define TEST_INST(x) \ - mos6502_dis_instruction(stream, x); \ + mos6502_dis_instruction(buf, sizeof(buf) - 1, x); \ assert_buf(#x) TEST_INST(ADC); @@ -216,7 +216,7 @@ Test(mos6502_dis, opcode) mos6502_set(cpu, 1, 0x38); bytes = mos6502_dis_opcode(cpu, stream, 0); - assert_buf(" AND #$38 ; pc=$0000 cy=02: 29 38\n"); + assert_buf(" AND #$38 ; pc:0000 cy:02 val:0038 a:00 x:00 y:00 p:00 s:00 | 29 38\n"); cr_assert_eq(bytes, 2); } @@ -235,9 +235,10 @@ Test(mos6502_dis, scan) // I'm honestly not sure. There are definitely times (e.g. during // runtime operation) when you don't want it to, but as a standalone // disassembler, it feels less useful when PC isn't emulated. - assert_buf(" AND #$38 ; pc=$0000 cy=02: 29 38\n" - " DEY ; pc=$0000 cy=02: 88\n" - " JMP ($1234) ; pc=$0000 cy=05: 6c 34 12\n" + assert_buf(" AND #$38 ; pc:0000 cy:02 val:0038 a:00 x:00 y:00 p:00 s:00 | 29 38\n" + " DEY ; pc:0000 cy:02 val:0000 a:00 x:00 y:00 p:00 s:00 | 88\n" + " JMP ($1234) ; pc:0000 cy:05 val:1234 a:00 x:00 y:00 p:00 s:00 | 6c 34 12\n" + ";;;\n" ); } @@ -271,6 +272,6 @@ Test(mos6502_dis, jump_label) Test(mos6502_dis, label) { - mos6502_dis_label(stream, 123); - assert_buf("ADDR_7b"); + mos6502_dis_label(buf, sizeof(buf) - 1, 123); + assert_buf("ADDR_007b"); }