Opened 19 years ago

Closed 19 years ago

#2028 closed defect (fixed)

MANIAC64: crash after using hunk-o-matic and pressing ESC

Reported by: SF/lord_nightmare Owned by: cyxx
Priority: high Component: Engine: SCUMM
Version: Keywords:
Cc: Game: Maniac Mansion

Description

After using the hunk-o-matic machine as Dave (maybe as others too, haven't tried it), pressing ESC causes scummvm to crash with a: Fatal signal: Segmentation Fault (SDL Parachute Deployed) message. This happens even if you switch kids after using the hunk-o-matic machine. I've attached a savefile of Dave standing outside the gym door, just open the door, go in, use the machine, and hit esc to observe the crash. This crash DOES happen even if you don't restore the game from a savefile, I tried it.

The gdb backtrace is this: Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 16384 (LWP 24507)] 0x0810acdc in Scumm::ScummEngine::fetchScriptByte (this=0x8425180) at scumm/script.cpp:468 /home/jonathan/scummvm/scumm/script.cpp:468:11360:beg:0x810acdc (gdb) bt #0 0x0810acdc in Scumm::ScummEngine::fetchScriptByte (this=0x8425180) at scumm/script.cpp:468 #1 0x0810abbe in Scumm::ScummEngine::executeScript (this=0x8425180) at scumm/script.cpp:444 #2 0x0810bd56 in Scumm::ScummEngine::runAllScripts (this=0x8425180) at scumm/script.cpp:799 #3 0x08063eaf in Scumm::ScummEngine::scummLoop (this=0x8425180, delta=4) at scumm/scumm.cpp:2183 #4 0x080633bf in Scumm::ScummEngine::go (this=0x8425180) at scumm/scumm.cpp:1964 #5 0x080591e0 in runGame (detector=@0xbffffc40, system=@0x8416fb0) at base/main.cpp:299 #6 0x080597b2 in main (argc=3, argv=0xbffffcf4) at base/main.cpp:436

The MD5 of 00.lfl: 7f45ddd6dbfbf8f80c0c0efea4c295bc ./maniac/00.lfl

Lord Nightmare

Ticket imported from: #1202487. Ticket imported from: bugs/2028.

Attachments (2)

maniac.s01 (3.5 KB ) - added by SF/lord_nightmare 19 years ago.
Savegame of Dave standing in front of gym door.
maniac_1202487.diff (516 bytes ) - added by cyxx 19 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 by sev-, 19 years ago

Summary: scummvm crashes after using hunk-o-matic and pressing ESCMM:crash after using hunk-o-matic and pressing ESC

by SF/lord_nightmare, 19 years ago

Attachment: maniac.s01 added

Savegame of Dave standing in front of gym door.

comment:2 by fingolfin, 19 years ago

To process your bug report appropriately, we need you to provide the following additional information:

* ScummVM version (PLEASE test the latest CVS/Daily build) * Bug details, including instructions on reproducing it * Language of game (English, German, ...) * Version of game (talkie, floppy, ...) * Platform and Compiler (Win32, Linux, MacOS, ...) * Attach a save game if possible * If this bug only occurred recently, please note the last version without the bug, and the first version including the bug. That way we can fix it quicker by looking at the changes made.

This should only take you a little time but will make it much easier for us to process your bug report in a way that satisfies both you and us.

Thank you for your support!

comment:3 by fingolfin, 19 years ago

Summary: MM:crash after using hunk-o-matic and pressing ESCMANIAC64: crash after using hunk-o-matic and pressing ESC

comment:4 by eriktorbjorn, 19 years ago

I can reproduce this with the latest CVS snapshot and the version of Maniac Mansion that was included with my copy of Day of the Tentacle.

comment:5 by SF/lord_nightmare, 19 years ago

aw crud. forgot to mention, this was with 2005-05-15 16:33 CVS In response to canned response by fingolfin: game version is english, dos floppy version of MM (the MD5 should show that anyhow) Platform is Linux I haven't tested this in older versions of ScummVM, so if it is a regression I don't know when it started.

Lord Nightmare

comment:6 by eriktorbjorn, 19 years ago

I think I see why it happens, but I'm not sure what the correct behaviour is. This is what script-29 does:

[0000] (0C) loadSound(30) [0003] (0C) loadSound(31) [0006] (58) beginOverride() [0007] (18) goto 002F; [000A] (60) cursorCommand(3842) [000D] (1A) Var[111] = 5; [0011] (07) setState08(111) [0014] (80) breakHere() [0015] (1C) startSound(30) [0017] (2E) delay(45) [001B] (47) clearState08(111) [001E] (80) breakHere() [001F] (1C) startSound(31) [0021] (2E) delay(45) [0025] (3A) Var[111] -= 1; [0029] (48) unless (Var[111] == 0) goto 0011; [002F] (B1) Var[1] = getBitVar(2944,VAR_EGO) [0034] (48) if (Var[1] == 0) { [003A] (9B) setBitVar(2944,VAR_EGO,1) [003F] (D8) printEgo("Ah. I feel much stronger now!") [0059] (18) } else { [005C] (D8) printEgo("I've had enough!") [006C] (**) } [006C] (60) cursorCommand(-2303) [006F] (A0) stopObjectCode() END

The beginOverride opcode will cause vm.cutScenePtr[0] to be set to zero and vm.cutSceneScript[0] is set to _currentScript.

But as far as I can tell, nothing resets these variables. So when the user hits Escape, it tries to abort a cutscene in a script that probably isn't valid any longer.

At least, one of the last things that happens is that it tries to run a script where the 'number' field is 0.

comment:7 by eriktorbjorn, 19 years ago

Sorry, what I meant to say was:

The beginOverride opcode will cause vm.cutScenePtr[0] to be set to 11 and vm.cutSceneScript[0] is set to _currentScript.

comment:8 by eriktorbjorn, 19 years ago

To clarify a bit, I'm suspecting it may be a script bug. At least, I thought scripts with a beginOverride opcode were supposed to also have an endCutscene opcode. This script doesn't.

Though that's just a guess. I don't know enough about how this works to say for sure.

comment:9 by fingolfin, 19 years ago

Priority: normalhigh

comment:10 by fingolfin, 19 years ago

Our o2_beginOverride is directly based on the ZAK version of the engine (Zak V2 I think). Note that cutscenes are handled differntly in V1/V2, in particular, there is no cutscene 'stack'.

So there is no strong need to have a begin/endCutscene opcode pair along with the beginOverride. Or at least, there shouldn't be :-).

AFAIK, our code does exactly what the Zak V2 code did. I see two possibilities: * MM V1 did handle overrides a bit differently * the original engine crashed, too. Or maybe didn't crash due to luck :-)

comment:11 by eriktorbjorn, 19 years ago

The same thing happens with enhanced (v2) Maniac Mansion as well. I don't have any savegame for it on this computer, but you can get there easily with the debugger: it's room 25.

by cyxx, 19 years ago

Attachment: maniac_1202487.diff added

comment:12 by cyxx, 19 years ago

I ran maniac v2 under the dosbox debugger and if we try to ESCape this cutscene, the 'overriden' script (5) is restarted with offset = 11. Exactly like in scummvm, so maniac doesn't seem to handle override differently than other games.

The reason why the original version doesn't crash is due to the fact that their getScriptBaseAddress() function verifies that the script resource is still in memory. If not, the script slot is marked as ssDead. My patch adds this extra check.

comment:13 by cyxx, 19 years ago

Err.. I actually meant 'if we try to hit ESC *after* the cutscene'.

And looking again at that patch, checking for '_scriptOrgPointer' is somewhat wrong, we could just check that we're not trying to execute a script slot whose number is 0.

So to sum up, here is how the original behaves : - the cutscene terminates - hitting ESC, we enter abortCutscene() - it enables again the script 5 - runAllScripts() is called, script 5 is restarted - in getScriptBaseAddress(), a check is made to see if script 5 is still in memory - by chance, it isn't anymore ; _currentScript is set to 0xFF - executeScript() is not called, no crash

So I don't know what is the best 'solution'. Maybe add a hack to reset the override vars at the end of this particular custcene ? Or to behave like the original, check that the script data is still valid, checking for ss->number != 0 in getScriptBaseAddress ?

But as Erik said, what happens here seems definitely a scripting error, other scripts have cutscene() and end_custcene() calls, which reset the override vars and won't lead to such situation...

comment:14 by fingolfin, 19 years ago

Owner: set to cyxx
Resolution: fixed
Status: newclosed

comment:15 by fingolfin, 19 years ago

The patch looks fine enough, and matches the original AFAICT.

Applied to CVS. Good work!

Note: See TracTickets for help on using tickets.