Opened 10 years ago

Closed 10 years ago

#4835 closed defect (fixed)

DRACI: Segfault Crash When Leaving Inventory

Reported by: digitall Owned by: rspalek
Priority: high Component: Engine: Draci
Keywords: Cc:
Game: Dragon History

Description

A segfault can be reliably generated by loading the attached savegame (at The Comedian's Tent), entering the inventory by going to the top of the screen, hitting F5 for Save/Load (which is disabled since inventory is open), then choosing "Resume", then exiting the inventory by moving to bottom right of screen.

Running this with valgrind gives :
==6715== Invalid read of size 4
==6715== at 0x82CB529: Draci::Game::handleInventoryLoop() (game.cpp:335)
==6715== by 0x82CC455: Draci::Game::loop(Draci::LoopSubstatus, bool) (game.cpp:539)
==6715== by 0x82CE61D: Draci::Game::start() (game.cpp:176)
==6715== by 0x82C75D2: Draci::DraciEngine::run() (draci.cpp:365)
==6715== by 0x80565C0: runGame(PluginSubclass<MetaEngine> const*, OSystem&, Common::String const&) (main.cpp:216)
==6715== by 0x8056F69: scummvm_main (main.cpp:389)
==6715== by 0x8053451: main (main.cpp:65)
==6715== Address 0x40 is not stack'd, malloc'd or (recently) free'd

ScummVM 1.1.0pre48399 (Mar 25 2010 02:42:50)
Features compiled in: Vorbis FLAC MP3 ALSA RGB zLib FluidSynth
built on Linux x86_32 2.6.31 by GCC 4.3.4

Ticket imported from: #2976774. Ticket imported from: bugs/4835.

Attachments (1)

draci.s16 (20.8 KB ) - added by digitall 10 years ago.
Savegame for Replication

Download all attachments as: .zip

Change History (8)

by digitall, 10 years ago

Attachment: draci.s16 added

Savegame for Replication

comment:1 by fingolfin, 10 years ago

Robert, can you take a look at this?

comment:2 by fingolfin, 10 years ago

Owner: set to rspalek
Priority: normalhigh

comment:3 by eriktorbjorn, 10 years ago

I can't try the savegame to actually see what happens right now, but assuming the line numbers are correct, this is where Valgrind warns:

333 If (_animUnderCursor != NULL && _animUnderCursor != _inventoryAnim && _animUnderCursor->getID() != kOverlayImage) {
334 _itemUnderCursor = getItem(kInventoryItemsID - _animUnderCursor->getID());
335 assert(_itemUnderCursor->_anim == _animUnderCursor);
336 } else {
337 _itemUnderCursor = NULL;
338 }

So ironically, it crashes when it should assert. I guess.

comment:4 by rspalek, 10 years ago

yes. I'll look at it and the other bugs this week, probably tomorrow night. I hope it is good enough; tonight I cannot do it

comment:5 by digitall, 10 years ago

I have traced this to _itemUnderCursor being NULL at line 335, so the assertion is dereferencing a NULL pointer.
This is due to getItem returning NULL as the parameter is negative.
This is due in turn to _animUnderCursor->getID() returning 274, rather than a valid Negative ID.

Since getItem() can return NULL, I have added a check for this to the assertion in SVN trunk r48434, but I'm not sure this is addressing the root cause.

spalek: if you could review this and advise, thanks.

comment:6 by rspalek, 10 years ago

Fixed by commit 48460. Your fix has just mitigated the assert, but the assert was valid, indicating that something was wrong. I believe the problem would have exerted itself by another means. I have fixed it properly; the problem was that the animations outside the inventory should not have been running but they were.

Thank you very much for looking into the problem when I was busy!

comment:7 by rspalek, 10 years ago

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.