SAGA: IHNM: "createThread wrong scriptEntryPointNumber" when starting with Benny

[dwatteau]

With either the English/Steam or French/GOG versions of IHNM (which trigger the SAMPLE.AD/SAMPLE.OPL warning), reinstalled from scratch to make sure the data is OK.

For whatever reason, I can only reproduce this with the OSX PowerPC port:

• It also happens with ScummVM 2.2.0, so it's not a recent regression.
• Using different compiler versions doesn't fix the problem.
• It's systematic on this port, i.e. playing with Benny is impossible there.
• With Linux on the same PowerPC machine, it's fine. Maybe the OSX PowerPC environment is "luckier" in triggering this bug, especially if it's related to threading?

Anyway, if I just do this:

1. Start the game
2. Choose Benny as a character
3. Go down the stairs with Benny

then the following error always appears in the console when Benny starts speaking to AM:

Script::createThread wrong scriptEntryPointNumber!

Skipping the dialogue or enabling/disabling the subtitles doesn't change anything.

On Windows, I can't trigger the error message, but sometimes (1 out of 10?) if I reproduce the steps above, Benny suddendly teleports to the cemetery, and if I go back to the previous room, he will resume reacting to AM, as if nothing happened.

GDB and debug logs to be attached below.

[dwatteau]

GDB log when the error happens

[dwatteau]

debug (-d9) log when AM teleports Benny to the first room

[dwatteau]

It's systematic on this port, i.e. playing with Benny is impossible there.

Well, actually there's a way of working around this on this port: look at the Psych Profile in the inventory, before you fall from the stairs with Benny.

It looks like it resets the room to a proper state, and thus prevents the wrong scriptEntryPointNumber error from happening.

[dwatteau]

ASAN (with clang++ on macOS 12.6) detects a heap-use-after-free with the same Benny action. Git HEAD is 20c1dbb50d.

==13418==ERROR: AddressSanitizer: heap-use-after-free on address 0x00011d412a38 at pc 0x000100775ccc bp 0x00016fd61760 sp 0x00016fd61758
READ of size 4 at 0x00011d412a38 thread T0
    #0 0x100775cc8 in Saga::HitZone::getFlags() const objectmap.h:56
    #1 0x1006ec924 in Saga::Actor::stepZoneAction(Saga::ActorData*, Saga::HitZone const*, bool, bool) actor.cpp:551
    #2 0x100712a0c in Saga::Actor::handleActions(int, bool) actor_walk.cpp:695
    #3 0x10071da30 in Saga::Actor::direct(int) actor_walk.cpp:727
    #4 0x1007d2f48 in Saga::SagaEngine::run() saga.cpp:357
    #5 0x1000dc7e0 in runGame(Plugin const*, Plugin const*, OSystem&, Common::String const&) main.cpp:318
    #6 0x1000d8130 in scummvm_main main.cpp:619
    #7 0x1000cf19c in main macosx-main.cpp:44
    #8 0x1024b1088 in start+0x204 (dyld:arm64e+0x5088)

I'm attaching the full ASAN log below.

[dwatteau]

ASAN trace when falling from the stairs with Benny

[dwatteau]

As per #14334 (see comment no. 12) this also happens with the ECS version of Inherit the Earth.

[johnpevensie]

I meant to say the AGA Amiga version of Inherit the Earth. The ECS version of Inherit the Earth works fine.

[PushmePullyu]

Thanks for investigating this and for the ASAN trace. I can not get the "Script::createThread wrong scriptEntryPointNumber!" error message on Linux x86_64, but i have attempted a fix based on the ASAN log: ​https://github.com/PushmePullyu/scummvm/tree/debug-ihnm-use-after-free

[dwatteau]

@johnpevensie: Oops, sorry for inverting them!

Thank you very much @PushmePullyu, here's the ThreadSanitizer trace I manage to get in ITE-AGA, for reference (attaching it as .txt).

[dwatteau]

@PushmePullyu: I've tested your PR with IHNM: the AddressSanitizer error is now gone, and it appears to fix the createThread wrong scriptEntryPointNumber error on OSX PPC, cool! :)

The ThreadSanitizer errors in ITE are still there, though (but they look unrelated?), and I couldn't reproduce the original ITE error on OSX PPC, so I can't say if it fixes both issues yet.

@johnpevensie: Can you grab the updated SAGA plugin at <​https://github.com/dwatteau/scummvm/releases/download/v0.7.1/saga.plugin>, use it to overwrite /Applications/ScummVM.app/Contents/Resources/saga.plugin (from the test build from the other Trac ticket), and report whether this fixes your error in ITE AGA too? Thanks!

[johnpevensie]

This does not fix the error on the AGA Amiga version of Inherit the Earth. I still get the same error at the same place (after the intro finishes).

[dwatteau]

@johnpevensie: Thanks for your new test! I can't reproduce the error you're having with this release of Inherit the Earth on my OSX PPC systems, though.

Can you tell me at which precise moments you're hitting the Esc key (or left-clicking) during the intro, in order to trigger the issue? It's probably timing-related, as with the issue I was having in IHNM. Thanks.

[PushmePullyu]

Replying to dwatteau:

@PushmePullyu: I've tested your PR with IHNM: the AddressSanitizer error is now gone, and it appears to fix the createThread wrong scriptEntryPointNumber error on OSX PPC, cool! :)

Thanks for testing. For reference: PR for this is ​https://github.com/scummvm/scummvm/pull/5054

I have also added debug output to ​https://github.com/PushmePullyu/scummvm/tree/debug-ihnm-use-after-free
Could you compile this for @johnpevensie with debugging enabled?

[bluegr]

The PR has been merged, so this can finally be closed. Thanks for your help on resolving this!

[dwatteau]

Reopening, since the PR fixed IHNM but apparently not ITE.

I'll provide a debug test build once my tendinitis is gone

[johnpevensie]

From my tests, this is not a timing issue. I get the same error no matter when I push ESC to skip the intro, or even when I let the intro play without skipping it.

[dwatteau]

OK, thanks.

Here you go for the updated SAGA plugin with more debug output: <​https://github.com/dwatteau/scummvm/releases/download/v0.7.1/saga-createThread-debug.plugin>

Can you move/rename it to /Applications/ScummVM.app/Contents/Resources/saga.plugin again (on top of the previous 2.7.1pre test build), then start the game from the OSX Terminal with something like this:

/Applications/ScummVM.app/Contents/MacOS/scummvm ite-cd-amiga

and then paste here the Script::createThread() lines it should output when encountering the issue? Thanks!

[johnpevensie]

I tested Inherit the Earth with the new plugin. I still get the same errors. However, I did run the game from Terminal and get more output. I have also added my own annotations:

AGA AMIGA INHERIT THE EARTH OUTPUT:

Running Inherit the Earth: Quest for the Orb (AGA CD/Amiga/English)
aga.exe: e6d93bbf0f89786eb930fbc81e02810d, 202328 bytes.
ite.rtn: 749885c0f7eaab4e977dc26a41d99ad8, 18524 bytes.
ite.sounds: f09b29e3204192de7e8cc6b073cb34f5, 640284 bytes.
ite.voices: daf62750f5322da03fab395e548b0b8f, 217376825 bytes.
-the list of data files loaded by the game

WARNING: Sprite::loadList offset exceeded!
WARNING: Sprite::loadList offset exceeded!
WARNING: Sprite::loadList offset exceeded!
WARNING: Sprite::loadList offset exceeded!
WARNING: Anim::load animId=0 Excessive dimensions 320x16191, skipping!
WARNING: Anim::load animId=1 Excessive dimensions 320x16191, skipping!
WARNING: Anim::load animId=2 Excessive dimensions 320x16191, skipping!
WARNING: Anim::load animId=3 Excessive dimensions 320x16191, skipping!
WARNING: Anim::load animId=4 Excessive dimensions 320x16191, skipping!
WARNING: Anim::load animId=5 Excessive dimensions 320x16191, skipping!
WARNING: Anim::load animId=6 Excessive dimensions 320x16191, skipping!
-these warnings show during the intro. The intro plays well, with speech or without.

Script::createThread(): calling loadModule(8)
Script::createThread(): scriptEntryPointNumber: 22
Script::createThread(): _modules[8].entryPoints.size: 0
Script::createThread wrong scriptEntryPointNumber!
-These entries attempt to begin the game, but ScummVM crashes with the above error message. The below displays in the debugger:

Debugger started, type 'exit' to return to the game.
Type 'help' to see a little list of commands and variables.
ERROR: Script::createThread wrong scriptEntryPointNumber!

ECS AMIGA INHERIT THE EARTH OUTPUT:

Running Inherit the Earth: Quest for the Orb (ECS CD/Amiga/English)
ecs.exe: 29665b96c2758aec3906ebc891079234, 204228 bytes.
ecs.rtn: c9d09514839d771efdc82ad761413349, 18584 bytes.
ite.sounds: f09b29e3204192de7e8cc6b073cb34f5, 640284 bytes.
ite.voices: daf62750f5322da03fab395e548b0b8f, 217376825 bytes.
-the list of data files loaded by the game

WARNING: Sprite::loadList offset exceeded!
WARNING: Sprite::loadList offset exceeded!
WARNING: Sprite::loadList offset exceeded!
WARNING: Sprite::loadList offset exceeded!
WARNING: Anim::load animId=0 Excessive dimensions 320x16191, skipping!
WARNING: Anim::load animId=1 Excessive dimensions 320x16191, skipping!
WARNING: Anim::load animId=2 Excessive dimensions 320x16191, skipping!
WARNING: Anim::load animId=3 Excessive dimensions 320x16191, skipping!
WARNING: Anim::load animId=4 Excessive dimensions 320x16191, skipping!
WARNING: Anim::load animId=5 Excessive dimensions 320x16191, skipping!
WARNING: Anim::load animId=6 animation magic mismatch (0xeeea vs 0x44), skipping!
-these warnings show during the intro. The intro plays well in both versions, with speech or without.

Script::createThread(): calling loadModule(8)
Script::createThread(): scriptEntryPointNumber: 22
Script::createThread(): _modules[8].entryPoints.size: 39
-This begins the game. It is here that the AGA version crashes. The ECS version continues. In fact, I have other "Script::createThread()" entries from further gameplay (although brief)

Script::createThread(): calling loadModule(8)
Script::createThread(): scriptEntryPointNumber: 23
Script::createThread(): _modules[8].entryPoints.size: 39
Script::createThread(): calling loadModule(8)
Script::createThread(): scriptEntryPointNumber: 21
Script::createThread(): _modules[8].entryPoints.size: 39
Script::createThread(): calling loadModule(8)
Script::createThread(): scriptEntryPointNumber: 17
Script::createThread(): _modules[8].entryPoints.size: 39
Script::createThread(): calling loadModule(8)
Script::createThread(): scriptEntryPointNumber: 18
Script::createThread(): _modules[8].entryPoints.size: 39
Script::createThread(): calling loadModule(8)
Script::createThread(): scriptEntryPointNumber: 20
Script::createThread(): _modules[8].entryPoints.size: 39

[PushmePullyu]

Replying to johnpevensie:

Thanks for the data.

Script::createThread(): calling loadModule(8)
Script::createThread(): scriptEntryPointNumber: 22

The numbers above look good.

Script::createThread(): _modules[8].entryPoints.size: 0

This should definitely not be 0. So we now need to figure out where that value comes from.
Checking the code, it seems to be set only once on module loading, so I suggest we check for data corruption or an incompatible version first.

Could you run the following in a terminal and post the resulting file as an attachment (warning: If you already have a file named "inheritmd5.txt" in your home directory, it will be overwritten):

( cd '/path/to/ite_data_files' && find ./ -type f -exec openssl md5 -r -- {} \; ) | tee ~/inheritmd5.txt

You will need to replace "/path/to/ite_data_files" with the location of your ITE data directory (it can be found in ScummVM (Game Options->Paths->Game Path).
This will output a list of all ITE files with their md5 hashes to the terminal and to "inheritmd5.txt", which I can then compare against my version of the game.

@dwatteau: Thanks for the build, and have a speedy recovery.

[johnpevensie]

@PushmePullyu:

I followed your suggestions for testing for data corruption. The game ran fine on my Intel machine, so I thought it was a version-specific bug. However, on my Mac, it turned out I had corrupt data files. The AGA version of Inherit the Earth worked after I copied the data files all over again.

Thank you.