I ran into a segfault recently. I compiled NSFPlay such that uninitialized data is randomized.
When NSFPlayer::Reset() is called, it iterates through the different chips and calls Tick(0). So in the NES_MMC5 object, in calc_sqr - it would wind up trying to use an out-of-bounds index when reading from the sqrtbl, since in here:
ret = sqrtbl[duty[i]][sphase[i]] ? v : 0;
sphase[i] will have some random, large value in it (and possibly duty but I only inspected the value in sphase). So - there's the out of bounds memory read.
I noticed that MMC5 and VRC6 don't call their Reset method in their constructor, whereas some other chips do - so I added that, and made sure values used in Reset are initialized (for example, in MMC5: the call to Write(0x5003) checks the value of enable, which hasn't been initialized yet).
I wasn't 100% if this is the right approach, or if these values should be set in the constructor instead and not call Reset? Happy to rework this to set defaults in the constructor instead, if there's a reason to not call Reset() in the constructor.
Hi there -
I ran into a segfault recently. I compiled NSFPlay such that uninitialized data is randomized.
When NSFPlayer::Reset() is called, it iterates through the different chips and calls Tick(0). So in the NES_MMC5 object, in
calc_sqr- it would wind up trying to use an out-of-bounds index when reading from thesqrtbl, since in here:sphase[i]will have some random, large value in it (and possiblydutybut I only inspected the value insphase). So - there's the out of bounds memory read.I noticed that MMC5 and VRC6 don't call their
Resetmethod in their constructor, whereas some other chips do - so I added that, and made sure values used inResetare initialized (for example, in MMC5: the call toWrite(0x5003)checks the value ofenable, which hasn't been initialized yet).I wasn't 100% if this is the right approach, or if these values should be set in the constructor instead and not call
Reset? Happy to rework this to set defaults in the constructor instead, if there's a reason to not callReset()in the constructor.