peace-maker / DHooks2

Dynamic detouring support for the DHooks 2 SourceMod extension
https://forums.alliedmods.net/showthread.php?p=2588686#post2588686
67 stars 11 forks source link

Crash from detour while inside same recursive detour #26

Open FortyTwoFortyTwo opened 3 years ago

FortyTwoFortyTwo commented 3 years ago

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;

    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;
}
peace-maker commented 3 years ago

Hm, that looks broken indeed, but do you have a real use case for calling the detoured function from its pre-detour callback multiple times?

FortyTwoFortyTwo commented 3 years ago

I have some cases for calling same function multiple times, but with different parameters or property values to get different outcomes. One example is TF2 function HandleRageGain manages different weapons, but expects client to equip only one weapon. My workaround fix is to use RequestFrame to call HandleRageGain multiple times under each weapons while outside detour.

Its possible there may be cases where such function need to be called multiple times without any delay, like returning detour's value/params, or set property values based on result from multiple calls. Although I currently don't have any examples on this.

It's an extreme scenario, but it should at least report error that recursive detour is not supported if it tough to fix.