[draft] fix / rewrite relative addressing

[binary1230]

DRAFT, for review.

This might actually be good to ship, I just want to throw some more tests and it, and maybe it run it on a few larger SNES assembly projects to make sure I didn't break anything. Disclaimer: I'm not an expert in SNES asm, just fixing stuff I have a good repro for.

Fixes #170 (details within)

Changes:

• Fix false positive out of bounds error message incorrectly firing for valid code
• Validate hex literals from assembly code to be in range (i.e. "BRA $FFFF" is more than the 1 byte limit, so flag it)
• Raise error if trying to jump to label outside our current bank (BRA/BRL can only change 16bit PC so can't change bank, they overflow PC)
• Rewrote to fix a bunch of issues related to signed/unsigned conversions and casting the 4 input bytes in 'num' to other types very carefully

Pre-merge checklist:

• [ ] Write some asm tests that mirror my unit tests
• [x] Step through with a SNES debugger to verify accuracy on some tests
• [x] Check this on a large dissasmbly project for 1:1 binary correctness
• [ ] Should we do any of the checks outside of pass 2?
• [x] I wrote this a little verbose, do you guys want me to condense more?
• [ ] Should I put this in a CPU agnostic file in the code (i.e. this might be also applicable for SPC). I haven't look at any of the other CPUs and if they implement things the same way. Guessing, probably.
• [ ] Test error message printout

Future me / future coders notes:

• Make sure you really understand the guts of how signed 2 complements work :)
• The value of 'num' is 4 raw bytes, when casting it to anything less than 4 bytes you have to be super-explicit
• BRA can only jump -127/128 bytes. HOWEVER, we need to convert this value to 16bit signed value once we've read that byte of user input. See the note in the BRA instruction in the SNES manual https://wiki.nesdev.com/w/images/7/76/Programmanual.pdf#page=308
• I wrote some unit tests outside of the project that test this function specifically, would be cool to include them somewhere but setting all that up is probably an entire other project.

[p4plus2]

Should I put this in a CPU agnostic file in the code (i.e. this might be also applicable for SPC). I haven't look at any of the other CPUs and if they implement things the same way. Guessing, probably.

Well, the SPC only supports short branches, but otherwise, this code could probably be reused there. That said, I don't think its a worth while adventure unless there is a plan to simply gut and replace the entire mnemonic processing core. I don't think anybody would disagree that the code in that section isn't exactly pretty or extendable...

As for the code, and this is sort of a problem in asar already, just keep in mind that this doesn't work well if you place labels in banks which are mirrors of each other. Not necessarily a deal breaking issue, just something to keep in mind. I don't see a need to condense too much, clean up the todo and it should be alright.

From a pure visual glance I'd give this a sign off(with a small side note below), but I have NOT actually tested this and that needs to be done a bit more in depth before I'd suggest a full green light. But, this certainly does seem to be an improvement.

Side note: pretty sure the lhs needs to be a string in the first error message to fix the mingw build.

[coveralls]

Coverage increased (+0.1%) to 79.925% when pulling cf2b5d97519a5c32d43904752f6aaed89d54539d on binary1230:fix_relative_addressing into 83f8c84510b184576b584cfa15bbd6a3d5d933e7 on RPGHacker:master.

[coveralls]

Coverage increased (+0.1%) to 79.93% when pulling cf2b5d97519a5c32d43904752f6aaed89d54539d on binary1230:fix_relative_addressing into 83f8c84510b184576b584cfa15bbd6a3d5d933e7 on RPGHacker:master.

[binary1230]

haven't forgotten about this! just been working on a release of the tool that depends on this change, should be trying to finish this out soon.

[binary1230]

hi folks, was finally revisiting getting this merged in. could I get whitelisted for appveyor, I tried running it on my own fork but there's some FTP upload stuff that's failing. I think I fixed the build error it was complaining about

[randomdude999]

i implemented this logic when i rewrote arch-65816.cpp. seems to work fine, thanks