Opened 15 years ago

Closed 15 years ago

Last modified 15 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 15 years ago.
Valgrind log of midi parser
samnmax.txt (855 bytes) - added by Kirben 15 years ago.
Valgrind log of screen saver

Download all attachments as: .zip

Change History (11)

Changed 15 years ago by Kirben

Attachment: samnmax_midi.txt added

Valgrind log of midi parser

Changed 15 years ago by Kirben

Attachment: samnmax.txt added

Valgrind log of screen saver

comment:1 Changed 15 years ago by Kirben

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 Changed 15 years ago by fingolfin

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 Changed 15 years ago by Kirben

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 Changed 15 years ago by fingolfin

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

comment:5 Changed 15 years ago by eriktorbjorn

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 Changed 15 years ago by eriktorbjorn

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

comment:7 Changed 15 years ago by Kirben

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

comment:8 Changed 15 years ago by eriktorbjorn

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

comment:9 Changed 15 years ago by eriktorbjorn

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