Raspberry Pi build fails in a number of ways

[spjewkes]

Raspberry pi build fails to build for the version of catch2. If I work around this, it fails on booting the 48.rom.

[spjewkes]

There seems to be an issue with the way it's handling subtraction of numbers. I think the code is making assumptions in some cases that need to be fixed.

The divergence happens very early on during ROM initialization at address 0x11e0. We hit the instruction:

JR NZ,0xFA

This should be treated as a negative number and subtract 0x6 from the current PC (which will be 0x11e2). Therefore we should jump back to 0x11dc.

However, on the Raspberry Pi we jump forward by 0xFA and end up at address: 0x12dc. At which point the whole code breaks considerably.

It's been a while since this code was written and I know it's probably been written quite sloppily. So we need to investigate this.

[spjewkes]

I think the issue is in the to_s32() function that takes the raw hex value and casts it into a signed value.

If I do the following on the Mac:

uint8_t data = 0xfa;
char converted = static_cast<char>(data);
std::cout << static_cast<int>(converted) << std::endl;

The result gets printed as '-6'. However, on the Raspberry PI it is displayed as '250'.

I'm obviously doing this in a non-portable way.

[spjewkes]

Doing a small test with 8-bit wide and 32-bit wide we see a difference. 32-bit wide properly handles the sign difference.

I think this is some undefined behaviour that we're relying on in C++ but I must admit I'm not sure what the best approach will be here. Do we have to manually detect the sign in the unsigned char and extend it?

[spjewkes]

My understanding of the issue is that char can be signed or unsigned. GCC appears to have an option to specify how this should be dealt with:

-fsigned-char
-funsigned-char

There are other options. So the issue is the sign extension from casting from char to int that is the problem here and it is compiler dependent.

[spjewkes]

Adding -fsigned-char as a build option for cmake fixes the issue. Perhaps we should be explicit here because of the assumption we're making. Although in future we might want to look at a better way of implementing this.

[spjewkes]

CMake file updated for now with a comment. Perhaps this could be looked at in future for a better way to deal with it.