Opened 8 months ago

Closed 8 months ago

#13416 closed defect (fixed)

MOHAWK: Riven: Cannot create a Switch command from data

Reported by: marcvinyals Owned by: bluegr
Priority: normal Component: Engine: Mohawk
Version: Keywords:
Cc: marcvinyals Game: Riven

Description

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.

Attachments (1)

riven-002.rvn (24.2 KB ) - added by marcvinyals 8 months ago.

Download all attachments as: .zip

Change History (10)

by marcvinyals, 8 months ago

Attachment: riven-002.rvn added

comment:1 by eriktorbjorn, 8 months ago

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.

Last edited 8 months ago by eriktorbjorn (previous) (diff)

comment:2 by antoniou79, 8 months ago

Component: --Unset--Engine: Mohawk
Summary: Riven: Cannot create a Switch command from dataMOHAWK: Riven: Cannot create a Switch command from data

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?

comment:3 by marcvinyals, 8 months ago

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.

comment:4 by eriktorbjorn, 8 months ago

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.)

comment:5 by marcvinyals, 8 months ago

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.

Last edited 8 months ago by marcvinyals (previous) (diff)

comment:6 by eriktorbjorn, 8 months ago

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 -

comment:7 by marcvinyals, 8 months ago

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/jspit.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/riven_scripts.cpp:312 that perhaps would work after the fix.

Version 2, edited 8 months ago by marcvinyals (previous) (next) (diff)

comment:8 by macca8, 8 months ago

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?

comment:9 by bluegr, 8 months ago

Owner: set to bluegr
Resolution: fixed
Status: newclosed

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

Note: See TracTickets for help on using tickets.