Opened 11 years ago

Closed 2 weeks ago

Last modified 2 weeks ago

#6356 closed defect (fixed)

AGOS: Simon1 Crash in Dwarf Cave

Reported by: SF/pbholmen Owned by: bluegr
Priority: high Component: Engine: AGOS
Version: Keywords:
Cc: Game: Simon the Sorcerer 1

Description

When loading a saved game in Simon the Sorcerer 1, where Simon is in the basement of the dwarf cave with the beer kegs and one sleeping dwarf, the game is successfully loaded, but as soon as Simon moves around, the game crashes. There is no error message, the game unexpectedly quits. The crash log says...

Exception Type: EXC_CRASH (SIGBUS).

ScummVM version: ScummVM 1.6.0, June 1 2013. The error is reproduced with the official nightly build June 19 (1.7.0git559-g125b146). Language of the game: English Version of game: The error occurs with both the floppy disk version for DOS and the CD (talkie) version for Windows. The saved game has been played and saved with the Floppy disk version. Downloaded from gog.com. System/platform: Mac OS X 10.8.4 - iMac 27" late 2012.

Steps required to reproduce the problem: 1) Start ScummVM 2) Start Simon the Sorcerer 3) Press esc through all cutscenes. 4) Use postcard 5) Load saved game 6) Move around in cave

Saved game is included. I can provide a detailed crash log if needed. I thought I'd omit it in case there can be security threats by sharing it publicly. I don't know anything about that stuff. If you ask, I'll provide crash log. If the included save game does not start inside the cave and the key is in the inventory, I might have misinterpreted the filenames of the saved games. Tell me, and I'll include the right file.

Ticket imported from: #3614549. Ticket imported from: bugs/6356.

Attachments (2)

simon1.004 (889 bytes ) - added by SF/pbholmen 11 years ago.
Saved game
Crashlog_scummvm_2020-07-17_13-42-23.txt (41.0 KB ) - added by raziel- 4 years ago.
Crashlog/stacktrace

Download all attachments as: .zip

Change History (10)

by SF/pbholmen, 11 years ago

Attachment: simon1.004 added

Saved game

comment:1 by digitall, 11 years ago

Owner: set to Kirben

comment:2 by digitall, 11 years ago

Assigned to AGOS maintainer for visibility.

Replicated with the latest Git master on Linux x86_64 using Simon1/DOS/English and the attached savegame (which needed to be renamed to simon1.001 in order to appear in the load listing).

Have replicated under valgrind. The cause of the segfault is due to: ==15424== Invalid read of size 2 ==15424== at 0x41FC48: READ_BE_UINT16(void const*) (endian.h:169) ==15424== by 0x41BE0F: AGOS::AGOSEngine::readUint16Wrapper(void const*) (res. cpp:106) ==15424== by 0x44A019: AGOS::AGOSEngine::vc48_setPathFinder() (vga_s1.cpp:201 ) ==15424== by 0x4427A9: AGOS::AGOSEngine::runVgaScript() (vga.cpp:175) ==15424== by 0x462F8F: AGOS::AGOSEngine::animateEvent(unsigned char const*, u nsigned short, unsigned short) (event.cpp:290) ==15424== by 0x462DC7: AGOS::AGOSEngine::processVgaEvents() (event.cpp:248) ==15424== by 0x4640F2: AGOS::AGOSEngine::timerProc() (event.cpp:657) ==15424== by 0x46360F: AGOS::AGOSEngine::delay(unsigned int) (event.cpp:451) ==15424== by 0x46DEE9: AGOS::AGOSEngine::waitForInput() (input.cpp:209) ==15424== by 0x4565D0: AGOS::AGOSEngine::go() (agos.cpp:1062) ==15424== by 0x41BA84: AGOS::AGOSEngine::run() (agos.h:223) ==15424== by 0x409440: runGame(PluginSubclass<MetaEngine> const*, OSystem&, C ommon::String const&) (main.cpp:226) ==15424== Address 0xe is not stack'd, malloc'd or (recently) free'd

comment:3 by digitall, 11 years ago

Summary: Simon the Sorcerer 1 - crash in dwarf cave Mac OS XAGOS: Simon1 Crash in Dwarf Cave

comment:4 by raziel-, 4 years ago

ScummVM 2.2.0git (Jul 15 2020 10:24:49)
Features compiled in: Vorbis FLAC MP3 RGB zLib MPEG2 Theora AAC A/52 FreeType2 FriBiDi JPEG PNG cloud (servers, local)

There is still a crash with above ScummVM version and below Simon version.

I need to add that i used the savegame from this report (it's the floppy version save, so not sure if that makes a difference?).

Crashlog added.

Simon the Sorcerer 1 (CD/Windows/English)

AmigaOS4 - PPC - BE - SDL

by raziel-, 4 years ago

Crashlog/stacktrace

comment:5 by somaen, 2 weeks ago

Priority: normalhigh

Would be good to resolve this for the 2.9.0 release.

comment:6 by bluegr, 2 weeks ago

The crash occurs here, when loading:

void AGOSEngine::vc48_setPathFinder() {

uint16 a = (uint16)_variableArrayPtr[12];
const uint16 *p = _pathFindArray[a - 1];

In this case, a is 6, but _pathFindArray only has 5 elements, so it goes out of bounds, reads null and crashes

I can fix the issue with this workaround, but is it a good way of fixing this issue?

diff --git a/engines/agos/vga_s1.cpp b/engines/agos/vga_s1.cpp
index 9e4ea8e6b89..13a5e3d1765 100644
--- a/engines/agos/vga_s1.cpp
+++ b/engines/agos/vga_s1.cpp
@@ -179,6 +179,11 @@ void AGOSEngine::vc47_addToVar() {

void AGOSEngine::vc48_setPathFinder() {

uint16 a = (uint16)_variableArrayPtr[12];

+ if (!_pathFindArray[a - 1]) {
+ warning("Invalid path, attempting to correct");
+ a = 1;
+ }
+

const uint16 *p = _pathFindArray[a - 1];


uint b = (uint16)_variableArray[13];

i.e. it picks the first element in the list, if the pointer is out of bounds

comment:7 by Filippos Karapetis <bluegr@…>, 2 weeks ago

Resolution: fixed
Status: newclosed

In 0905e16a:

AGOS: Add a workaround when an invalid path is selected - fixes bug 6356

comment:8 by bluegr, 2 weeks ago

Owner: changed from Kirben to bluegr

I've talked with aquadran and he agreed that a workaround can be used in such a case. Fixed in commit 0905e16a9bd015e2278932ff4e837d6363569655

Note: See TracTickets for help on using tickets.