Opened 15 years ago

Closed 15 years ago

Last modified 6 years ago

#1780 closed defect (fixed)

SCUMM: Invalid write when drawing charset to background

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

Description

Latest ScummVM cvs version.
Compiled under mingw with GCC 3.4.1 and running under
Windows XP.

I noticed ScummVM crashes during the scrolling credits,
at the end of HE72+ games. I tried running ScummVM
under valgrind and it reports several invalid writes in
charset code. Around the point of scumm/charset.cpp
where comments mention code might be broken.
Adding bug report since it looks like general issue in
ScummVM and not a bug specific to the currently
unsupported HE 72+ games.

Attached gdb backtrace and valgrind log. Also a
screenshot of scrolling credits in a HE game in case it
helps too.

Ticket imported from: #1033857. Ticket imported from: bugs/1780.

Attachments (4)

credits.png (43.1 KB ) - added by Kirben 15 years ago.
Screenshot from PJS2
credits.txt (14.4 KB ) - added by Kirben 15 years ago.
Valgrind log
gdb.txt (2.8 KB ) - added by Kirben 15 years ago.
GDB back trace
pjs.txt (16.4 KB ) - added by Kirben 15 years ago.
PJS1 Valgind log with gdb backtrace

Download all attachments as: .zip

Change History (16)

by Kirben, 15 years ago

Attachment: credits.png added

Screenshot from PJS2

by Kirben, 15 years ago

Attachment: credits.txt added

Valgrind log

by Kirben, 15 years ago

Attachment: gdb.txt added

GDB back trace

comment:1 by fingolfin, 15 years ago

Hm, the FIXME code close to your crash is indeed potentially
wrong -- but not in a crashy way, only in the sense that it might
produce something which looks wrong.
What *does* look fishy, though, is that drawTop is apparently -1.
It might be interesting to know the values of vs->topline and
_top at the time it crashes...

comment:2 by Kirben, 15 years ago

I add some printfs just before crash point to check and last
few outputs are:
vs->topline 0 _top 1
vs->topline 0 _top 0
vs->topline 0 _top 0
vs->topline 0 _top 0
vs->topline 0 _top 2
vs->topline 0 _top 0
vs->topline 0 _top 20
vs->topline 0 _top 0
vs->topline 0 _top 0
vs->topline 0 _top 2
vs->topline 0 _top 20
vs->topline 0 _top 2
vs->topline 0 _top -1

comment:3 by Kirben, 15 years ago

The drawTop value of -1 at crash point, is caused by offsY.
The initial _top value is 2 but a offsY value of of -3 is added.

comment:4 by fingolfin, 15 years ago

Aha, so it's simply text being drawn outside the screen,
causing a memory a overwrite. The drawing code probably
isn't doing clipping properly in some cases...

comment:5 by Kirben, 15 years ago

I noticed a recent CVS log mentioned it might fix this issue,
but it doesn't help.

comment:6 by Kirben, 15 years ago

Resolution: fixed
Status: newclosed

by Kirben, 15 years ago

Attachment: pjs.txt added

PJS1 Valgind log with gdb backtrace

comment:7 by Kirben, 15 years ago

I didn't re-test enough, the credits in Pajama Sam 2 are
fixed but not the credits in Pajama Sam 1, so leaving open.
Attached Valgrind log and gdb backtrace of when invalid
write occurs in Pajama Sam 1 credits.

comment:8 by Kirben, 15 years ago

Resolution: fixed
Status: closednew

comment:9 by fingolfin, 15 years ago

Did my last changes help any?

comment:10 by Kirben, 15 years ago

Yes fixed now, I meant to close this one again.

comment:11 by Kirben, 15 years ago

Resolution: fixed
Status: newclosed

comment:12 by Strangerke, 6 years ago

Component: --Unset--Engine: SCUMM
Owner: fingolfin removed
Note: See TracTickets for help on using tickets.