Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#6074 closed defect (fixed)

SCI: SQ4 CD - Crash in sewer when text and speech enabled

Reported by: SF/a60wattfish Owned by: bluegr
Priority: normal Component: Engine: SCI
Keywords: script Cc:
Game: Space Quest 4

Description

On the current nightly build (Mac OS X (Intel)), the game crashes when the message starts playing in the sewer (right at the start). I've tested in 1.4.1 and the game works ok at this point. After some testing, it appears as though this only occurs when you have both text and speech enabled.

To replicate do the following: Enable text and speech, start the game, skip intro, move one screen left, one screen up, hand on sewer grate and then hand on the mat on the desk. A hologram will appear and will start talking, when the text is displayed the game will then crash.

The following error is logged in the console:
01/07/2012 23:27:43.045 [0x0-0x39039].org.scummvm.scummvm: Uninitialized read for temp 1000 from method theProfessor::startText (room 70, script 928, localCall ffffffff)!

Ticket imported from: #3539350. Ticket imported from: bugs/6074.

Attachments (1)

sq4-workaround-for-bug-3539350.patch (1.5 KB) - added by digitall 7 years ago.
SQ4 Workaround Patch

Download all attachments as: .zip

Change History (9)

comment:1 Changed 7 years ago by SF/a60wattfish

Summary: SCI: SQ4 CD - Crash in sewer when text and speach enabledSCI: SQ4 CD - Crash in sewer when text and speech enabled

comment:2 Changed 7 years ago by digitall

Replicated on Linux x86_32 with latest Git master. As the submitter indicated, it only occurs when Both is chosen (Warning, this is the value in the central box, not the value on the button name in the game dialog).

One correction: To replicate you, from the start, you need to go move one screen up and one screen right to find the sewer grate. Not sure if there are multiple grates, but I think this is a actually a minor error for the most common walkthrough for this, and Javacat cribbed the replication sequence from there.

I will look at adding a script workaround for this bug and attach it as a patch, but I suspect this will need a SCI engine dev to look closely at the scripts as this could be a nasty bug which corrupts game state. I did try changing the Text/Speech/Both mode from the game dialog during the Professor's speech, which also caused the following addition errors to occur:
WARNING: SegManager: seg 2a is removed from memory, but not removed from hash_map!
SegManager::deallocate(): invalid segment ID!
Segmentation fault

comment:3 Changed 7 years ago by SF/a60wattfish

Oops.. I meant right, not left! I wrote it from memory and was slightly distracted :P

Thanks for that correction and taking a look!

comment:4 Changed 7 years ago by digitall

Attaching a script workaround patch, which fixes this bug, however, it induces a crash on ScummVM exit as previously seen:
WARNING: SegManager: seg 29 is removed from memory, but not removed from hash_map!
SegManager::deallocate(): invalid segment ID!
Segmentation fault

It should be noted that a similar workaround already exists for Narrator::startText and this was used as a basis, but if this case occurs for further "speaking" characters then these should probably be merged into a general case for SQ4 script 928 method "startText" without a restriction by object-name.

Changed 7 years ago by digitall

SQ4 Workaround Patch

comment:5 Changed 7 years ago by digitall

Ah. Have located the segmentation fault/crash using gdb.
The issue here is that the engine calls error("SegManager::deallocate(): invalid segment ID!") which is a seperate engine issue.

The segfault is caused by the error() function code, specifically line 99 of common/textconsole.cpp:
<snip>
// If there is an error handler, invoke it now
if (Common::s_errorHandler)
(*Common::s_errorHandler)(buf_output);
</snip>

This calls an error handler if the static variable Common::s_errorHandler is not NULL i.e. 0, which it defaults to by global assignment.. This can be set by setErrorHandler, but as far as I can see this is not set...
It appears the crash occurs as Common::s_errorHandler is pointing at invalid memory i.e. is thus not 0, but calling it causes a segfault.

comment:6 Changed 7 years ago by bluegr

Keywords: script added
Owner: set to bluegr
Resolution: fixed
Status: newclosed

comment:7 Changed 7 years ago by bluegr

Fixed on master (commit 4493511), by checking for the base talker object class instead of the object itself. Thanks for reporting! The fix should be available in the next daily version.

comment:8 Changed 7 years ago by digitall

Minor update. Have been unable to replicate the segfault. Probably due to memory corruption due to the engine bugs, but if this occurs again, I will try to trace the cause.

Note: See TracTickets for help on using tickets.