From 4ededcb94a7243d478566b0adc67e853fdfa2edc Mon Sep 17 00:00:00 2001 From: Aaron Ferrucci Date: Tue, 17 Apr 2012 22:33:48 -0700 Subject: [PATCH 1/5] My local mods to the Makefile, probably not interesting to anyone else. --- Makefile | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Makefile b/Makefile index af9075f..7db8e40 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,5 @@ +CC := gcc CFLAGS := -Wall -g all: dcpu a16 @@ -14,5 +15,9 @@ a16: $(A16_OBJS) dcpu.c: emulator.h emulator.c: emulator.h +.PHONY: test +test: + dcpu test.hex + clean: rm -f dcpu a16 *.o From fb0b53b96c98356488810380104e584695d85d09 Mon Sep 17 00:00:00 2001 From: Aaron Ferrucci Date: Tue, 17 Apr 2012 22:39:43 -0700 Subject: [PATCH 2/5] This test shows a bug on line 92 of dcpu_skip. The conditional there misses the pc increment for the b operand in a basic instruction. The test shows that a 3-word instruction is not properly skipped. The 3rd word of the instruction (SET [0x1000], [0xEEE0]), when mistakenly interpreted as the 1st word of the next instruction, causes an illegal opcode bailout in the monitor. --- tests/testskip.s | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 tests/testskip.s diff --git a/tests/testskip.s b/tests/testskip.s new file mode 100644 index 0000000..aa62937 --- /dev/null +++ b/tests/testskip.s @@ -0,0 +1,21 @@ +; Skip test. Assumes skipping a 3-word instruction may be faulty, but +; skipping a 2-word instruction works. + + SET A, 0x10 ; c001 + SET [0x1000], A ; 01e1, 1000 + SET [0xEEE0], 0 ; 81e1, eee0 + IFN A, 0x10 ; c00d + SET [0x1000], [0xEEE0] ; 79e1, 1000, eee0 + SET A, [0x1000] ; 7801, 1000 + ; A should still be 0x10 + + IFN A, 0x10 ; c00d + SET PC, fail ; 7dc1, 000f + +:pass + WORD 0x3FF0 ; 3ff0 + +:fail + WORD 0xEEE0 ; eee0 + + From 368192edee1ac38eac31f9b85450033bd543a1cd Mon Sep 17 00:00:00 2001 From: Aaron Ferrucci Date: Tue, 17 Apr 2012 22:41:48 -0700 Subject: [PATCH 3/5] Fix to skip on line 92 of emulator.c. Also hijacked one of the illegal non-basic opcodes to use as an emulator success code. --- emulator.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/emulator.c b/emulator.c index bd944e0..a989035 100644 --- a/emulator.c +++ b/emulator.c @@ -89,7 +89,7 @@ static u8 skiptable[32] = { /* operand forms that advance pc */ void dcpu_skip(struct dcpu *d) { u16 op = d->m[d->pc++]; d->pc += skiptable[op >> 10]; - if ((op & 15) == 0) + if ((op & 15) != 0) d->pc += skiptable[(op >> 4) & 31]; } @@ -133,8 +133,11 @@ void dcpu_step(struct dcpu *d) { d->m[--(d->sp)] = d->pc; d->pc = a; return; + case 0x3F: + fprintf(stderr, "< ILLEGAL OPCODE (success code)>\n"); + exit(0); default: fprintf(stderr, "< ILLEGAL OPCODE >\n"); - exit(0); + exit(1); } } From 89de205cdfab8de5663feee371263ea53db44402 Mon Sep 17 00:00:00 2001 From: Aaron Ferrucci Date: Wed, 18 Apr 2012 20:33:40 -0700 Subject: [PATCH 4/5] Reverting my local Makefile changes. --- Makefile | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Makefile b/Makefile index 7db8e40..af9075f 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,4 @@ -CC := gcc CFLAGS := -Wall -g all: dcpu a16 @@ -15,9 +14,5 @@ a16: $(A16_OBJS) dcpu.c: emulator.h emulator.c: emulator.h -.PHONY: test -test: - dcpu test.hex - clean: rm -f dcpu a16 *.o From 8b214f5002e9a8ec624f2c4582d5235ca02428e4 Mon Sep 17 00:00:00 2001 From: Aaron Ferrucci Date: Thu, 19 Apr 2012 22:22:20 -0700 Subject: [PATCH 5/5] Fix a bug in dcpu_skip(). The 'b' instruction operand (or 'a' operand for non-basic opcodes) could overflow skiptable, when used as an index. Masking the operand to force it within the table size has its own problem, as it causes aliasing from some constant operands to operands which "advance pc". Example: c00d 009c: IFN A, 16 fc03 009d: SUB A, 31 The constant 31 in the second instruction is 'b' operand value 0x3F. If masked with 31 (0x1F) to stay within the table, it becomes 0x1F, which has value 1 within skiptable - that's incorrect, the constant does not require another instruction word. The solution in this commit tries to perturb the existing code as little as possible; I just bump skiptable up to 64-locations, so there's no need to mask the operand before using it as an index. The other change is to update the mask on the basic-opcode 'a' operand so that literals are handled correcty for that operand as well. I've included a test case, "testskip3.s", which fails before the fix and passes after the fix. All 32 literal operand values for the 'b' operand are tested. --- emulator.c | 4 +- tests/testskip3.s | 168 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 170 insertions(+), 2 deletions(-) create mode 100644 tests/testskip3.s diff --git a/emulator.c b/emulator.c index a989035..9e665dd 100644 --- a/emulator.c +++ b/emulator.c @@ -80,7 +80,7 @@ u16 *dcpu_opr(struct dcpu *d, u16 code) { } } -static u8 skiptable[32] = { /* operand forms that advance pc */ +static u8 skiptable[64] = { /* operand forms that advance pc */ [0x10] = 1, [0x11] = 1, [0x12] = 1, [0x13] = 1, [0x14] = 1, [0x15] = 1, [0x16] = 1, [0x17] = 1, [0x1E] = 1, [0x1F] = 1, @@ -90,7 +90,7 @@ void dcpu_skip(struct dcpu *d) { u16 op = d->m[d->pc++]; d->pc += skiptable[op >> 10]; if ((op & 15) != 0) - d->pc += skiptable[(op >> 4) & 31]; + d->pc += skiptable[(op >> 4) & 63]; } void dcpu_step(struct dcpu *d) { diff --git a/tests/testskip3.s b/tests/testskip3.s new file mode 100644 index 0000000..db3133c --- /dev/null +++ b/tests/testskip3.s @@ -0,0 +1,168 @@ + SET A, 0x10 + + IFN A, 0x10 + SUB A, 0 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 1 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 2 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 3 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 4 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 5 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 6 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 7 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 8 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 9 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 10 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 11 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 12 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 13 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 14 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 15 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 16 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 17 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 18 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 19 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 20 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 21 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 22 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 23 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 24 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 25 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 26 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 27 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 28 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 29 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 30 + IFN A, 0x10 + SET PC, fail + + IFN A, 0x10 + SUB A, 31 + IFN A, 0x10 + SET PC, fail + +:pass + WORD 0x3FF0 + +:fail + WORD 0xEEE0 +