Closed ZornsLemma closed 3 years ago
Thanks, I think I have fixed it, but it would be great if you could confirm. I'll leave this open for a while in case you find something else.
Thanks for taking a look at that. The code change looks good to me and I've run through the benchmark with a selection of history buffer sizes (ensuring the history buffer finishes just before the stack for maximum potential corruption) and I couldn't get it to go wrong.
Thanks for checking. It seems that the fix worked.
Congratulations on the release of 7.0!
I think I've found a small bug in the history code. In add_line_to_history:
if X is 0, the dex will cause it to wrap round to 255 and we'll scribble past the end of the history buffer (unless of course it is a 256 byte buffer).
This doesn't happen very often, and it's quite sensitive to the precise size of the history buffer and the commands entered. (I noticed this because seemingly harmless code changes were breaking one particular version of my Acorn port, and making tiny code changes to debug it would make the problem go away. The history buffer was being dynamically sized to take up spare space below the stack and so the code changes would alter the history buffer size.)
You should be able to reproduce this by forcing history_size to be exactly $CE and running the benchmark with history support; the DEX will then wrap round from 0 to 255 on move 178. (I tested this by adding "CPX #$FF:HANG:BEQ HANG" immediately after the DEX and confirming it did hang in a C64 emulator.)
I think the fix is simply to check for wrapping here and make sure we wrap to the history buffer size not 255.