Opened 15 years ago

Closed 15 years ago

Last modified 5 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 15 years ago.
Valgrind log

Download all attachments as: .zip

Change History (7)

Changed 15 years ago by Kirben

Attachment: loom_pass.txt added

Valgrind log

comment:1 Changed 15 years ago by fingolfin

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

comment:2 Changed 15 years ago by Kirben

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

comment:3 Changed 15 years ago by fingolfin

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 Changed 15 years ago by Kirben

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 Changed 15 years ago by Kirben

Owner: set to fingolfin
Resolution: fixed
Status: newclosed

comment:6 Changed 5 months ago by digitall

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