STARK: The Steam Version of The Longest Journey crashes

[toby63]

scummvm -v:

ScummVM 2.3.0git (Jul 24 2021 21:36:03)
Features compiled in: Vorbis FLAC MP3 ALSA SEQ TiMidity RGB zLib MPEG2 FluidSynth Theora AAC A/52 FreeType2 FriBiDi JPEG PNG GIF cloud (servers, local) TinyGL OpenGL (with shaders) GLEW

commit: f0fd086948

OS: Arch Linux
Graphics Driver: Nvidia official

Game:
Language: English
Version: Steam (recognized as "DVD")

I remember that it used to work with residualvm.

Step-by-Step Guide:

• Add the Game (it is recognized as "The longest Journey (DVD/Windows/English)" )
• Click start

I have tried multiple different options, but nothing changed.

The log shows:

scummvm: common/stream.cpp:51: Common::SeekableReadStream* Common::ReadStream::readStream(uint32): Assertion `dataSize > 0' failed.
Aborted (core dumped)

Compiled with:

 ./configure \
    --enable-c++11 \
    --enable-release \
    --enable-engine=stark,sword25 \
    --prefix=/usr

[criezy]

Crash confirmed on macOS as well.
It used to be working in ScummVM, so this is a regression.

Address sanitizer reports a stack buffer overflow in the SDL backend:

READ of size 9 at 0x00016d776d90 thread T0
  #1 0x102ce623c in SdlGraphicsManager::setState(SdlGraphicsManager::State const&) sdl-graphics.cpp:89
  #2 0x102747390 in OSystem_SDL::setGraphicsMode(int, unsigned int) sdl.cpp:790
  #3 0x1029df75c in initGraphics3d(int, int) engine.cpp:399
  #4 0x1027a7040 in Stark::Gfx::Driver::create() driver.cpp:48
  #5 0x10292b81c in Stark::StarkEngine::run() stark.cpp:98

Address 0x00016d776d90 is located in stack of thread T0 at offset 80 in frame
  #0 0x102ce5d84 in SdlGraphicsManager::setState(SdlGraphicsManager::State const&) sdl-graphics.cpp:79

[criezy]

Regression is from

d33487f641bcd6e9e4d31a0bf91f7abf2f92c2e0 is the first bad commit
commit d33487f641bcd6e9e4d31a0bf91f7abf2f92c2e0
Author: Cameron Cawley <ccawley2011@gmail.com>
Date:   Fri Apr 9 23:32:45 2021 +0100

    SDL: Refactor OpenGLSdlGraphics3dManager to inherit from SdlGraphicsManager

[criezy]

Thank you for your report.
The issue has now been fixed (in 23ca3604b80d375aa31e4bbd410c8749978570ef).

The issue was not specific to The Longest Journey and other 3D games were probably impacted.

[toby63]

Sadly I cannot confirm that it is fixed.

I still get the same error with the newest commit ​9720b96f383b76dace83baecadfefb630ee4877f.

[criezy]

Then this is a different issue.
I have the version from GOG (recognised as GOG/Windows/English) and it now starts properly. Maybe the issue you are having is specific to the Steam version. I don't have that version though, so I cannot verify. Hopefully a developer who has that version will be able to check.

It would also help if you are able to provide a back trace to understand what is causing that assertion failure.

[toby63]

@criezy: Ok thanks for the help so far and for fixing the other bug.

I will try to debug it and report back.

[Substr]

I can confirm the same issue running the most recent nightly build using the Steam version of The Longest Journey. I'm running this on Windows 10, is there any more information I can provide to assist?

[antoniou79]

I think I've found the issue for this specific crash, but I cannot be sure my fix for this would prevent other similar crashes.

Basically, the game.exe files for GOG and Steam are different and have very different sizes. The GOG one is 424KB and the Steam one is 94KB.

The stark engine reads "game.exe" to get resources, seemingly only to provide "styled confirmation dialogs" as per:
​https://github.com/scummvm/scummvm/blob/fed702c0593323c1509745223b9383666ce53241/engines/stark/stark.cpp#L315

This fails for the steam version. In particular, it fails in the loadBackground() method here:

Common::SeekableReadStream *stream = executable.getResource(Common::kWinBitmap, 147);

​https://github.com/scummvm/scummvm/blob/fed702c0593323c1509745223b9383666ce53241/engines/stark/ui/dialogbox.cpp#L226

The code has fallback for the case that the "stream" returned by this getResource() attempt is null ("If we were not able to load the background, fallback to dark blue")
​https://github.com/scummvm/scummvm/blob/fed702c0593323c1509745223b9383666ce53241/engines/stark/ui/dialogbox.cpp#L58

But the SeekableReadStream *PEResources::getResource() method, calls the SeekableReadStream *ReadStream::readStream() method on the exe file stream and this last method produces the assertion fault.

I've pushed a fix for this, that checks the version info of the executable (game.exe) and if it matches the Steam version (1.0.0.161) then it skips trying to load the background image resource. There probably is a better way to do this, since if the build version of the Steam release changes, then the issue could come back.
​https://github.com/scummvm/scummvm/commit/565a0559ed09660daa83cd529afa4e0efb6949c5

[toby63]

@antoniou79 Thx for looking into this.

Due to another problem I was not able to provide any debug info yet.

I will try your fix.
Interesting that the game versions differ in these specific things.

Regarding the detection method:
What about letting the user set it to a specific Steam version in the Menu?
Or scummvm detecting it, based on the Steamfolder?

[antoniou79]

Replying to toby63:

Regarding the detection method:
What about letting the user set it to a specific Steam version in the Menu?
Or scummvm detecting it, based on the Steamfolder?

The first suggestion would still mean that the user would be likely to get the crash until they set it themselves as "Steam version" (somehow, which I am also not sure we support currently).
The second (which I also don't think we support currently, ie. detection that takes the folder name into consideration) would be an automated solution, but it would assume that future Steam builds for the .exe (how likely are those anyway?) won't fix the missing resources issue; but perhaps they will...

We could check for the md5 hash of the game.exe file (which would be similar to checking the version info, but... safer I guess?) but we can't guarantee that the game folder will always have the game.exe file, and the engine does run without it, so I am not sure how this would complicate the game detection code (currently stark engine's detection does not consider the game.exe file at all).

[antoniou79]

An alternative fix is submitted as a PR:
​https://github.com/scummvm/scummvm/pull/3318

This one uses the MD5 hash to detect the steam version with the particular exe file that is missing the background image resource.

[toby63]

Replying to antoniou79:

An alternative fix is submitted as a PR:
​https://github.com/scummvm/scummvm/pull/3318

This one uses the MD5 hash to detect the steam version with the particular exe file that is missing the background image resource.

Just tried your PR and it is working :).

Thx.

I guess you will close this once the PR is merged? In that case I will leave this open.

[antoniou79]

Thank you for the feedback. The PR is now merged, so I'm closing the ticket.