Opened 20 years ago

Closed 20 years ago

Last modified 11 years ago

#1780 closed defect (fixed)

SCUMM: Invalid write when drawing charset to background

Reported by: Kirben Owned by:
Priority: normal Component: Engine: SCUMM
Version: 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 20 years ago.
Screenshot from PJS2
credits.txt​ (14.4 KB ) - added by Kirben 20 years ago.
Valgrind log
gdb.txt​ (2.8 KB ) - added by Kirben 20 years ago.
GDB back trace
pjs.txt​ (16.4 KB ) - added by Kirben 20 years ago.
PJS1 Valgind log with gdb backtrace

Download all attachments as: .zip

Change History (16)

by Kirben, 20 years ago

Attachment: credits.png​ added

Screenshot from PJS2

by Kirben, 20 years ago

Attachment: credits.txt​ added

Valgrind log

by Kirben, 20 years ago

Attachment: gdb.txt​ added

GDB back trace

comment:1 by fingolfin, 20 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, 20 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, 20 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, 20 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, 20 years ago

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

comment:6 by Kirben, 20 years ago

Resolution: → fixed
Status: new → closed

by Kirben, 20 years ago

Attachment: pjs.txt​ added

PJS1 Valgind log with gdb backtrace

comment:7 by Kirben, 20 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, 20 years ago

Resolution: fixed
Status: closed → new

comment:9 by fingolfin, 20 years ago

Did my last changes help any?

comment:10 by Kirben, 20 years ago

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

comment:11 by Kirben, 20 years ago

Resolution: → fixed
Status: new → closed

comment:12 by Strangerke, 11 years ago

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