Broken Sword 2.5 crash

[RetroJaro]

[2021-02-19 03:33:37] ScummVM 2.3.0git12803-gd855b6db3f (Feb 19 2021 05:32:34)
[2021-02-19 03:33:37] TAINTED Vorbis FLAC MP3 RGB zLib MPEG2 FluidSynth Theora AAC A/52 FreeType2 JPEG PNG cloud (servers, local) TinyGL OpenGL (with shaders) GLEW
[2021-02-19 03:33:37] --- Log opened.
[2021-02-19 03:34:55] WARNING: Sound::findHandle(): Unknown handle!
[2021-02-19 03:34:55] WARNING: Sound::findHandle(): Unknown handle!

I try to enter this building and crash.

[criezy]

Do you have a savegame before entering the building?
If you do, please attach it to this ticket to help with investigating the crash.

[criezy]

I don't know if this is the same crash, but I get a crash right at the beginning of the game when trying to leave Nico's building. And this seems to be a regression because it works with ScummVM 2.2.0.

[criezy]

This is a regression introduced between 6d00724 and 6d00724. I cannot narrow the range more as the game does not even start between these two commits. But the commit before 6d00724 works, and 233adb3 is broken.

The range is quite small and contains some trivial commits. At this point the most likely culprit is 6d00724: SWORD25: Make the extracted version working

[criezy]

Building with address sanitizer and running with debug output gives the following output, which would be consistent with 6d00724 being the culprit:

PackageManager::doSearch(..., "/rooms/Str/ani/*_ani.xml", "", 2)
AddressSanitizer:DEADLYSIGNAL
=================================================================
==33300==ERROR: AddressSanitizer: SEGV on unknown address 0xe8e665e6dafedf6 (pc 0x00010137c8a8 bp 0x00010137c870 sp 0x00016f4662e0 T0)
==33300==The signal is caused by a UNKNOWN memory access.
    #0 0x10137c8a8 in Common::FSNode::getPath() const fs.cpp:113

==33300==Register values:
 x[0] = 0x0000000127455660   x[1] = 0x000000016f4664f0   x[2] = 0x000000016f4657c8   x[3] = 0x0000000000000348  
 x[4] = 0x0000000000000028   x[5] = 0x0000000000000002   x[6] = 0x000000016ec74000   x[7] = 0x0000000000000001  
 x[8] = 0x0000000127455660   x[9] = 0x74732f736d6f6f72  x[10] = 0x74732f736d6f6fb2  x[11] = 0x0000007000020000  
x[12] = 0x0e8e665e6dafedf6  x[13] = 0x0000000000000000  x[14] = 0x0000000000000000  x[15] = 0xf3f3f300f1f1f1f1  
x[16] = 0x000000010137bb04  x[17] = 0x0000000000000000  x[18] = 0x000000012dc38140  x[19] = 0x000000016f466720  
x[20] = 0x000000016f466340  x[21] = 0x000000010896dd40  x[22] = 0x0000000101820442  x[23] = 0x0000000100b57d74  
x[24] = 0x00000000000000f8  x[25] = 0x000000016f466af0  x[26] = 0xf2f2f2f2f2f8f8f8  x[27] = 0xf2f2f2f8f8f8f8f8  
x[28] = 0xf2f8f2f2f2f8f2f2  fp = 0x000000016f466330  lr = 0x000000010137c870  sp = 0x000000016f4662e0  
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV fs.cpp:113 in Common::FSNode::getPath() const
==33300==ABORTING
zsh: abort      ./scummvm -d9

[criezy]

I have identified the issue.

When PackageManager::loadDirectoryAsPackage() gets called it sets the _extractedFiles flag to true, which indicates that we are playing the game with extracted files.

Then in PackageManager::doSearch when it iterates on items in the ArchiveMemberList, if we are playing with extracted files there is a block where it assumes the ArchiveMember are FSNode.

The issue here is that when we are playing with the non-extracted game, the PackageManager::loadDirectoryAsPackage() function still gets called when we load or save a game with the savegame path. And as a result after we have saved or loaded a savegame, it thinks we are playing with extracted files. And then it crashes when trying to access ArchiveMember as FSNode, because they are not actually a FSNode.

[criezy]

Thank you for your report.
The crash should be fixed with e0d3b05730.

I am assuming the crash you saw is the same one I saw. If you the game still crashes for you please reopen this ticket and if possible attach a save game.