MOHAWK: Riven: Cannot create a Switch command from data

[marcvinyals]

When trying to climb up the rope in the gallows in the lake in Jungle Island the game crashes with the following error:

"ERROR: Cannot create a Switch command from data"

To reproduce from the attached saved game: click on the string to pull it, wait for a rope to lower, then click on the dangling rope. The game should crash immediately.

ScummVM version is 2.6.0git5013-g7631d04f29d. Using git bisect I was able to trace this back to the point where ResidualVM got merged into ScummVM, in other words commit ae268bf990a2615983311bb594cb09bf17a01850 works fine but commit 5f34c5d7797c1ddd066a5e7e9adb7993f8544d97 crashes. However I do not know how to continue bisecting from here, since the ResidualVM branch does not contain the riven engine.

The game version is 25th Anniversary/Windows/English from GOG. I am running on Gentoo, configured with ./configure --disable-all-engines --enable-engine=riven --disable-fluidsynth, in case it makes a difference.

[eriktorbjorn]

The savegame is compatible with my old DVD version (not the CD version, though), but I can't reproduce the bug with it. At least not with the current development version of ScummVM.

[antoniou79]

I am unable to reproduce the issue with the GOG version (tested by installing via the offline installer and again with GOG Galaxy).

Maybe it's a specific game option or scummvm configuration option that interferes?

Did you try removing and re-adding the game?

[marcvinyals]

Removing and re-adding the game did not make a difference, but I tried a daily build on a Windows VM and I could not reproduce the bug either. I will try different build and configuration settings on my end to see if I can isolate the problem.

[eriktorbjorn]

Just for the record, I'm running ScummVM in Linux too, though I'm using Debian.

In the past I've heard of Gentoo using more aggressive optimizations when compiling than Linux distributions in general do. If so, is that only for their own packages, or is it for any compiling? (I'm wondering since optimizations can sometimes trigger bugs that are dormant in debug builds.)

[marcvinyals]

You were spot on in suggesting that optimizations may have something to do with the bug. If I compile with CPPFLAGS="-O1" everything works flawlessly, but CPPFLAGS="-O2" causes the bug to appear. My GCC version is 11.2, by the way.

[eriktorbjorn]

Unfortunately, I wasn't able to reproduce it with -O2 either. I'm using Debian's GCC 11.2 too, but of course it's possible we're compiling with different flags. (My computer is a bit old.)

I found this suggestion for finding out what flags GCC uses by default, and this is what it prints for me:

$ echo | gcc -v -E - 2>&1 | grep cc1
- -mtune=generic -march=x86-64 -fasynchronous-unwind-tables -dumpbase -

[marcvinyals]

I was able to find (what I believe to be) the source of the problem using valgrind, this is what I get:

==639379== Conditional jump or move depends on uninitialised value(s)
==639379==    at 0x40336E: Mohawk::RivenScriptManager::createScriptFromData(unsigned int, ...) (riven_scripts.cpp:162)
==639379==    by 0x412E15: Mohawk::RivenStacks::JSpit::xvga1300_carriage(Common::Array<unsigned short> const&) (jspit.cpp:309)
==639379==    by 0x4143D4: Common::Functor1Mem<Common::Array<unsigned short> const&, void, Mohawk::RivenStacks::JSpit>::operator()(Common::Array<unsigned short> const&) const (func.h:460)
==639379==    by 0x408822: Mohawk::RivenStack::runCommand(unsigned short, Common::Array<unsigned short> const&) (riven_stack.cpp:186)
==639379==    by 0x402367: Mohawk::RivenSimpleCommand::runExternalCommand(unsigned short, Common::Array<unsigned short> const&) (riven_scripts.cpp:616)
==639379==    by 0x4016DD: Mohawk::RivenSimpleCommand::execute() (riven_scripts.cpp:826)
==639379==    by 0x400D77: Mohawk::RivenScript::run(Mohawk::RivenScriptManager*) (riven_scripts.cpp:226)
==639379==    by 0x400DE2: Mohawk::RivenScriptManager::runQueuedScripts() (riven_scripts.cpp:141)
==639379==    by 0x3EF383: Mohawk::MohawkEngine_Riven::doFrame() (riven.cpp:227)
==639379==    by 0x3EFBED: Mohawk::MohawkEngine_Riven::run() (riven.cpp:202)
==639379==    by 0x3D2544: runGame(Plugin const*, Plugin const*, OSystem&, Common::String const&) (main.cpp:318)
==639379==    by 0x3D52C2: scummvm_main (main.cpp:627)

Looking around I noticed that in engines/mohawk/riven_stacks/riven_scripts.cpp, line 170 variable commandCount is used as the end condition for the loop, but it would make more sense to use variable argumentCount instead. So I tried that and now valgrind is happy and I do not hit the bug anymore. I submitted a pull request as ​https://github.com/scummvm/scummvm/pull/3811.

There is only one other call to createScriptFromData() where commandCount is different from argumentCount, namely engines/mohawk/riven_stacks/aspit.cpp:377, so it is not that surprising that this is the only place where the problem occurs.

By the way, there is some commented out code in engines/mohawk/riven_stacks/jspit.cpp:312 that perhaps would work after the fix.

[macca8]

Just curious, and not to suggest that your proposed fix won't work, but I can't help noticing that only your own personal build displays this issue.
Does building with the mohawk engine instead of the riven sub engine resolve this issue for you, without this fix?

[bluegr]

The PR has been merged, so this can be closed now. Thanks for your work!