#3806 FW: crash with italian amiga version

[glorifindel]

I'm using the current SVN build on win32 and the italian amiga version of Future Wars; when I click on "ok" in the pre-copy protection screen, ScummVM crash with this message:

Assertion failed: idx < _size, file engines/cine/script_fw.cpp, line 273

This application has requested the Runtime to terminate it in an unusual way.
Please contact the application's support team for more information.

[Eugene Sandulenko]

Logged In: YES
user_id=166507
Originator: NO

Kari, could you take a look at it. This bug is nice to get fixed before the release.

[Kari Salminen]

Logged In: YES
user_id=1803212
Originator: NO

I could reproduce this bug exactly with the Italian Amiga version.
It seems opcode 0x0E (i.e. o1_compareVar) is called with parameters
0xFB (Variable index), 0x00 (Variable type), 0x0000 (Immediate value).
So the following line is executed in o1_compareVar:
_compare = compareVars(_localVars[varIdx], value);
And as _localVars has only 50 elements and varIdx is over that
(It's 0xFB i.e. 251) there is an out of array bounds access happening
here and that's what makes the assertion go off.

Checked the Italian Amiga version's disassembly (Woah, haven't done
almost any Motorola 68k disassembly before), found the opcode list
and checked o1_compareVar's implementation. Looking at the branch
where the variable type is zero (It's the branch that's throwing
the assertion here) it looks identical to that branch's implementation
in ScummVM. So can't find fault there...

On another note the branch where variable type is not
zero it looks like there's only the implementation for comparing a
local variable against another local variable, no global variables
anywhere to be seen. And it's also the code that's *always* taken if
variable type is anything else than zero - no tests for 1 or 2! This
differs from the current implementation in ScummVM.

My next idea would be to check the scripts to see what's happening
around the offending script position (What opcodes are executed etc).

So there's my current investigation into this bug. Couldn't find the
cause yet but learned a bit of Motorola 68k assembly :-).

[Kari Salminen]

Logged In: YES
user_id=1803212
Originator: NO

The automatically executed script file AUTO00.PRC
calls CODE2.PRC from inside the PART04B part file.
Inside CODE2.PRC there's a script of which here is a small part:

// script.prc 1 size 0x870
<snip>
[01F0] (09) localvars[1] = mouse.x
[01F4] (09) localvars[2] = mouse.y
[01F8] (09) localvars[0] = globalvars[251]
[01FC] (0A) ADD localvars[0], localvars[1]
[0200] (0C) MUL localvars[0], localvars[2]
[0204] (52) SET globalvars[251], localvars[0]
[0208] (0E) CMP localvars[251], 0
</snip>

So it calculates something and stores the value in
global variable number 251. But then it goes and compares
*local* variable 251 instead of the global one. Seems
like a typing mistake. They probably meant to test the
global variable number 251.

Checked an English Amiga version and an Atari ST version
and they both used the identical CODE2.PRC file so the
problem is there too.

In the PC version AUTO00.PRC calls TOTO.PRC from inside
PART01 part file. There's no localVars[251] access in it
anywhere that I can see so no problem there.

So my conclusion is that Atari ST and Amiga versions of
Future Wars have a scripting bug that tries to access a
local variable 251 when it wants to access global variable
251. I'm looking into fixing this next...

[Kari Salminen]

Logged In: YES
user_id=1803212
Originator: NO

Fixed in the trunk in revision 33068:
http://scummvm.svn.sourceforge.net/viewvc/scummvm?view=rev&revision=33068

[Kari Salminen]

Logged In: YES
user_id=1803212
Originator: NO

Ah, here's a description of the fix too:

WORKAROUND: A workaround for a script bug in script file CODE2.PRC
in at least some of the Amiga and Atari ST versions of Future Wars.
Fixes bug #2016647 (FW: crash with italian amiga version). A local
variable 251 is compared against value 0 although it's quite apparent
from the context in the script that instead global variable 251 should
be compared against value 0. So looks like someone made a typo when
making the scripts. Therefore we change that particular comparison
from using the local variable 251 to using the global variable 251.

I also added a fix in the same trunk revision for making ScummVM not
crash when failing copy protection in an Amiga or Atari ST version of
Future Wars.