Opened 16 years ago

Closed 14 years ago

#1191 closed defect (fixed)

MI1: Crash on church entry

Reported by: SF/durzel Owned by: Kirben
Priority: normal Component: Engine: SCUMM
Keywords: Cc:
Game: Monkey Island 1

Description

I am running ScummVM 0.5.1 on my SonyEricsson P800.
When I attempt to enter the church on Meleé Island
(the last act of the game), the game crashes (and my
phone) crashes.

This is reproducable - have tried entering the church
twice now and the game/phone has crashed both times.

I was able to enter the church previously in the game
(i.e. when no one was in there) but I can only assume
that there must be some kind of visual artifact giving
the phone problems.

I am using:
"Monkey Island 1 (256 color floppy version)"

Ticket imported from: #795214. Ticket imported from: bugs/1191.

Attachments (1)

save2.txt (658 bytes ) - added by Kirben 14 years ago.
Updated patch

Download all attachments as: .zip

Change History (37)

comment:1 by fingolfin, 16 years ago

Owner: set to fingolfin
Resolution: wontfix
Status: newclosed
Summary: MI1 - Crash on church entryMI1: Crash on church entry

comment:2 by fingolfin, 16 years ago

The SonyEricsson P800 port is currently not official and thus not
supported by us in any way. Please contact the person who made
the port available to you for support.

comment:3 by SF/dns25, 15 years ago

I can replicate the problem on linux using the same game
and revision on top of scummvm-0.7.0

The segfault occurs during bitmap decoding. If I make the vm
skip that particular resource, I can continue the game to
its end.

Backtrace:
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread -1212733056 (LWP 17154)]
Scumm::Gdi::decompressBitmap (this=0x8962b90,
dst=0x899661c
"ccddeeffgghhijkjklljcdcdefdghhiijjkiiklifgijlncdcffhijkl\201\201\201\201\201\200\200",
dstPitch=320,
src=0xbd564b2 <Address 0xbd564b2 out of bounds>,
numLinesToProcess=144) at scumm/gfx.cpp:1503
1503 byte code = *src++;
(gdb) bt
#0 Scumm::Gdi::decompressBitmap (this=0x8962b90,
dst=0x899661c
"ccddeeffgghhijkjklljcdcdefdghhiijjkiiklifgijlncdcffhijkl\201\201\201\201\201\200\200",
dstPitch=320,
src=0xbd564b2 <Address 0xbd564b2 out of bounds>,
numLinesToProcess=144) at scumm/gfx.cpp:1503
#1 0x080a81ac in Scumm::Gdi::drawBitmap (this=0x8962b90,
ptr=0x899661c
"ccddeeffgghhijkjklljcdcdefdghhiijjkiiklifgijlncdcffhijkl\201\201\201\201\201\200\200",
vs=0x896eae8, x=0,
y=0, width=24, height=144, stripnr=0, numstrip=3, flag=0
'\0', table=0x140) at scummsys.h:378
#2 0x080b708f in Scumm::ScummEngine::drawObject
(this=0x8962b30, obj=1, arg=0) at scumm/object.cpp:488
#3 0x080b6ce8 in Scumm::ScummEngine::drawRoomObject
(this=0x8962b30, i=1, arg=320) at scumm/object.cpp:377
#4 0x080b6db5 in Scumm::ScummEngine::drawRoomObjects
(this=0x8962b30, arg=0) at scumm/object.cpp:395
#5 0x080a6ab4 in Scumm::ScummEngine::redrawBGAreas
(this=0x8962b30) at scumm/gfx.cpp:631
#6 0x0805e856 in Scumm::ScummEngine::scummLoop
(this=0x8962b30, delta=6) at scumm/scumm.cpp:1676
#7 0x0805dd0d in Scumm::ScummEngine::go (this=0x8962b30) at
scumm/scumm.cpp:1436
#8 0x080560ea in runGame (detector=@0xbffff5d0,
system=0x894df10) at base/main.cpp:268
#9 0x080564cb in main (argc=3, argv=0xbffff6c4) at
base/main.cpp:390
(gdb)

so, the src offset calculated from the section header
delivered to Scumm::Gdi::decompressBitmap() seems broken.

any suggestions on how to fix this? i can provide more data
as soon as you guys tell me what i should be looking for.
attached are 4k of the SMAP starting from the offset as
drawBitmap() finds it.

comment:4 by SF/dns25, 15 years ago

I can replicate the problem on linux using the same game
and revision on top of scummvm-0.7.0

The segfault occurs during bitmap decoding. If I make the vm
skip that particular resource, I can continue the game to
its end.

Backtrace:
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread -1212733056 (LWP 17154)]
Scumm::Gdi::decompressBitmap (this=0x8962b90,
dst=0x899661c
"ccddeeffgghhijkjklljcdcdefdghhiijjkiiklifgijlncdcffhijkl\201\201\201\201\201\200\200",
dstPitch=320,
src=0xbd564b2 <Address 0xbd564b2 out of bounds>,
numLinesToProcess=144) at scumm/gfx.cpp:1503
1503 byte code = *src++;
(gdb) bt
#0 Scumm::Gdi::decompressBitmap (this=0x8962b90,
dst=0x899661c
"ccddeeffgghhijkjklljcdcdefdghhiijjkiiklifgijlncdcffhijkl\201\201\201\201\201\200\200",
dstPitch=320,
src=0xbd564b2 <Address 0xbd564b2 out of bounds>,
numLinesToProcess=144) at scumm/gfx.cpp:1503
#1 0x080a81ac in Scumm::Gdi::drawBitmap (this=0x8962b90,
ptr=0x899661c
"ccddeeffgghhijkjklljcdcdefdghhiijjkiiklifgijlncdcffhijkl\201\201\201\201\201\200\200",
vs=0x896eae8, x=0,
y=0, width=24, height=144, stripnr=0, numstrip=3, flag=0
'\0', table=0x140) at scummsys.h:378
#2 0x080b708f in Scumm::ScummEngine::drawObject
(this=0x8962b30, obj=1, arg=0) at scumm/object.cpp:488
#3 0x080b6ce8 in Scumm::ScummEngine::drawRoomObject
(this=0x8962b30, i=1, arg=320) at scumm/object.cpp:377
#4 0x080b6db5 in Scumm::ScummEngine::drawRoomObjects
(this=0x8962b30, arg=0) at scumm/object.cpp:395
#5 0x080a6ab4 in Scumm::ScummEngine::redrawBGAreas
(this=0x8962b30) at scumm/gfx.cpp:631
#6 0x0805e856 in Scumm::ScummEngine::scummLoop
(this=0x8962b30, delta=6) at scumm/scumm.cpp:1676
#7 0x0805dd0d in Scumm::ScummEngine::go (this=0x8962b30) at
scumm/scumm.cpp:1436
#8 0x080560ea in runGame (detector=@0xbffff5d0,
system=0x894df10) at base/main.cpp:268
#9 0x080564cb in main (argc=3, argv=0xbffff6c4) at
base/main.cpp:390
(gdb)

so, the src offset calculated from the section header
delivered to Scumm::Gdi::decompressBitmap() seems broken.

any suggestions on how to fix this? i can provide more data
as soon as you guys tell me what i should be looking for.
attached are 4k of the SMAP starting from the offset as
drawBitmap() finds it.

comment:5 by fingolfin, 15 years ago

Owner: fingolfin removed
Resolution: wontfix
Status: closednew

comment:6 by fingolfin, 15 years ago

We never were able to reproduce the problem on a supported
system, IIRC. If you can now, "good". Please tell us the exact
version of MI you have: VGA/EGA? How many floppies?

comment:7 by SF/dns25, 15 years ago

as already pointed out below, it is the 256-color VGA version.

monkeyVGA Monkey Island 1 (256 color Floppy
version) (German/PC)

no floppies. the game was installed from a collection cdrom
(MI1, MI2 + MI3 demo (iirc, I do not have the disk at hand))

directory listing:
dns@thinkpad:~/w32/programs/mi1$ l
total 4480
-r-xr-xr-x 1 dns dns 8357 2005-01-08 03:45 000.LFL
-r-xr-xr-x 1 dns dns 2572 2005-01-08 03:45 901.LFL
-r-xr-xr-x 1 dns dns 4023 2005-01-08 03:45 902.LFL
-r-xr-xr-x 1 dns dns 2572 2005-01-08 03:45 903.LFL
-r-xr-xr-x 1 dns dns 4782 2005-01-08 03:45 904.LFL
-r-xr-xr-x 1 dns dns 1101583 2005-01-08 03:45 DISK01.LEC
-r-xr-xr-x 1 dns dns 1118422 2005-01-08 03:45 DISK02.LEC
-r-xr-xr-x 1 dns dns 1176753 2005-01-08 03:45 DISK03.LEC
-r-xr-xr-x 1 dns dns 1048787 2005-01-08 03:45 DISK04.LEC
-r-xr-xr-x 1 dns dns 69637 2005-01-08 03:45 MONKEY.EXE
-r-xr-xr-x 1 dns dns 1417 2005-01-08 03:45 README.VGA

the README.VGA has some additional information on display
modes. roughly translated: If you do not own a VGA card but
only an EGA, running 'MONKEY E' utilizes a Hi-Res EGA mode
emulating a VGA colorspace on the EGA card.

Just added for completeness. Not sure whether this is a
special feature, I suppose it does not really mean much
about the bitmap coding. The church frame seems to be the
only scene in question exhibiting that behavior.

I'm not even sure whether it's a bitmap problem at all: As
mentioned, if I patch the vm to just skip that single
particular resource, the church scene still gets rendered
correctly, or at least does not miss anything obvious.

I've tried to force the smap into EGA-mode decoding. I'm not
to deep into the SCUMM data model, maybe this idea is a
little bit far off, but I thought it might worth a try. This
produces garbage (obviously), but does not crash or trap
into debug mode. Haven't spend more time looking at the
code, does this fact mean it could be data encoded for EGA?

regards,
daniel

comment:8 by fingolfin, 15 years ago

See also bug #1144865 (now closed as a duplicate).

comment:9 by fingolfin, 15 years ago

dns25, there is nothing attached from you (probably because
only the bug submitter or project members can attach to the
bug). If you email me the SMAP dump, I can attach it for you
(and take a look at it).

comment:10 by eriktorbjorn, 15 years ago

Hmm... for some reason the savegame that's attached to bug
#1144865 no longer works for me. The game won't let me enter
the church. Or leave the room for that matter. It's not that
it crashes, it's just as if it doesn't recognize that there
are any exits from the room.

I know an extension was made to the savegame format
recently, but I don't see how that would change anything
like that...

comment:11 by eriktorbjorn, 15 years ago

In fact, I get no mouse-over texts in that room at all with
the savegame. I forget, is it the game engine or the game
scripts that are responsible for deciding when to update the
status line?

comment:12 by eriktorbjorn, 15 years ago

Never mind, I found out that it's the game scripts that
handles that. I think I have fixed the regression when
loading old savegames. Entering the church now triggers the
following assertion:

scummvm: scumm/gfx.cpp:1381: void
Scumm::Gdi::drawBitmap(const byte*, Scumm::VirtScreen*, int,
int, int, int, int, int, unsigned char, Scumm::StripTable*):
Assertion `offset < READ_LE_UINT32(smap_ptr)' failed.

At this point, stripnr is 0 (which I suppose is reasonable),
offset is 53758287 (which isn't) and smap_ptr points to data
that begins with

000000: 08 00 00 00 4f 49 34 03 80 00 00 00 4f 49 35 03
|....OI4.....OI5.|
000010: 6e 00 00 00 0c 00 00 00 3c 00 00 00 12 58 80 98
|n.......<....X..|
000020: a0 29 c2 82 98 08 02 c2 6a 2a a6 d6 10 40 10 4d
|.)......j*...@.M|
000030: c5 94 59 68 a9 09 20 88 a6 62 4a 2c b2 cc 42 0b
|..Yh.. ..bJ,..B.|
000040: 20 50 31 35 59 e4 02 0d 4c d0 01 00 1c 97 01 24
| P15Y...L......$|
000050: 96 59 10 61 55 a5 ca 00 42 cb 2c 88 b0 aa 52 59
|.Y.aU...B.,...RY|
000060: 00 a9 09 22 ac aa 04 1a 00 41 84 55 55 82 81 26
|...".....A.UU..&|
000070: 91 60 58 54 25 50 4d 19 0b 2c 89 00 00 00 0a 00
|.`XT%PM..,......|

comment:13 by fingolfin, 15 years ago

Thanks for fixing the savegames, Erik, good work as usual
;-)

As you probably know, I added that asser int drawBitmap
recently, because of this very bug; glad to hear that it
catches (although of course it doesn't solve the issue...)

Now, regarding the hexdump you pasted: It contains "OI"
(object image) tags (corresonds to "OBIM" in modern games);
and the first four bytes contain a size value of "8". Maybe
we are dealing with an essentially empty "BM" resource here?
Hard to say w/o some more information..

Can you say what comes just *before* the smap_ptr ? I.e. a
hexdump starting at "smap_ptr-16" might be useful.

Also, I assume drawBitmap is still being called from
drawObject(obj=1, arg=0), right? In that case, it would be
good to check out the object we are drawing. So, what data
does _objs[1] contain? Using that information, you might be
able to set a breakpoint at the start of drawObject() that
triggers when the problematic object is being drawn; and
we can then step through the code to see what exactly is
the problem (A bug in our code? A buggy datafile?).

comment:14 by eriktorbjorn, 15 years ago

I'm not really familiar with the data layout, but here's a
hexump of smap_ptr - 16:

000000: 1f 0f 07 01 8f 00 00 00 08 00 00 00 4f 49 33 03
|............OI3.|
000010: 08 00 00 00 4f 49 34 03 80 00 00 00 4f 49 35 03
|....OI4.....OI5.|
000020: 6e 00 00 00 0c 00 00 00 3c 00 00 00 12 58 80 98
|n.......<....X..|
000030: a0 29 c2 82 98 08 02 c2 6a 2a a6 d6 10 40 10 4d
|.)......j*...@.M|
000040: c5 94 59 68 a9 09 20 88 a6 62 4a 2c b2 cc 42 0b
|..Yh.. ..bJ,..B.|
000050: 20 50 31 35 59 e4 02 0d 4c d0 01 00 1c 97 01 24
| P15Y...L......$|
000060: 96 59 10 61 55 a5 ca 00 42 cb 2c 88 b0 aa 52 59
|.Y.aU...B.,...RY|
000070: 00 a9 09 22 ac aa 04 1a 00 41 84 55 55 82 81 26
|...".....A.UU..&|

comment:15 by eriktorbjorn, 15 years ago

As far as I can tell it still crashes in the same way as
before, and this appears to be the contents of _objs[1].
(Though this MinGW shell isn't an ideal debugging
environment, if you ask me, so I could have made some
mistake... :-)

_objs[1] = {
OBIMoffset = 22631
OBCDoffset = 25914
walk_x = -25
walk_y = 143
obj_nr = 819
x_pos = 0
y_pos = 0
width = 24
height = 144
actordir = 0
parent = 0
parentstate = 0
state = 1
fl_object_index = 0
flags = 0
}

comment:16 by fingolfin, 15 years ago

Very strange. I inserted some code to print out info about 819
in my copy in MonkeyVGA (german). Turns out the object is
named "Ausgang" (="Exit"), and has the same _objs specs as in
your case. It is in fact one of the two "objects" which form
the exit of that room on the left and right sides. One can
verify that by entering the church earlier in the game.
Nothing to be drawn for them; the OI (=OBIM) data looks
exactly the same for me as it does in your hexdump.

The question now is: why does it try to "draw" that object
with that savegame, but not when with my own savegame (or when
I start with bootparam 8888 and play till that point) ?

Looking a bit further, there *is* a difference: for you,
od.state is 1, for me it is 0. That explains why it's being
drawn in one case but not the other. The question remains:
why is it 1 in that savegame?

Maybe this is just a case of a corrupt old savegame; maybe it has
a deep reason? In either case, we could add a workaround by
changing the new asserts in drawBitmap to checks which
abort from the method if the object being drawn has no strip data?

comment:17 by fingolfin, 15 years ago

Owner: set to fingolfin

comment:18 by fingolfin, 15 years ago

I improved the checking code in drawBitmap(); and changed
the asserts to warnings. As a result, we now just produce a
warning and go on undisturbed otherwise. If somebody could
verify that this works as intended, please...

comment:19 by eriktorbjorn, 15 years ago

Works well enough for that particular savegame at least. I
didn't notice anything missing from the church scene, and I
was able to finish the game.

comment:20 by fingolfin, 15 years ago

Yeah, the exit is invisible, anyway, so nothing should be missing.
My hack simply helps to reuse some broken savegames. Of course
the broken object state is a bit worrying, but for now, this is the
best we can do, I think.

comment:21 by fingolfin, 15 years ago

Resolution: fixed
Status: newclosed

comment:22 by SF/madshark, 14 years ago

Hi people!

I had the same problem with scummVM 0.8.0 in a PC with WinXP
SP2, but the FIRST time the problem arised was just after
freeing the guy in prison playing with scummvm 0.7.x

When i met the same error at last chapter, I tried ScummVM
0.8.0, that said:

ERROR: DrawBitmap: "trying to draw non-existant strip"

what can I do???
It's supposed not to happen with this version of scummVM?

Can I skip this error? how?
Should I download a savegame somewhere?

thanks!

Mine is floppy version, 4 disks I think. VGA Spanish.

comment:23 by fingolfin, 14 years ago

Indeed. Seems Kirben did that when he changed lots of warnings to errors... See
<http://cvs.sourceforge.net/viewcvs.py/scummvm/scummvm/scumm/gfx.cpp?
r1=2.466&r2=2.467>

Kirben, please re-fix this.

comment:24 by fingolfin, 14 years ago

Owner: changed from fingolfin to Kirben
Status: closednew

comment:25 by SF/madshark, 14 years ago

Hi again!

I write for dummies!! (I'm almost one...)
I have "solved" it as "eriktorbjorn" said in other topic;
using scummVM from the command line (inside Accesories,
the black screen), with the parameter bootparam -b# before
the path of the game files; f.ex.

C:\...\scummvm.exe -f -b8888 -pC:\monkey monkey

where -f corresponds to Full Screen mode,
C:\...\ is the path were you installed scummVM,
and C:\monkey is the path were you installed the game,
of course! ^^)

With -b8888 you appear just before going from monkey
island to the church, but it corresponds to the story in
which you HAVE destroyed your ship with the catapult.

If you HAVEN'T destroyed it, the bootparam is -b8889; the
one that "eriktorbjorn" gave.

This way you can see the two different endings (I don't
know if there are more endings) easily.

I love this program!! THANKS for all the efforts!!!

(how do you find the bootparams that work???)

comment:26 by Kirben, 14 years ago

Reading the notes, it sounds like changes were made to
support old broken saved games only. I don't think we
should be supporting broken or corrupt saved games at all.

If the problem can still occur with recent saved games, the
real cause should be found.

comment:27 by fingolfin, 14 years ago

madshark: So, from when is your savegame? Is it "new" or
old?

Kirben: I don't quite agree. Yeah, from a "pure" point of
view, we shouldn't let in *any* hacks to support broken
savegames, broken games, etc..

But I disagree with the notion that using "error" every will
do that. It has it's benefits, and I strive to follow this
ideal where it is makes sense, but IMHO in this case it
doesn't make sense to deliberately choose to break old
*slightly* damaged savegames by reacting to them with a
*crash* when we *know* that we can safely and trivially
compensate for them.

Yes, *if* the problem occurs with recent savegames, we need
to analyze the issue more. But we won't find out by
switching a safe warning to a hard error: People will just
be frustrated by a crash, lowering our reputation. By
experience I can say that only a tiny percentage of them
will ever report the crash.

Much better would be to add code that tracks whether the
current game was loaded from a savegame, and what version
the format of that savegame was. If it's an old savegame,
just print the warning. If it's a new savegame, either show
a dialog telling the user to report this, or still use an
error, but insert an explicit request to report the crash.

So I see three choices: 1) (bad) Stay at error. Frustrate
users, probably gain nothing 2) (good) Revert back to
warning, maybe with a check added for this specific bug
scenario (otherwise it could still be a warning). Keeps
things working, users are happy, and with the restricted
hack we don't even loos the hypothetical advantage of 1) 3)
(best, but expensive) Implement the check & handling
described above.

comment:28 by SF/madshark, 14 years ago

Well, I suppose its what you consider "new", but i'm not
sure so I'll explain the thing.

I installed the game (had never played it before), use
scummVM 0.7.0 and it worked perfectly. I played it from
the beggining, saving some times, and when I freed the guy
in prison, I saved but continue playing without quiting. I
talked to the person that had appeared in the left arc, I
went to the church, that HAD THE DOORS OPENED YET (for if
it had something to do), and the game crashed at entering.

So I loaded just after freeing the guy, and I evaded the
church. I finally ended chapter three, went to the church,
the doors were still opened (that scared me, for the
continuity, that could mean continuity on the "bug")
and... it crashed again. Tried to close the door and
stupid things like that, but crashed anyway when I entered
it.

I downloaded scummVM 0.8.0, loaded the last save, and the
famous error appeared:
ERROR: DrawBitmap: "trying to draw non-existant
strip"

I read this forum, and find the bootparam trick. It worked.

Good luck!

comment:29 by Kirben, 14 years ago

The problem with supporting broken or corrupt saved games,
is we can't always be sure there aren't other issues caused
too.

It would be a better idea to just add an additional check
when loading the old broken saved games. Specifically check
if object 819 is set to state 1, and set that object back
to state 0. See the attached patch.

comment:30 by fingolfin, 14 years ago

Kirben, sorry, I didn't notice you attached a patch.

That patch will only work if you are already inside the
church. Given that it crashes once you enter it, I don't
think many people who need the fix will have a savegame
there... :-).

You really would want to add a "putState(819, 0)" there,
instead of the loop. And of course a "WORKAROUND bug #XYZ"
comment etc. would be needed.

comment:31 by Kirben, 14 years ago

I'm not sure if work around should be applied to all
languages, or just the German version?

I attached an updated patch, please commit, if it looks
fine.

comment:32 by Kirben, 14 years ago

Resolution: fixed

by Kirben, 14 years ago

Attachment: save2.txt added

Updated patch

comment:33 by fingolfin, 14 years ago

We know that it occurs at the very least with the german and the spanish
versions. I don't think that the fix should be limited by the language. In theory it
shouldn't cause any problems, ever, so there is no need to limit it either...

So the patch is now in SVN. If anybody with a savegame affected by this problem
could try a daily build tomorrow to verify if it's gone, that would be nice... :-)

comment:34 by fingolfin, 14 years ago

Resolution: fixed
Status: newpending

comment:35 by SF/sf-robot, 14 years ago

Status: pendingclosed

comment:36 by SF/sf-robot, 14 years ago

This Tracker item was closed automatically by the system. It was
previously set to a Pending status, and the original submitter
did not respond within 14 days (the time period specified by
the administrator of this Tracker).

Note: See TracTickets for help on using tickets.