TITANIC: Asking "what should I do?" will cause a crash

[tylerszabo]

General Information

• ScummVM Version: 2.2.0 (Sep 14 2020 23:43:32)
• Bug details: Load the attached saved game titanic-win.002 then click on DoorBot's icon to summon. Once greeted type "what should I do?" and await a crash.
• Language of game: English
• Version of game: GOG installed via GOG Galaxy
• Operating System: Windows 10 Pro (19041.1.amd64fre.vb_release.191206-1406)
• Saved game: (attached)

Bug Details

Summoning DoorBot and asking "what should I do?" will cause a crash.

Additional Notes

In addition to DoorBot asking BellBot or DeskBot "what should I do?" will also cause a crash. The punctuation doesn't seem to matter.

At the start of the game (before the credits/intro) the input will just be ignored.

Expected Behavior

DoorBot responds with one of a variety of generic unhelpful responses.

Saved Game and Steps to Reproduce

Load the attached saved game titanic-win.002 then click on DoorBot's icon to summon. Once greeted type "what should I do?" and await a crash.

[antoniou79]

I think I can reproduce this on the latest code (master branch) on Windows 10, msys2 build.
Just writing "should" suffices to trigger the crash (segmentation fault).

Seems to occur because in this part of code (TTparser::considerRequests()), in this case, _conceptP is nullptr but there's no check for it. For some reason the execution seems to go into findByWordClass() and there this is treated as non-null which leads to segmentation fault.

TTconcept *conceptP = _conceptP->findByWordClass(WC_ACTION);

​https://github.com/scummvm/scummvm/blob/dc1717067322bade8c43536679ece9a9b9a87b49/engines/titanic/true_talk/tt_parser.cpp#L1000

Oddly, while debugging with Visual Studio, the execution goes into findByWordClass() but this is treated as null and the method returns nullptr.

We could fix this by doing something like:

TTconcept *conceptP = (_conceptP != nullptr) ? _conceptP->findByWordClass(WC_ACTION) : nullptr;

However, I can see multiple other instances in the same class, where we use _conceptP methods and members unchecked. Not sure if we should fix all the other cases too, or fix the reason why _conceptP is nullptr at that part of the code -- maybe the code wrongly assumes that it should have been initialized earlier or maybe it should have been initialized and it's not?

[tylerszabo]

I just got local builds working and even Release flavor from MSVC seems to not exhibit the issue. It does look like a nuance of treatment of undefined behavior between compilers. Perhaps, since it's occurring in multiple places that TTconcept * should be replaced with something like PTTconcept that will perform checks automatically and perhaps add non-fatal logging?

[dreammaster]

For now, I've changed the method to be static and pass in the TTconcept as a parameter. This is semantically the same as the method in the original, so this should work around any undefined behaviour issues causing by it previously being an instance method.