GOB: Adibou 2 (Demo): random setGoblinState() SIGSEGV at game start

[dwatteau]

Current Git HEAD, with the Adibou 2 French demo (adibou2-demo-win-fr) at: <​https://downloads.scummvm.org/frs/demos/gob/adibou2-dos-demo-fr.zip>

The crash appears around 1 time out of 5, when starting a new game.

In order to trigger it:

1. Start a new game of the demo above
2. Create a new character (or load an exiting character; I've attached mine below)
3. Wait for a new game to start

In the first few seconds of the game (e.g. when there's the visual effect gradually showing Adibou's world, or when Adibou appears on screen and everyone dances a bit), a SIGSEGV can randomly happen.

Here's a quick summary of the crash in GDB:

Program received signal SIGSEGV, Segmentation fault.
0x112c40c0 in Gob::Goblin_v7::setGoblinState (this=0x16d75328, obj=0x16da942a, animState=5791) at engines/gob/goblin_v7.cpp:79
79                              if (animVariablesForState[0] == 0) { 

(gdb) p animVariablesForState
$1 = (int16 *) 0x17a996a4
(gdb) p *animVariablesForState
Cannot access memory at address 0x17a996a4

(gdb) bt
#0  0x112c40c0 in Gob::Goblin_v7::setGoblinState (this=0x16d75328, obj=0x16da942a, animState=5791) at engines/gob/goblin_v7.cpp:79
#1  0x113026ec in Gob::Inter_v7::o7_setGoblinState (this=0x16d038d8) at engines/gob/inter_v7.cpp:324
#2  0x1130bd0c in Common::Functor0Mem<void, Gob::Inter_v7>::operator() (this=0x16bbaa28) at ./common/func.h:397
#3  0x112c7bcc in Gob::Inter::executeOpcodeDraw (this=0x16d038d8, i=85 'U') at engines/gob/inter.cpp:80
#4  0x112d5b00 in Gob::Inter_v1::o1_drawOperations (this=0x16d038d8, params=...) at engines/gob/inter_v1.cpp:1420
#5  0x112dfb80 in Common::Functor1Mem<Gob::OpFuncParams&, void, Gob::Inter_v1>::operator() (this=0x16ca2188, v1=...) at ./common/func.h:460
#6  0x112c7e04 in Gob::Inter::executeOpcodeFunc (this=0x16d038d8, i=1 '\001', j=14 '\016', params=...) at engines/gob/inter.cpp:91
#7  0x112c91bc in Gob::Inter::funcBlock (this=0x16d038d8, retFlag=2) at engines/gob/inter.cpp:323
#8  0x112c93a0 in Gob::Inter::callSub (this=0x16d038d8, retFlag=2) at engines/gob/inter.cpp:352
#9  0x112d260c in Gob::Inter_v1::o1_callSub (this=0x16d038d8, params=...) at engines/gob/inter_v1.cpp:730
#10 0x112dfb80 in Common::Functor1Mem<Gob::OpFuncParams&, void, Gob::Inter_v1>::operator() (this=0x16db8468, v1=...) at ./common/func.h:460
#11 0x112c7e04 in Gob::Inter::executeOpcodeFunc (this=0x16d038d8, i=0 '\000', j=0 '\000', params=...) at engines/gob/inter.cpp:91
#12 0x112c91bc in Gob::Inter::funcBlock (this=0x16d038d8, retFlag=0) at engines/gob/inter.cpp:323
#13 0x112d3668 in Gob::Inter_v1::o1_if (this=0x16d038d8, params=...) at engines/gob/inter_v1.cpp:929
#14 0x112dfb80 in Common::Functor1Mem<Gob::OpFuncParams&, void, Gob::Inter_v1>::operator() (this=0x16c8a860, v1=...) at ./common/func.h:460
#15 0x112c7e04 in Gob::Inter::executeOpcodeFunc (this=0x16d038d8, i=0 '\000', j=8 '\b', params=...) at engines/gob/inter.cpp:91
#16 0x112c91bc in Gob::Inter::funcBlock (this=0x16d038d8, retFlag=2) at engines/gob/inter.cpp:323
#17 0x112c93a0 in Gob::Inter::callSub (this=0x16d038d8, retFlag=2) at engines/gob/inter.cpp:352
#18 0x112d260c in Gob::Inter_v1::o1_callSub (this=0x16d038d8, params=...) at engines/gob/inter_v1.cpp:730
#19 0x112dfb80 in Common::Functor1Mem<Gob::OpFuncParams&, void, Gob::Inter_v1>::operator() (this=0x16db8468, v1=...) at ./common/func.h:460

[cut]

Fuller, much longer GDB trace attached below.

[dwatteau]

Full GDB trace when the crash happens

[dwatteau]

The character I've created and which I load before the crash happens (1 time out of 5)

[bluegr]

I've tried this, and can't reproduce the crash...
What sound options are you using?

[sdelamarre]

In 1de09bc2:

GOB: Bugfix in Goblin_v7::setGoblinState (Adibou2) - bug #15516

[sdelamarre]

I could not reproduce the crash either - but this opcode does some dangerous computations, where pointers offsets are read from games variables, so it is easy to run into undefined behaviour.

And after comparing carefully with the original, I found that we were missing a "break" statement on line 82. This would explain the crash pretty well.

I pushed the correction on master (1de09b), can you check if it fixes the issue @dwatteau ? If so I will backport it to 2.9.

[dwatteau]

Thanks, I applied commit 1de09bc2 above, but unfortunately, that didn't fix the issue; I still get the same GDB backtrace.

I'm attaching a Valgrind log when this happens, and also my scummvm.ini config file.

FWIW, I can reproduce the problem with the VM documented in <​https://wiki.scummvm.org/index.php?title=HOWTO-Debug-Endian-Issues> (it might require a few tries at pressing "Esc" when the first playable room appears). Just building this engine, with -O0.

[dwatteau]

Valgrind log when the problem happens

[dwatteau]

scummvm.ini file for this game

[dwatteau]

On regular macOS/x64, I can't reproduce the crash, but here are some findings with the Sanitizer build options:

--enable-asan: so far, nothing

--enable-ubsan: got this, once, when starting the game:

engines/gob/draw_v7.cpp:206:9: runtime error: load of value 36, which is not a valid value for type 'bool'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior engines/gob/draw_v7.cpp:206:9 in 

good to be fixed, but it's probably unrelated to the current issue.

--enable-tsan: got one trace when starting a new game (after several tries), and another one when quitting the game and returning to the ScummVM launcher. Both full traces attached below.

[dwatteau]

ThreadSanitizer output when starting a new game (after several tries)

[dwatteau]

ThreadSanitizer output when quitting a game and returning to the ScummVM launcher

[sdelamarre]

In 83eece56:

GOB: Bugfix in Goblin_v7::setGoblinState (Adibou2) - bug #15516

[sdelamarre]

Thanks for the traces, actually my previous change was a no-op, the so-called "missing break statement" was actually done a few lines below - I missed it because the nesting blocks organization was quite contrived.

I simplified the nesting in that function (commit fb5155) and fixed the uninitialized variable detected by your ubsan (b07f0d) but this will probably not solve the crash. The setGoblinState() function seems in par with the original now, it would mean that the issue is elsewhere in the engine, at the time the game variables read by setGoblinState() were written, which will be longer to track.

[dwatteau]

Thanks for the changes! I've tested them, but indeed so far, the bug still remains. I'm including an updated GDB backtrace, but it looks mostly the same (as is probably expected).

[dwatteau]

Updated GDB backtrace with 2024-11-27 changes

[dwatteau]

I got an AddressSanitizer trace when doing an --enable-asan build GCC 5.5.0 on that environment: <​https://wiki.scummvm.org/index.php?title=HOWTO-Debug-Endian-Issues>. Had to start a new game several times before hitting the issue.

Log attached below.

[dwatteau]

ASAN trace when building with GCC 5.5.0 (--enable-asan --disable-optimizations --enable-debug) on the Debian 8 powerpc VM described on the wiki

[sdelamarre]

In 8884cc4d:

GOB: Fix endianess issue in Goblin_v7::setGoblinState (Adibou2) - bug #15516

[sdelamarre]

In e6030ea3:

GOB: Fix endianess issue in Goblin_v7::setGoblinState (Adibou2) - bug #15516

[sdelamarre]

Finally fixed - it turned out to be an endianness issue. Many thanks to @dwatteau for his support!