#3806 closed defect (fixed)
FW: crash with italian amiga version
| Reported by: | SF/glorifindel | Owned by: | SF/buddha_ |
|---|---|---|---|
| Priority: | high | Component: | Engine: Cine |
| Version: | Keywords: | script | |
| Cc: | Game: | Future Wars |
Description
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.
Ticket imported from: #2016647. Ticket imported from: bugs/3806.
Change History (8)
comment:1 by , 16 years ago
comment:2 by , 16 years ago
| Owner: | set to |
|---|---|
| Summary: | FUTURE WARS: crash with italian amiga version → FW: crash with italian amiga version |
comment:3 by , 16 years ago
| Priority: | normal → high |
|---|
comment:4 by , 16 years ago
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 :-).
comment:5 by , 16 years ago
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...
comment:6 by , 16 years ago
Fixed in the trunk in revision 33068: http://scummvm.svn.sourceforge.net/viewvc/scummvm?view=rev&revision=33068
comment:7 by , 16 years ago
| Keywords: | script added |
|---|---|
| Resolution: | → fixed |
| Status: | new → closed |
comment:8 by , 16 years ago
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.
Kari, could you take a look at it. This bug is nice to get fixed before the release.