Opened 5 months ago

Closed 9 days ago

Last modified 9 days ago

#15162 closed defect (fixed)

TITANIC: Using the word "that" when talking to a character causes a segfault

Reported by: benblank Owned by: dreammaster
Priority: blocker Component: Engine: Titanic
Version: Keywords: crash, wc_pronoun, word, parser, this, those, that
Cc: benblank Game: Starship Titanic

Description (last modified by benblank)

I'm playing Starship Titanic using the game data from GOG[1] and ScummVM 2.8.0 from Ubuntu 24.04[2].

When talking to a character (any character, I think, as long as you've gotten past the opening credits), using the complete word "that" in conversation causes a segmentation fault, closing ScummVM.

Certain specific variations also cause a segfault (for example, "that's" and "thats"), but not other words containing those characters (for example, "sthat" and "t'haiti" are both fine). Using the word in a sentence triggers the segfault, as well (e.g. "what does that mean").

I first encountered the issue partway through a playthough, but it also happens if you say "that" to the nearby DeskBot immediately after the opening credits (I've attached a game save made standing in front of her, having done just that). Saying "that" outside of of a conversation is fine, even if standing in front of a character you can normally converse with; in that situation, you have to initiate a conversation by clicking on them first. Saying "that" to the DoorBot during the opening sequence (before the opening credits) is also fine.

To reproduce, you should be able to simply launch the game, load the save, click on the DeskBot, type the word "what", and press Enter.

I've attached:

game.sha256sum
SHA-256 hashes of the game files from GOG which I'm using.

sha256sum --binary game/** > game.sha256sum

lotta-debug.log
A log of the game's output with as much logging as I could find enabled. Sadly, I didn't see anything which looked like a smoking gun.

scummvm --debuglevel=32767 --debugflags=eventrec,detection,core,scripts,graphics,starfield --config=scummvm.ini titanic-win > lotta-debug.log 2>&1

scummvm.ini
The configuration file I used. (This is the same configuration file included with the GOG installer, but with the game and saves paths changed.)
saves/timestamps
From the game saves directory; I suspect this isn't relevant information, but have included it in case I'm wrong about that.
saves/titanic-win.000
A game save created by starting the game, proceeding through the "on rails" pre-credits sequence, watching the opening credits, then walking over and standing in front of the DeskBot. All of the behaviors I described above were tested using this save.
/var/crash/_usr_games_scummvm.1000.crash
A core dump created by loading the above save game, clicking on the DeskBot, typing the word "that", and pressing Enter.

1 The Linux installer from GOG doesn't produce a runnable game, but the data files seem fine; I've attached an sha256sum file for them.

2 From the official Ubuntu repository. Unfortunately, I wasn't able to get the daily build "debian-x86-64-master-5ed3dd8d" working, as several of the required shared libraries are of versions which aren't readily available in Ubuntu 24.04. I tried cobbling together a set of libraries of the correct versions, but wasn't ultimately successful in getting ScummVM to run.

Attachments (5)

game.sha256sum (39.4 KB ) - added by benblank 5 months ago.
scummvm.ini (584 bytes ) - added by benblank 5 months ago.
timestamps (27 bytes ) - added by benblank 5 months ago.
titanic-win.000 (106.7 KB ) - added by benblank 5 months ago.
lotta-debug.log.gz (1013.2 KB ) - added by benblank 5 months ago.

Download all attachments as: .zip

Change History (13)

by benblank, 5 months ago

Attachment: game.sha256sum added

by benblank, 5 months ago

Attachment: scummvm.ini added

by benblank, 5 months ago

Attachment: timestamps added

by benblank, 5 months ago

Attachment: titanic-win.000 added

by benblank, 5 months ago

Attachment: lotta-debug.log.gz added

comment:1 by benblank, 5 months ago

Actually, the core dump is (unsurprisingly, in retrospect) too large to post. As was the log file, before gzipping it; perhaps I enabled too much debugging. 😅

comment:2 by benblank, 5 months ago

Description: modified (diff)

comment:3 by antoniou79, 5 months ago

This does not happen only with the word "that".
Other words that may cause this are "this" and "those" (but not "these").
Also some words (like "this") seems to be treated in a special way with some bots so with those, the crash won't occur.

I can see where the seg fault occurs (MSVC did not help there because, at least with my config, it does not crash, even though it does try to access a property or method of a non existing object).

The crash seems to happen here: https://github.com/scummvm/scummvm/blob/718353f51218c0a66b820e0438ef15d6f702d919/engines/titanic/true_talk/tt_parser.cpp#L1210

because for this case the _conceptP is a nullptr.

But the cause for it should be rooted earlier in the execution, probably related to the process assigning a category to the word.

That's to say, checking for nullptr at the crash point, fixes this specific crash, but I don't think it's the right solution.

comment:4 by antoniou79, 5 months ago

Component: --Unset--Engine: Titanic
Keywords: crash wc_pronoun word parser this those that added

comment:5 by tag2015, 5 months ago

Summary: Using the word "that" when talking to a character causes a segfaultTITANIC: Using the word "that" when talking to a character causes a segfault

comment:6 by somaen, 12 days ago

Priority: normalblocker

I would prefer if we resolved this before the 2.9.0 release.

comment:7 by dreammaster, 9 days ago

Owner: set to dreammaster
Resolution: fixed
Status: newclosed

I tried looking into the problem months ago, and I swear that I couldn't replicate the problem at the time. Maybe it's true what told me earlier, that MSVC is silently allowing dereferencing null pointers without throwing a wobbly. Anyway, when I added some explict asserts, I was finally able to replicate the problem. Then tracing through the TTparser code execution, I finally tracked down the issue properly. At the start of TTparser::checkForAction, it's meant to go into the block if !_conceptP, so one can be set up. But previously, it was only going into the block if one *was* set up, leading to the situation where a dummy _conceptP was never being set. I'm just committing a fix now.

comment:8 by dreammaster, 9 days ago

In 4c30db39:

TITANIC: Fix crash when using 'that' as a sentence

Fixes bug #15162

Note: See TracTickets for help on using tickets.