Opened 16 years ago

Closed 15 years ago

Last modified 12 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 16 years ago.
Valgrind log

Download all attachments as: .zip

Change History (7)

by Kirben, 16 years ago

Attachment: loom_pass.txt added

Valgrind log

comment:1 by fingolfin, 16 years ago

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

comment:2 by Kirben, 16 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, 16 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, 15 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, 15 years ago

Owner: set to fingolfin
Resolution: fixed
Status: newclosed

comment:6 by digitall, 12 months ago

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