Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed ACS scripts executing a function from another library that cause a division/modulus of zero from aborting the game #2910

Merged

Conversation

AKMDM
Copy link
Contributor

@AKMDM AKMDM commented Jan 24, 2025

A while ago, I discovered this issue from a Zandronum mod that caused Zandronum to crash, and only looked further into it recently. Currently in GZDoom, this issue causes a fatal error but no crash.

If an ACS script (let's pretend said script is defined in a library called "LIB1") executes a function that's defined in another library (let's also pretend this library is called "LIB2"), and that function causes a division/modulus of zero and forces the script to terminate, the game will abort because of this code:

if (runaway != 0 && InModuleScriptNumber >= 0)
{
	auto scriptptr = activeBehavior->GetScriptPtr(InModuleScriptNumber);
	if (scriptptr != nullptr)
	{
		scriptptr->ProfileData.AddRun(runaway);
	}
	else
	{
		// It is pointless to continue execution. The script is broken and needs to be aborted.
		I_Error("Bad script definition encountered. Script %d is reported running but not present.\nThe most likely cause for this message is using 'delay' inside a function which is not supported.\nPlease check the ACS compiler used for compiling the script!", InModuleScriptNumber);
	}
}

The reason why this happens is because when a function from another library/module is executed, DLevelScript::activeBehavior is temporarily changed to that of the function. There are several or more p-codes that can change the script's state to SCRIPT_DivideBy0 or SCRIPT_ModulusBy0, but they won't revert activeBehavior back to savedActiveBehavior, unlike when the script hits an unknown p-code. As a result, scriptptr will be invalid when the code above executes.

Adding this check just before the code snippet above fixes the error:

// There are several or more p-codes that can trigger a division or modulus of zero.
// Reset the active behavior back to the original if this happens.
if (state == SCRIPT_DivideBy0 || state == SCRIPT_ModulusBy0)
	activeBehavior = savedActiveBehavior;

Here's a link to a minimal example WAD to test:

  • Typing pukename test1 in the console executes an ACS script that terminates properly when encountering a division of zero.
  • Typing pukename test2 executes an ACS script that causes the game to abort instead without the fix.

…e a division/modulus of zero from aborting the game.
@RicardoLuis0 RicardoLuis0 merged commit 162ab3c into ZDoom:master Jan 28, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants