SCI: Crash in Island of Dr. Brain microscope puzzle

[eriktorbjorn]

Latest ScummVM snapshot
English "SierraClassics" release of the game

There are at least two ways the microscope puzzle in Island of Dr. Brain can crash when asking for a hint, though I've only been able to reproduce one of them.

I'm attaching two savegames. One before even starting the microscope puzzle (since the puzzle you get is random), and one where opening the puzzle and pressing the hint button will always crash with the following message:

ERROR: Uninitialized read for temp 0 from method cartesian::buyClue (room 160, script 165, localCall ffffffff)!

The other hint button crash had this message, but I haven't been able to reproduce it:

ERROR: Invalid arithmetic operation (division - params: 0000:fffd and 0000:0000) from method carthesian::buyClue (room 160, script 165, localCall ffffffff)!

For the second one, I was playing on my phone so that was using ScummVM 2.7.

Annoyingly, since I filed this bug report I have not been able to come up with any new test cases, so this bug seems to be a slippery thing.

[deckarep]

I spent some time trying to understand this crash and I think I've come to a possible solution. Thanks @eriktorbjorn for providing an attached file that helped me put together a patch.

Of course it needs careful review by the ScummVM team: ​https://github.com/scummvm/scummvm/pull/5342

[bluegr]

The PR has been merged, so this should be fixed now. Thanks for your work on this!

[sluicebox]

I'm late to the party on this one, but I'd like to clarify some things:

1. deckarep's workaround is correct. I verified the initial value of the uninitialized variable in the original interpreter, and it is indeed 1. The workaround gets us the original behavior that avoids the divide by zero.

2. eriktorbjorn got two different crashes but they were really the same thing. The difference is that on dev builds we call error on uninitialized reads and on release builds we call warning and use zero and hope for the best; zero is usually the right answer and that lets players proceed with the game. In this case that didn't work, and led to the divide by zero. The phone was running a release build so it got the divide by zero error instead of the uninitialized read error.