the unit tests work with wrong 2's complement numbers

[irmen]

Hi, even though the unit tests all succeed, I think there is a problem in all the test cases that use a 2's complement number.

The way the 2's complement is calculated is not correct. For example it uses rel = (0x06 ^ 0xFF + 1) # two's complement of 6 but that results in the value of 262 which is a) outside the range of an 8 bit value and b) not the 2's complement of 6. It should be 250. An easy way to calculate this is by doing 256 + (-6).

Here is an example of such a unit test (with a correction that I suggest):

     def test_bpl_negative_clear_branches_relative_backward(self):
         mpu = self._make_mpu()
         mpu.p &= ~(mpu.NEGATIVE)
         mpu.pc = 0x0050
         # $0050 BPL -6
-        rel = (0x06 ^ 0xFF + 1)  # two's complement of 6
+        rel = 256-6 # two's complement of 6
         self._write(mpu.memory, 0x0050, (0x10, rel))
         mpu.step()
-        self.assertEqual(0x0052 + rel, mpu.pc)
+        self.assertEqual(0x0052 - 6, mpu.pc)

The check on the actual PC result address needed a correction as well.

Is this something you want to fix? I'm willing to make a pull request for this.

[MorrisMA]

• rel = (0x06 ^ 0xFF + 1) # two's complement of 6
• rel = 256-6 # two's complement of 6
• rel = (0x06 ^ 0xFF) + 1 # two's complement of 6

I think that the parenthesis should be about the first operation. The problem is likely that the priority of the bit wise logical operators have lower precedence than the addition (+) and subtraction (-) operators.

[MorrisMA]

Another thought on this subject: the formula rel = (0x06 ^ 0xFF) + 1 will produce the correct value: 0xFA or -6. However, in general, due to the nature of the way integers are represented in python, this basic algorithm for computing the 2's complement of an 8-bit quantity will fail if the value being negated is 0x00. The formula rel = 256 - 0 will also fail. In both cases, the result will be 256 which exceeds the range of an 8-bit integer. To solve the issue, the computed value must be restricted to the range 0..255 by applying a bitwise AND: rel = ((val ^ 0xFF) + 1) & 0xFF. A bitwise AND operation, using a mask value equal to the desired width, should be applied to all integers computed in python. This operation only needs to applied on the writes to memory, but I've applied them to both writes and reads to ensure the values stored are within the desired range.

For the test in question, with the value used, the correct result will be provided since the value used is not 0x00.

[mnaberez]

To solve the issue, the computed value must be restricted to the range 0..255 by applying a bitwise AND: rel = ((val ^ 0xFF) + 1) & 0xFF.

The actual emulation code does the & 0xFF. It's only missing from these few test cases. I did not think it was necessary in these test cases since the fixed values used are always within bounds (as you mentioned).

[irmen]

@mnaberez by the way, I discovered this small issue while porting the test suite to Kotlin, to use for my Kotlin 6502 simulator :nerd_face:

[mnaberez]

I discovered this small issue while porting the test suite to Kotlin, to use for my Kotlin 6502 simulator 🤓

I'm glad the tests have been helpful and thanks for contributing back your fixes!