Opened 17 years ago

Closed 15 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 15 years ago.
Updated patch

Download all attachments as: .zip

Change History (37)

comment:1 by fingolfin, 17 years ago

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

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

Owner: fingolfin removed
Resolution: wontfix
Status: closednew

comment:6 by fingolfin, 16 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, 16 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, 16 years ago

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

comment:9 by fingolfin, 16 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, 16 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, 16 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, 16 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, 16 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, 16 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, 16 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, 16 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, 16 years ago

Owner: set to fingolfin

comment:18 by fingolfin, 16 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, 16 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, 16 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, 16 years ago

Resolution: fixed
Status: newclosed

comment:22 by SF/madshark, 15 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, 15 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, 15 years ago

Owner: changed from fingolfin to Kirben
Status: closednew

comment:25 by SF/madshark, 15 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, 15 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, 15 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, 15 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, 15 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, 15 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, 15 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, 15 years ago

Resolution: fixed

by Kirben, 15 years ago

Attachment: save2.txt added

Updated patch

comment:33 by fingolfin, 15 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, 15 years ago

Resolution: fixed
Status: newpending

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

Status: pendingclosed

comment:36 by SF/sf-robot, 15 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.