[thesupremecommander]

(I'm not sure where you should post for SourceMod extension help, so I'm putting it in this forum. Feel free to move it if that's not the case.)

I'm having a pretty weird problem. I'm trying to hook two functions, SetWinningTeam and SetStalemate, which are both in CTFGameRules, and call forwards so that a plugin can modify the values. For SetWinningTeam, it's working perfectly fine, but for SetStalemate, it throws an exception "0xC0000005: Access violation writing location" when I modify the parameter. The extension and plugin both compile fine without errors, everything else is working without a hitch, and if I comment out the code changing the parameter, the SetStalemate forward seems to work perfectly fine. I have no idea why one function would work and the other wouldn't, since I don't think that there's a major difference between either that would make an impact. Of course, I may be dumb and missing a simple error, but I can't find it if I am.

Relevant part of IDA dump for CTFGameRules:

// Auto reconstructed from vtable block @ 0x00EC18C0 // from "server_srv.so", by ida_vtables.idc 156 CTFGameRules::SetWinningTeam(int,int,bool,boo l,bool) 157 CTFGameRules::SetStalemate(int,bool,bool)

Code for creating forwards:

		g_SetWinningTeamForward = forwards->CreateForward("CompCtrl_OnSetWinningTeam", ET_Hook, 5, NULL, Param_CellByRef, Param_CellByRef, Param_CellByRef, Param_CellByRef, Param_CellByRef);
		g_SetStalemateForward = forwards->CreateForward("CompCtrl_OnSetStalemate", ET_Hook, 3, NULL, Param_CellByRef, Param_CellByRef, Param_CellByRef);

The code calling the forwards:

void GameRulesManager::Hook_CTFGameRules_SetWinningTeam(int team, int iWinReason, bool bForceMapReset, bool bSwitchTeams, bool bDontAddScore) {
	cell_t forceMapReset = bForceMapReset;
	cell_t switchTeams = bSwitchTeams;
	cell_t dontAddScore = bDontAddScore;

	g_SetWinningTeamForward->PushCellByRef(&team);
	g_SetWinningTeamForward->PushCellByRef(&iWinReason);
	g_SetWinningTeamForward->PushCellByRef(&forceMapReset);
	g_SetWinningTeamForward->PushCellByRef(&switchTeams);
	g_SetWinningTeamForward->PushCellByRef(&dontAddScore);

	cell_t result = 0;

	g_SetWinningTeamForward->Execute(&result);

	if (result > Pl_Changed) {
		RETURN_META(MRES_SUPERCEDE);
	}
	else if (result == Pl_Changed) {
		RETURN_META_MNEWPARAMS(MRES_HANDLED, CTFGameRules_SetWinningTeam, (team, iWinReason, (bool)forceMapReset, (bool)switchTeams, (bool)dontAddScore));
	}
	else {
		RETURN_META(MRES_IGNORED);
	}
}

void GameRulesManager::Hook_CTFGameRules_SetStalemate(int iReason, bool bForceMapReset, bool bSwitchTeams) {
	cell_t forceMapReset = bForceMapReset;
	cell_t switchTeams = bSwitchTeams;

	g_SetWinningTeamForward->PushCellByRef(&iReason);
	g_SetWinningTeamForward->PushCellByRef(&forceMapReset);
	g_SetWinningTeamForward->PushCellByRef(&switchTeams);

	cell_t result = 0;

	g_SetStalemateForward->Execute(&result);

	if (result > Pl_Changed) {
		RETURN_META(MRES_SUPERCEDE);
	}
	else if (result == Pl_Changed) {
		RETURN_META_MNEWPARAMS(MRES_HANDLED, CTFGameRules_SetStalemate, (iReason, (bool)forceMapReset, (bool)switchTeams));
	}
	else {
		RETURN_META(MRES_IGNORED);
	}
}

The relevant forward definitions:

/**
 * @brief 				When SetWinningTeam in CTFGameRules is called by the game.
 *
 * @param team			The team that has won.
 * @param reason		The reason why the team is winning.
 * @param forceMapReset	Whether to reset the map (usually after a full round is completed).
 * @param switchTeams	Whether to switch teams.
 * @param dontAddScore	Whether to not score this round.
 * @return				Plugin_Continue to continue with original values, Plugin_Changed to use new values, Plugin_Handled or Plugin_Stop to stop the game call.
 */
forward Action:CompCtrl_OnSetWinningTeam(&TFTeam:team, &WinReason:reason, &bool:forceMapReset, &bool:switchTeams, &bool:dontAddScore);

/**
 * @brief 				When SetStalemate in CTFGameRules is called by the game.
 *
 * @param reason		The reason why the team is winning.
 * @param forceMapReset	Whether to reset the map (usually after a full round is completed).
 * @param switchTeams	Whether to switch teams.
 * @return				Plugin_Continue to continue with original values, Plugin_Changed to use new values, Plugin_Handled or Plugin_Stop to stop the game call.
 */
forward Action:CompCtrl_OnSetStalemate(&WinReason:reason, &bool:forceMapReset, &bool:switchTeams);

The relevant plugin code:

public Action:CompCtrl_OnSetWinningTeam(&TFTeam:team, &WinReason:reason, &bool:forceMapReset, &bool:switchTeams, &bool:dontAddScore) {
	if (g_InMatch) {
		if (g_SwitchTeams) {
			if (GameRules_GetProp("m_bStopWatch", 1) && GetStopwatchStatus() == StopwatchStatus_ChaseTarget) {
				switchTeams = false;
			}
			else {
				switchTeams = true;
			}
			
			g_SwitchTeams = false;
			return Plugin_Changed;
		}
	}
	
	return Plugin_Continue;
}

public Action:CompCtrl_OnSetStalemate(&WinReason:reason, &bool:forceMapReset, &bool:switchTeams) {
	if (g_InMatch) {
		if (g_SwitchTeams) {
			if (GameRules_GetProp("m_bStopWatch", 1) && GetStopwatchStatus() == StopwatchStatus_ChaseTarget) {
				switchTeams = false;
			}
			else {
				switchTeams = true;
			}
			
			g_SwitchTeams = false;
			return Plugin_Changed;
		}
	}
	
	return Plugin_Continue;
}

If anyone can shed some light into where the problem is occurring, that would be insanely helpful. Thanks in advance!

[Powerlord]

...you're better off using the DHooks extension instead of writing your own extension.

Incidentally, iReason, team, and iWinReason aren't actually cells... I'm surprised you don't get a compiler warning or error trying to use them as such.

[asherkin]

It's late and I only glanced over it, so I'm probably miles off, but my first hunch would be that SetStalemate isn't actually a void return, try changing it to an int (well, anything that's 4-bytes).

[thesupremecommander]

Originally Posted by Powerlord ...you're better off using the DHooks extension instead of writing your own extension. Incidentally, iReason, team, and iWinReason aren't actually cells... I'm surprised you don't get a compiler warning or error trying to use them as such.

I wrote my own extension intentionally, since I want to distribute a plugin with all of the requirements (except for SourceMod/MM:S) self-contained, and also because I wanted to learn how to properly write an SM extension.

There isn't a compiler warning, I presume, because a cell_t is an int32_t, and the int casts without a warning. I'll change it to avoid any sort of problems.

Originally Posted by asherkin It's late and I only glanced over it, so I'm probably miles off, but my first hunch would be that SetStalemate isn't actually a void return, try changing it to an int (well, anything that's 4-bytes).

Debugging is showing that it's crashing with the forward call, I think, because the next statement is the if statement. It could be that the return type is different, but I would hope not, because I'm relying on the SDK definition.

--

UPDATE: I figured out how to attach the SourceMod symbols to the debugger. Here's the stack trace I pulled.

 	137afda5()	Unknown
 	[Frames below may be incorrect and/or missing]	
>	sourcepawn.jit.x86.dll!JITX86::InvokeFunction(BaseRuntime * runtime, JitFunction * fn, int * result) Line 2045	C++
 	sourcepawn.jit.x86.dll!BaseContext::Execute2(SourcePawn::IPluginFunction * function, const int * params, unsigned int num_params, int * result) Line 652	C++
 	sourcepawn.jit.x86.dll!CFunction::CallFunction2(SourcePawn::IPluginContext * pContext, const int * params, unsigned int num_params, int * result) Line 59	C++
 	sourcepawn.jit.x86.dll!CFunction::Execute2(SourcePawn::IPluginContext * ctx, int * result) Line 281	C++
 	sourcepawn.jit.x86.dll!CFunction::Execute(int * result) Line 168	C++
 	sourcemod.logic.dll!CForward::Execute(int * result, SourceMod::IForwardFilter * filter) Line 384	C++
 	compctrl.ext.2.tf2.dll!GameRulesManager::Hook_CTFGameRules_SetStalemate(int iReason, bool bForceMapReset, bool bSwitchTeams) Line 140	C++
 	compctrl.ext.2.tf2.dll!fastdelegate::FastDelegate3<int,bool,bool,void>::operator()(int p1, bool p2, bool p3) Line 1164	C++
 	compctrl.ext.2.tf2.dll!__SourceHook_MFHCls_CTFGameRules_SetStalemate::CMyDelegateImpl::Call(int p1, bool p2, bool p3) Line 7	C++
 	compctrl.ext.2.tf2.dll!__SourceHook_MFHCls_CTFGameRules_SetStalemate::Func(int p1, bool p2, bool p3) Line 7	C++

It seems like the exception is being thrown while executing the function attached to the forward, but I'm not sure as to why.

[Powerlord]

Originally Posted by asherkin It's late and I only glanced over it, so I'm probably miles off, but my first hunch would be that SetStalemate isn't actually a void return, try changing it to an int (well, anything that's 4-bytes).

You'd think so, but...

void CTeamplayRoundBasedRules::SetStalemate( int iReason, bool bForceMapReset /* = true */, bool bSwitchTeams /* = false */ )

is the definition in SourceSDK 2013.

[asherkin]

Slightly concerning... what's the output of 'sm version'?

[thesupremecommander]

This is the output of "sm version". It should be the latest version on the 1.6-dev branch that I compiled manually for testing, and the extension and plugins were built using this same version.

] sm version SourceMod Version Information: SourceMod Version: 1.6.1-dev+4544 SourcePawn Engine: SourcePawn 1.2, jit-x86 (build 1.6.1-dev+4544) SourcePawn API: v1 = 4, v2 = 6 Compiled on: Aug 1 2014 18:14:06 Built from: https://github.com/alliedmodders/sou...commit/7b45e96 Build ID: 4544:7b45e96 http://www.sourcemod.net/

If you'd like to see my code for the purposes of testing, it's available in a GitHub repository. The actual project seems to compile fine and without errors.

Since the included plugins are complex and not really suited for testing, this small plugin successfully reproduces the error:

#include <sourcemod>

#include <compctrl>

public Action:CompCtrl_OnSetWinningTeam(&TFTeam:team, &WinReason:reason, &bool:forceMapReset, &bool:switchTeams, &bool:dontAddScore) {
	switchTeams = true;
	
	return Plugin_Continue;
}

public Action:CompCtrl_OnSetStalemate(&StalemateReason:reason, &bool:forceMapReset, &bool:switchTeams) {
	switchTeams = true;
	
	return Plugin_Continue;
}

As before, it doesn't crash in CompCtrl_OnSetWinningTeam (like when capturing final point on a 5CP map), but it does in CompCtrl_OnSetStalemate (like when the timelimit expires and scores are tied for both teams).

[thesupremecommander]

This is a case of "I'm an idiot".

I was pushing parameters to one (incorrect) forward and executing another (correct) forward. Obviously, since no proper cells were pushed to the executed forward, the referenced memory locations would be invalid and thus cause a crash when they were modified.

I'm sorry for wasting your time.

[asherkin]

Originally Posted by thesupremecommander This is a case of "I'm an idiot". I was pushing parameters to one (incorrect) forward and executing another (correct) forward. Obviously, since no proper cells were pushed to the executed forward, the referenced memory locations would be invalid and thus cause a crash when they were modified. I'm sorry for wasting your time.

Hah, it's so obvious once you notice it! Totally overlooked that as well. I wonder if we can make this safer though.

[thesupremecommander]

Since forwards are supposed to be registered with a list of parameters, perhaps throwing an exception on execute if the parameters are not of the correct type would work.