Opened 3 years ago
Closed 3 years ago
#12950 closed defect (fixed)
TITANIC: Asking "what should I do?" will cause a crash
Reported by: | tylerszabo | Owned by: | dreammaster |
---|---|---|---|
Priority: | normal | Component: | Engine: Titanic |
Version: | Keywords: | ||
Cc: | Game: | Starship Titanic |
Description
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.
Attachments (1)
Change History (4)
by , 3 years ago
Attachment: | titanic-win.002 added |
---|
comment:1 by , 3 years ago
Summary: | Asking "what should I do?" will cause a crash → TITANIC: Asking "what should I do?" will cause a crash |
---|
comment:2 by , 3 years ago
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?
comment:3 by , 3 years ago
Owner: | set to |
---|---|
Resolution: | → fixed |
Status: | new → closed |
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.
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 intofindByWordClass()
and therethis
is treated as non-null which leads to segmentation fault.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:
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?