Opened 13 months ago

Last modified 13 months ago

#10365 new defect

SAGA: ITE: Crash when talking to Sist

Reported by: dafioram Owned by:
Priority: blocker Component: Engine: SAGA
Keywords: Cc:
Game: Inherit the Earth

Description

ScummVM: Kirben32 2.1.0git-104-g789a38b38c
Game: Inherit the Earth/GOG/Multi-OS/English
OS: Win7-64

Crash when talking to Sist.

  1. Load save game
  2. Walk to the door where Sist is by following directions at bottom of page
  3. Open door with right click, enter door with left click
  4. Once you take your robe off right click on sist.

It will crash:

Assertion failed: idx < _size, file ./common/array.h, line 192

This save is before entering the tunnel if you load a saved game that is already
in the tunnel then the crash doesn't happen.

Its also possible that the crash will also happen under simplier circumstances.
I have difficulty reproducing this on linux. I was able to reproduce it once on
linux, but then not after. I can always repro on the windows build.

The directions to Sist's room are given below with respect to the direction your
character is facing and a direction is only given when there is a choice to be made.

-- Spoiler --

Right, Pass door, Straight, Right, through the next 2 doors, Left, Right, Straight

Attachments (1)

ite-cd.s13 (15.5 KB) - added by dafioram 13 months ago.
Before Rat tunnel the first time.

Download all attachments as: .zip

Change History (4)

Changed 13 months ago by dafioram

Attachment: ite-cd.s13 added

Before Rat tunnel the first time.

comment:1 Changed 13 months ago by csnover

Priority: normalblocker

comment:2 Changed 13 months ago by digitall

Have tried to replicate with latest Git ie. ScummVM 2.1.0git161-g80dd7b2c0d on Linux x86_64.

Have loaded savegame and followed instructions to reach Sist and talk to him, but have not been able to trigger that array.h exception.

Running under GDB did not yield any useful information, but running under Valgrind showed three bad memory accesses in the SAGA engine code where unallocated/freed memory is accessed. This would likely account for the issue so these should be fixed to see if this corrected the problem.

These three accesses have very similar backtraces as follows:
==1876== Invalid read of size 4
==1876== at 0x18CEF02: Saga::HitZone::getFlags() const (objectmap.h:57)
==1876== by 0x18FB836: Saga::Actor::stepZoneAction(Saga::ActorData*, Saga::HitZone const*, bool, bool) (actor.cpp:552)
==1876== by 0x1904BE5: Saga::Actor::handleActions(int, bool) (actor_walk.cpp:695)
==1876== by 0x1904D46: Saga::Actor::direct(int) (actor_walk.cpp:727)
==1876== by 0x18DB323: Saga::SagaEngine::run() (saga.cpp:380)
==1876== by 0x576FDE: runGame(Plugin const*, OSystem&, Common::String const&) (main.cpp:264)
==1876== by 0x578221: scummvm_main (main.cpp:530)
==1876== by 0x5750EB: main (posix-main.cpp:45)
==1876== Address 0xb1b2ff0 is 80 bytes inside a block of size 120 free'd
==1876== at 0x4C2C13B: free (vg_replace_malloc.c:530)
==1876== by 0x18D2648: Common::Array<Saga::HitZone>::freeStorage(Saga::HitZone*, unsigned int) (array.h:320)
==1876== by 0x18D25AD: Common::Array<Saga::HitZone>::clear() (array.h:218)
==1876== by 0x18D2287: Saga::ObjectMap::clear() (objectmap.cpp:188)
==1876== by 0x18E1F49: Saga::Scene::endScene() (scene.cpp:1169)
==1876== by 0x18DFBEF: Saga::Scene::changeScene(short, int, Saga::SceneTransitionType, int) (scene.cpp:503)
==1876== by 0x18EC02C: Saga::Script::sfScriptGotoScene(Saga::ScriptThread*, int, bool&) (sfuncs.cpp:470)
==1876== by 0x18E5817: Saga::Script::opCcallV(Saga::ScriptThread*, Common::SeekableReadStream*, bool&, bool&) (script.cpp:637)
==1876== by 0x18F4CF8: Saga::Script::runThread(Saga::ScriptThread&) (sthread.cpp:208)
==1876== by 0x18F4951: Saga::Script::executeThreads(unsigned int) (sthread.cpp:156)
==1876== by 0x18DB363: Saga::SagaEngine::run() (saga.cpp:384)
==1876== by 0x576FDE: runGame(Plugin const*, OSystem&, Common::String const&) (main.cpp:264)
==1876== Block was alloc'd at
==1876== at 0x4C2AF0F: malloc (vg_replace_malloc.c:299)
==1876== by 0x18D2944: Common::Array<Saga::HitZone>::allocCapacity(unsigned int) (array.h:309)
==1876== by 0x18D270E: Common::Array<Saga::HitZone>::reserve(unsigned int) (array.h:273)
==1876== by 0x18D24C3: Common::Array<Saga::HitZone>::resize(unsigned int) (array.h:283)
==1876== by 0x18D21CF: Saga::ObjectMap::load(Saga::ByteArray const&) (objectmap.cpp:179)
==1876== by 0x18E165F: Saga::Scene::processSceneResources(Common::Array<Saga::SceneResourceData>&) (scene.cpp:1013)
==1876== by 0x18E04DD: Saga::Scene::loadScene(Saga::LoadSceneParams&) (scene.cpp:682)
==1876== by 0x18DFC08: Saga::Scene::changeScene(short, int, Saga::SceneTransitionType, int) (scene.cpp:506)
==1876== by 0x18DE6EA: Saga::SagaEngine::load(char const*) (saveload.cpp:369)
==1876== by 0x18DB05F: Saga::SagaEngine::run() (saga.cpp:343)
==1876== by 0x576FDE: runGame(Plugin const*, OSystem&, Common::String const&) (main.cpp:264)
==1876== by 0x578221: scummvm_main (main.cpp:530)

The second and third differ as follows:
3c3
< ==1876== by 0x18FB836: Saga::Actor::stepZoneAction(Saga::ActorData*, Saga::HitZone const*, bool, bool) (actor.cpp:552)
---

==1876== by 0x18FB85A: Saga::Actor::stepZoneAction(Saga::ActorData*, Saga::HitZone const*, bool, bool) (actor.cpp:552)

3c3
< ==1876== by 0x18FB836: Saga::Actor::stepZoneAction(Saga::ActorData*, Saga::HitZone const*, bool, bool) (actor.cpp:552)
---

==1876== by 0x18FB909: Saga::Actor::stepZoneAction(Saga::ActorData*, Saga::HitZone const*, bool, bool) (actor.cpp:563)

35a36

==1876==

This looks to be a likely issue in the Actor stepZoneAction / Hitzone code which should be corrected.

comment:3 Changed 13 months ago by dafioram

I tested three different cases with valgrind running to check for uninitialized reads.

case 1: Load saved game right outside of Sist's room, enter itm and then talk to him
Valgrind report: no uninitialized reads

case 2: Load saved game at beginning of rat tunnel then walk to Sist's room, enter it and then talk to him.
Valgrind report: no uninitialized reads

case 3: Load saved game right outside of the rat tunnel, enter tunnel, then walk to Sist's room, enter it and then talk to him.
Valgrind report: The same three uninitialized reads reported above by digitall.

I am confident that fixing these uninitialized reads will prevent the crash on windows since the windows crash only happens in case 3 (only case when valgrind reports uninitialized reads).

At a high level I am not sure where things are going wrong.

At a low level Saga::Actor::handleActions is passing in a HitZone pointer that has not been loaded (I.e., HitZone::load() has not been called on the hitZone object) into Saga::Actor::stepZoneAction.

If the hitZone->load() is not called on a hitZone object then hitZone->_flag will not be initialized and Saga::HitZone::getFlags() (which returns hitZone->_flag) doesn't exist hence the uninitialized read.

I was able to determine this by adding a member variable to hitZone that starts off false and becomes true once hitZone->load() is called. When in the bug case, as I am entering Sist's room it calls stepZoneAction() ( actor_walk.cpp line 694) with a hitZone that has not been loaded. In the bug free cases all hitZones are loaded for each debug stop at actor_walk.cpp line 694.

I added some checks to return from the stepZoneAction() function (by doing a check in stepZoneAction) and returning if hitZone has not been loaded, but even then there are still other uninitialized reads. Namely: two of the conditionals in Actor::stepZoneAction

  1. hitZone->isLoaded() (my function for determining if hitZone has been loaded) and
  2. actor != _protagonist

I have even seen that my Boolean member variable that is the flag for being loaded or not, is occasionally set to a number!

I think the final fix will involve some conditional changes in Actor::handleActions rather than defensive changes in Actor::stepZoneAction. Since the changes to the latter wouldn't address the root cause.

Note: See TracTickets for help on using tickets.