Opened 17 years ago

Closed 16 years ago

Last modified 22 months ago

#1403 closed defect (fixed)

PASS: Invalid write in Loom demo

Reported by: Kirben Owned by: fingolfin
Priority: normal Component: Engine: SCUMM
Keywords: Cc:
Game: Loom

Description

Latest ScummVM cvs version. Passport to Adventure Compiled under mingw with GCC 3.2.3 and running under Windows XP.

Valgrind reports an invalid write in setBoxFlags() when Bobbin enters the tent heading to the Elders. A log is attached.

Ticket imported from: #881132. Ticket imported from: bugs/1403.

Attachments (1)

loom_pass.txt (1.0 KB ) - added by Kirben 17 years ago.
Valgrind log

Download all attachments as: .zip

Change History (7)

by Kirben, 17 years ago

Attachment: loom_pass.txt added

Valgrind log

comment:1 by fingolfin, 17 years ago

That's obviously a direct consequence of the evil hack in that part.

comment:2 by Kirben, 17 years ago

OK, I only mentioned it as ScummVM crashes at that point in Loom demo of PASS sometimes, but not always.

comment:3 by fingolfin, 17 years ago

Aye. Well that hack clearly *is* wrong, anyway :-)

Either, the value should be "clipped" (that is, something like if (ptr[0] == box) box--; That's certainly not correct in any meaning of the word, but at least won't crash and is trivial to do any verify.

Or we should enlarge the rtMatrix-1 resource... we could for example make it exactly one box "bigger", and then fill that additional space with the data that follows in the resource file immediately after the box data (that will be the res matrix). That way, we'd essentially emulate the (buggy) behaviour of the original engine.

comment:4 by Kirben, 16 years ago

Adding that clip check to getBoxBaseAddr() for pass works and stops the valgrind warning. That clip check could replace the current checkRange() adjustment for earlier games. Maybe just add that clip check for scumm 1 - 4 games in getBoxBaseAddr(), replacing the current fixme ?

It might be best to emulate the buggy behaviour of original as you mentioned, so we end up with exact same data for boxes. But I will leave the decision up to you.

comment:5 by Kirben, 16 years ago

Owner: set to fingolfin
Resolution: fixed
Status: newclosed

comment:6 by digitall, 22 months ago

Component: Engine: SCUMM
Game: Loom
Note: See TracTickets for help on using tickets.