-
Notifications
You must be signed in to change notification settings - Fork 100
Description
Description
memoryBytes is allocated with 6 banks:
Astro8-Computer/Astro8-Emulator/main.cpp
Line 443 in 41fb1a5
| memoryBytes = vector<vector<uint16_t>>(6, vector<uint16_t>(65535, 0)); |
However, each of the instructions capable of updating the bank register limit the register range with this code:
Astro8-Computer/Astro8-Emulator/main.cpp
Line 1766 in 41fb1a5
| BankReg = arg & 0b111; |
This limits BankReg to values between 0 and 7, but only 0 to 5 are allocated.
Steps to Reproduce
- Create an Astrisc program with the following code which attempts to write 65 to bank 6 at word 50000.
LDIA 65
STLGE 6
HERE 50000- Run this program in a debugger (this was tested on GDB in Debian), and observe that it crashes with a segmentation fault at
SetMem(bank = 6, address = 50000, data=65).
Expected behavior
From the Memory Layout documentation, it appears only banks 0, 1, and 2 should be valid. Perhaps only 3 banks should be allocated and BankReg be instead limited to those 3 banks? Either way, the issue here is simply that Astro-8 programs can perform out-of-bounds writes, so each instance where the emulator updates BankReg should perform a limit check that matches the number of "rows" allocated by:
Astro8-Computer/Astro8-Emulator/main.cpp
Line 443 in 41fb1a5
| memoryBytes = vector<vector<uint16_t>>(6, vector<uint16_t>(65535, 0)); |