Opened 16 years ago

Closed 16 years ago

Last modified 16 years ago

#1465 closed defect (fixed)

SAM: Crash when screen saver is active

Reported by: Kirben Owned by: eriktorbjorn
Priority: normal Component: Engine: SCUMM
Keywords: Cc:
Game: Sam and Max

Description

Latest ScummVM cvs version (Either branch) English version of Sam and Max CD Compiled under mingw with GCC 3.2.3 and running under Windows XP

Sam and Max sometimes crashes when the screen saver is activated during the ending credits. This is due to crash in midi parser.

One of the screen savers seems enable to draw over verbs area correctly too.

I have attached valgrind logs, which report invalid reads at several points. Jamieson, could you have a look at music issue ?

Ticket imported from: #899249. Ticket imported from: bugs/1465.

Attachments (2)

samnmax_midi.txt (30.8 KB ) - added by Kirben 16 years ago.
Valgrind log of midi parser
samnmax.txt (855 bytes ) - added by Kirben 16 years ago.
Valgrind log of screen saver

Download all attachments as: .zip

Change History (11)

by Kirben, 16 years ago

Attachment: samnmax_midi.txt added

Valgrind log of midi parser

by Kirben, 16 years ago

Attachment: samnmax.txt added

Valgrind log of screen saver

comment:1 by Kirben, 16 years ago

GDB output of crash: Program received signal SIGSEGV, Segmentation fault. 0x00465e02 in Scumm::ScummEngine_v6::o6_kernelGetFunctions() (this=0x146ac58) at scumm/script_v6.cpp:2715 2715 push(vs->screenPtr[args[1] + args [2] * vs->width]); (gdb) bt #0 0x00465e02 in Scumm::ScummEngine_v6::o6_kernelGetFunctions() (this=0x146ac58) at scumm/script_v6.cpp:2715 #1 0x0045e296 in Scumm::ScummEngine_v6::executeOpcode (unsigned char) (this=0x146ac58, i=200 '') at scumm/script_v6.cpp:379 #2 0x0044b730 in Scumm::ScummEngine::executeScript() (this=0x146ac58) at scumm/script.cpp:425 #3 0x0044cf18 in Scumm::ScummEngine::runAllScripts() (this=0x146ac58) at scumm/script.cpp:750 #4 0x0041b062 in Scumm::ScummEngine::scummLoop(int) (this=0x146ac58, delta=1) at scumm/scummvm.cpp:1495 #5 0x00419f7b in Scumm::ScummEngine::mainRun() (this=0x146ac58) at scumm/scummvm.cpp:1279 #6 0x00418503 in Scumm::ScummEngine::go() (this=0x146ac58) at scumm/scummvm.cpp:918 #7 0x0040765b in runGame(GameDetector&, OSystem*) (detector=@0x22ff38, system=0x145eb98) at base/main.cpp:258 #8 0x00407b78 in main (argc=3, argv=0x1412530) at base/main.cpp:369 (gdb)

comment:2 by fingolfin, 16 years ago

The backtrace you posted is not in the MIDI parser, though. How does it add up to what you said earlier? What exactly shows that this is a crash in the MIDI parser?

Looking at the Valgrind log, it seems as if indeed MidiParser::readVLQ and MidiParser_SMF::parseNextEvent sometimes read a byte too much, which should be fixed, but the crash itself occurs due to a read access when trying to access the screenPtr, no?

comment:3 by Kirben, 16 years ago

I noticed this happened at another point in Sam & Max when screen saver was activated too, so looks like it isn't limited to this point (Ending credits) in game.

It looks like either sutation can cause ScummVM to crash, the first time it crashed it was in MIDI parser. I should have posted both earlier, GDB output of first crash is below: Program received signal SIGSEGV, Segmentation fault. [Switching to thread 1400.0xe6c] 0x005a0924 in MidiParser::readVLQ(unsigned char*&) (data=@0x153d9ac) at sound/midiparser.cpp:72 72 str = data[0]; (gdb) bt #0 0x005a0924 in MidiParser::readVLQ(unsigned char*&) (data=@0x153d9ac) at sound/midiparser.cpp:72 #1 0x005a17b6 in MidiParser_SMF::parseNextEvent (EventInfo&) (this=0x153d708, info=@0x153d9c4) at sound/midiparser_smf.cpp:68 #2 0x005a0e91 in MidiParser::onTimer() (this=0x153d708) at sound/midiparser.cpp:226 #3 0x004a9f2b in Scumm::Player::onTimer() (this=0x151d3c4) at scumm/imuse_player.cpp:984 #4 0x004864e8 in Scumm::IMuseInternal::sequencer_timers (MidiDriver*) (this=0x151d0d8, midi=0x147f088) at scumm/imuse.cpp:350 #5 0x00486467 in Scumm::IMuseInternal::on_timer (MidiDriver*) (this=0x151d0d8, midi=0x147f088) at scumm/imuse.cpp:326 #6 0x004898ec in Scumm::IMuse::on_timer(MidiDriver*) (this=0x151e770, midi=0x147f088) at scumm/imuse.cpp:1768 #7 0x00489335 in Scumm::IMuseInternal::midiTimerCallback (void*) (data=0x147f088) at scumm/imuse.cpp:1694 #8 0x005ea373 in Timer::handler(int) (this=0x145ab18, t=10) at common/timer.cpp:99 #9 0x005ea20c in Timer::timer_handler(int) (t=10) at common/timer.cpp:79 #10 0x10024339 in SDL_SetTimer () from C:\msys\1.0 \local\bin\SDL.dll

comment:4 by fingolfin, 16 years ago

Owner: SF/jamieson630 removed
Summary: SAMNMAX: Crashes in ending creditsSAM: Crash when screen saver is active

comment:5 by eriktorbjorn, 16 years ago

I don't know about the MIDI parser crash, but I added some bounds-checking to that kernel operation so that pixels outside the screen are now claimed to have colour 0. Hopefully that should help some.

Actually, on the trunk Fingolfin had already added a couple of asserts, but since we already *know* Sam & Max will trigger those I replaced them by said bounds-checking, and a big fat warning message. It seemed like a kinder, gentler thing to do.

comment:6 by eriktorbjorn, 16 years ago

What's the status of this now? Does it still happen, or can it be closed?

comment:7 by Kirben, 16 years ago

I'm unable to reproduce the issue now, so seems it is fixed.

comment:8 by eriktorbjorn, 16 years ago

I'm closing this, then. If SourceForge will stay responsive long enough for me to do so...

comment:9 by eriktorbjorn, 16 years ago

Owner: set to eriktorbjorn
Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.