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

[Jonathan Gevaryahu]

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

[Jonathan Gevaryahu]

Savegame of Dave standing in front of gym door.

[Max Horn]

Logged In: YES
user_id=12935

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!

[Torbjörn Andersson]

Logged In: YES
user_id=577918

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.

[Jonathan Gevaryahu]

Logged In: YES
user_id=278245

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

[Torbjörn Andersson]

Logged In: YES
user_id=577918

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.

[Torbjörn Andersson]

Logged In: YES
user_id=577918

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.

[Torbjörn Andersson]

Logged In: YES
user_id=577918

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.

[Max Horn]

Logged In: YES
user_id=12935

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 :-)

[Torbjörn Andersson]

Logged In: YES
user_id=577918

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.

[Gregory Montoir]

[Gregory Montoir]

Logged In: YES
user_id=230314

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.

[Gregory Montoir]

Logged In: YES
user_id=230314

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...

[Max Horn]

Logged In: YES
user_id=12935

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

Applied to CVS. Good work!