Add proper support for detouring at instruction addresses with DHooks

[chrb22]

Help us help you

• I have checked that my issue doesn't exist yet.
• I have tried my absolute best to reduce the problem-space and have provided the absolute smallest test-case possible.
• I can always reproduce the issue with the provided description below.

Environment

• Operating System version: Linux
• Game/AppID (with version if applicable):
• Current SourceMod version: 1.11.0.6923
• Current Metamod: Source snapshot: 1.11.0-dev+1145

• I have updated SourceMod to the latest version and it still happens.
• I have updated SourceMod to the latest snapshot and it still happens.
• I have updated SourceMM to the latest snapshot and it still happens.

Description

Detouring a function that gets called a lot can end up causing performance issues. However sometimes you're only interested in a conditional part of a function, which can help avoiding a whole lot of unnecessary detours. Could also be that the function only gets interesting further into the function. It would therefore be useful to have proper support for detouring individual instructions. While it is currently possible to detour in the middle of a function at an instruction address using DHooks, it consumes 4 bytes of memory every time the instruction is reached which doesn't get cleared.

That happens because DHooks tries to create a post-hook callback. For a detour created at the start of a function, a copy of the return address at ESP+0x0 gets copied into a vector, whereafter ESP+0x0 is replaced with the post-hook handler address. The function will then RET to the post-hook, which afterwards removes the stored return address from the vector and uses it to return to the original caller. But if the detour is at an arbitrary address, then DHooks will copy and write a return address at the stack variable ESP+0x0 (can cause a crash if unlucky). Since the post-hook handler return address doesn't get used when the function finishes, it means the stored return address never gets cleared. That way the vector gets larger and larger without stopping.

I have something that can be used to handle this at master...chrb22:sourcemod:dhooks-instruction. Basically it's a new "calling convention" specifically made for detouring at instruction addresses. It works by simply not creating a post-hook.

The post-hook creation in question (called via CHook::CHook -> CHook::CreateBridge -> CHook::Write_ModifyReturnAddress):

sourcemod/extensions/dhooks/DynamicHooks/hook.cpp

Lines 264 to 295 in a9a1939

	void CHook::Write_ModifyReturnAddress(sp::MacroAssembler& masm)
	{
	// Save scratch registers that are used by SetReturnAddress
	static void* pEAX = NULL;
	static void* pECX = NULL;
	static void* pEDX = NULL;
	masm.movl(Operand(ExternalAddress(&pEAX)), eax);
	masm.movl(Operand(ExternalAddress(&pECX)), ecx);
	masm.movl(Operand(ExternalAddress(&pEDX)), edx);
	// Store the return address in eax
	masm.movl(eax, Operand(esp, 0));
	// Save the original return address by using the current esp as the key.
	// This should be unique until we have returned to the original caller.
	void (__cdecl CHook::*SetReturnAddress)(void*, void*) = &CHook::SetReturnAddress;
	masm.push(esp);
	masm.push(eax);
	masm.push(intptr_t(this));
	masm.call(ExternalAddress((void *&)SetReturnAddress));
	masm.addl(esp, 12);
	// Restore scratch registers
	masm.movl(eax, Operand(ExternalAddress(&pEAX)));
	masm.movl(ecx, Operand(ExternalAddress(&pECX)));
	masm.movl(edx, Operand(ExternalAddress(&pEDX)));
	// Override the return address. This is a redirect to our post-hook code
	m_pNewRetAddr = CreatePostCallback();
	masm.movl(Operand(esp, 0), intptr_t(m_pNewRetAddr));
	}

[KyleSanderson]

Are you open to making this a pull request?

[chrb22]

Are you open to making this a pull request?

That was my intention from the start. I only opened an issue first because a GitHub popup suggested me to read the contributing guidelines.

[KyleSanderson]

Got it - lets move to new PR. Thank you for doing that.