Opened 12 years ago

Closed 12 years ago

#3894 closed defect (fixed)

GOB1: Crash in level 15

Reported by: SF/marticus Owned by: DrMcCoy
Priority: normal Component: Engine: Gob
Keywords: Cc:
Game: Gobliiins

Description

Win XP 32 SP2; Level 15 TCNGTOV Daily snapshot 22/8 and in 0.11.1

Gobliiins CD (v1/DOS/English(US)) Crashes to desktop (no error even on debug level 5) as soon as I click pick up the mallet right near the end of the level.

Tried it twice in both versions and it crashed both times.

Also, it might be relevant that I usually try to skip the intro cutscene by hitting esc and both mouse buttons a few times.

Ticket imported from: #2069177. Ticket imported from: bugs/3894.

Change History (6)

comment:1 by eriktorbjorn, 12 years ago

I managed to reproduce this once in the 0.12 snapshot. I don't know what's causing it, but maybe someone else will be able to interpret these Valgrind warnings:

==30030== ==30030== Invalid read of size 4 ==30030== at 0x82CD3E8: Gob::Goblin::targetItem() (goblin.cpp:821) ==30030== by 0x82D0251: Gob::Goblin::moveInitStep(short, short, short, Gob::Goblin::Gob_Object*, short*, short*) (goblin.cpp:1046) ==30030== by 0x82D0534: Gob::Goblin::doMove(Gob::Goblin::Gob_Object*, short, short) (goblin.cpp:1153) ==30030== by 0x82709C3: Gob::Inter_v1::o1_moveGoblin(Gob::Inter::OpGobParams&) (inter_v1.cpp:2818) ==30030== by 0x82757CF: Gob::Inter_v1::executeGoblinOpcode(int, Gob::Inter::OpGobParams&) (inter_v1.cpp:700) ==30030== by 0x8273412: Gob::Inter_v1::o1_goblinFunc(Gob::Inter::OpFuncParams&) (inter_v1.cpp:1828) ==30030== by 0x8275910: Gob::Inter_v1::executeFuncOpcode(unsigned char, unsigned char, Gob::Inter::OpFuncParams&) (inter_v1.cpp:678) ==30030== by 0x826B772: Gob::Inter::funcBlock(short) (inter.cpp:259) ==30030== by 0x82751F9: Gob::Inter_v1::o1_if(Gob::Inter::OpFuncParams&) (inter_v1.cpp:1298) ==30030== by 0x8275910: Gob::Inter_v1::executeFuncOpcode(unsigned char, unsigned char, Gob::Inter::OpFuncParams&) (inter_v1.cpp:678) ==30030== by 0x826B772: Gob::Inter::funcBlock(short) (inter.cpp:259) ==30030== by 0x826B82A: Gob::Inter::callSub(short) (inter.cpp:287) ==30030== Address 0x64bc618 is 0 bytes after a block of size 112 alloc'd ==30030== at 0x402309E: operator new[](unsigned) (vg_replace_malloc.c:268) ==30030== by 0x82850F7: Gob::Map_v1::init() (map_v1.cpp:53) ==30030== by 0x82854D8: Gob::Map_v1::loadMapObjects(char const*) (map_v1.cpp:89) ==30030== by 0x82D18DE: Gob::Goblin::loadObjects(char const*) (goblin.cpp:1187) ==30030== by 0x8270BCC: Gob::Inter_v1::o1_loadObjects(Gob::Inter::OpGobParams&) (inter_v1.cpp:2786) ==30030== by 0x82757CF: Gob::Inter_v1::executeGoblinOpcode(int, Gob::Inter::OpGobParams&) (inter_v1.cpp:700) ==30030== by 0x8273412: Gob::Inter_v1::o1_goblinFunc(Gob::Inter::OpFuncParams&) (inter_v1.cpp:1828) ==30030== by 0x8275910: Gob::Inter_v1::executeFuncOpcode(unsigned char, unsigned char, Gob::Inter::OpFuncParams&) (inter_v1.cpp:678) ==30030== by 0x826B772: Gob::Inter::funcBlock(short) (inter.cpp:259) ==30030== by 0x826B82A: Gob::Inter::callSub(short) (inter.cpp:287) ==30030== by 0x8275EF1: Gob::Inter_v1::o1_callSub(Gob::Inter::OpFuncParams&) (inter_v1.cpp:1154) ==30030== by 0x8275910: Gob::Inter_v1::executeFuncOpcode(unsigned char, unsigned char, Gob::Inter::OpFuncParams&) (inter_v1.cpp:678) ==30030== ==30030== Invalid read of size 2 ==30030== at 0x82CD3FA: Gob::Goblin::targetItem() (goblin.cpp:821) ==30030== by 0x82D0251: Gob::Goblin::moveInitStep(short, short, short, Gob::Goblin::Gob_Object*, short*, short*) (goblin.cpp:1046) ==30030== by 0x82D0534: Gob::Goblin::doMove(Gob::Goblin::Gob_Object*, short, short) (goblin.cpp:1153) ==30030== by 0x82709C3: Gob::Inter_v1::o1_moveGoblin(Gob::Inter::OpGobParams&) (inter_v1.cpp:2818) ==30030== by 0x82757CF: Gob::Inter_v1::executeGoblinOpcode(int, Gob::Inter::OpGobParams&) (inter_v1.cpp:700) ==30030== by 0x8273412: Gob::Inter_v1::o1_goblinFunc(Gob::Inter::OpFuncParams&) (inter_v1.cpp:1828) ==30030== by 0x8275910: Gob::Inter_v1::executeFuncOpcode(unsigned char, unsigned char, Gob::Inter::OpFuncParams&) (inter_v1.cpp:678) ==30030== by 0x826B772: Gob::Inter::funcBlock(short) (inter.cpp:259) ==30030== by 0x82751F9: Gob::Inter_v1::o1_if(Gob::Inter::OpFuncParams&) (inter_v1.cpp:1298) ==30030== by 0x8275910: Gob::Inter_v1::executeFuncOpcode(unsigned char, unsigned char, Gob::Inter::OpFuncParams&) (inter_v1.cpp:678) ==30030== by 0x826B772: Gob::Inter::funcBlock(short) (inter.cpp:259) ==30030== by 0x826B82A: Gob::Inter::callSub(short) (inter.cpp:287) ==30030== Address 0x10 is not stack'd, malloc'd or (recently) free'd

comment:2 by eriktorbjorn, 12 years ago

I managed to get it to crash again, though this time I had to pick up the mallet and drop it again near the bottom of the screen. When I tried to pick it up again, ScummVM tried to access _itemsMap[28][7], which is out of bounds. (The "height" of the items map is 28, starting at index 0, so 28 is the 29th element.)

So it's probably possible to reproduce it at any point where an object appears, or is placed, too close to the bottom of the screen.

Actually, there are a couple of times where we access _itemsMap something like _itemsMap[y + yoff][x + xoff], which means that even if both x and y are within bounds, the access could be out of bounds.

Maybe we should access it through a function instead, so we can do bounds checking there? If so, are there other such structures we should fix, too?

comment:3 by eriktorbjorn, 12 years ago

Owner: set to DrMcCoy

comment:4 by fingolfin, 12 years ago

Summary: GOB1: CTD in level 15GOB1: Crash in level 15

comment:5 by DrMcCoy, 12 years ago

Resolution: fixed
Status: newclosed

comment:6 by DrMcCoy, 12 years ago

Should be fixed in the SVN repository / next daily build.

> Maybe we should access it through a function instead

Did just that, thanks. :)

Note: See TracTickets for help on using tickets.