alliedmodders / sourcemod

SourceMod - Source Engine Scripting and Administration
http://www.sourcemod.net/
990 stars 425 forks source link

DHooks: Crash from detour while inside same recursive detour #1793

Open FortyTwoFortyTwo opened 2 years ago

FortyTwoFortyTwo commented 2 years ago

This issue is basically copied from old dhooks repo, just that dhooks repo is no longer used and is now maintained here.

While inside plugin's detour pre hook, If same detour were to be called again as recursive, recursive detour were to work fine, but after whole plugin's pre detour were to be done, game would eventually crash. Using different functions on this doesn't really give any same crash log as it depends on what game is doing after pre detour.

If recursive detour were to be called more than once while inside pre hook, assert were to happen at CHook::GetReturnAddress after whole pre detour were to be done. This assert also happens if it were to be recursive called at least once while inside post hook.

Example usage on recursive detour, crash/assert after whole pre detour is done with all of the recursive:

#include <sdktools>
#include <dhooks>

Handle g_hSDKCanPlayerTeleport;

public void OnPluginStart()
{
    GameData hGameData = new GameData("sm-tf2.games");

    DynamicDetour hDetour = new DynamicDetour(Address_Null, CallConv_THISCALL, ReturnType_Bool, ThisPointer_CBaseEntity);
    hDetour.SetFromConf(hGameData, SDKConf_Signature, "CanPlayerTeleport");
    hDetour.AddParam(HookParamType_CBaseEntity);
    hDetour.Enable(Hook_Pre, DHook_CanPlayerTeleport);

    StartPrepSDKCall(SDKCall_Entity);
    PrepSDKCall_SetFromConf(hGameData, SDKConf_Signature, "CanPlayerTeleport");
    PrepSDKCall_AddParameter(SDKType_CBaseEntity, SDKPass_Pointer);
    PrepSDKCall_SetReturnInfo(SDKType_Bool, SDKPass_Plain);
    g_hSDKCanPlayerTeleport = EndPrepSDKCall();
    if (g_hSDKCanPlayerTeleport == null)
        LogMessage("CanPlayerTeleport");
}

public MRESReturn DHook_CanPlayerTeleport(int iTeleporter, DHookReturn hReturn, DHookParam hParams)
{
    static bool bSkip;
    if (bSkip)
        return MRES_Ignored;

    bSkip = true;   //Prevent infinite loop
    SDKCall(g_hSDKCanPlayerTeleport, iTeleporter, DHookGetParam(hParams, 1));
    //SDKCall(g_hSDKCanPlayerTeleport, iTeleporter, DHookGetParam(hParams, 1)); //Uncommenting this would assert instead of usual crash
    bSkip = false;
    return MRES_Ignored;
}
jensewe commented 9 months ago

https://github.com/jensewe/sourcemod/commit/078ca8f1c5517ea412e4dddd430bb36a6f229e6b

Got a working version for this case, but not carefully tested and no idea regarding improvements on this ugly workaround (I'm kinda noob at C++). Appreciate if anyone could proceed.

FortyTwoFortyTwo commented 9 months ago

Compiled and tested it myself on TF2 windows, and using my Randomizer plugin to heavily make use of recursive detours, didn't experienced any issues or crashes from it!

Can't say if code is the best solution to use, but at the very least it works.