Opened 16 months ago

Closed 14 months ago

Last modified 11 months ago

#14546 closed defect (fixed)

SCI: Crash in Island of Dr. Brain microscope puzzle

Reported by: eriktorbjorn Owned by: deckarep
Priority: normal Component: Engine: SCI
Version: Keywords:
Cc: Game: Island of Dr. Brain

Description (last modified by 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.

Attachments (2)

islandbrain.010 (33.6 KB ) - added by eriktorbjorn 16 months ago.
islandbrain.012 (36.2 KB ) - added by eriktorbjorn 16 months ago.

Download all attachments as: .zip

Change History (6)

by eriktorbjorn, 16 months ago

Attachment: islandbrain.010 added

by eriktorbjorn, 16 months ago

Attachment: islandbrain.012 added

comment:1 by eriktorbjorn, 16 months ago

Description: modified (diff)

comment:2 by deckarep, 14 months ago

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

comment:3 by bluegr, 14 months ago

Owner: set to deckarep
Resolution: fixed
Status: newclosed

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

comment:4 by sluicebox, 11 months ago

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.
  1. 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.
Note: See TracTickets for help on using tickets.