alliedmodders / sourcemod

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

Add proper support for detouring at instruction addresses with DHooks #1956

Closed chrb22 closed 1 year ago

chrb22 commented 1 year ago

Help us help you

Environment

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 https://github.com/alliedmodders/sourcemod/compare/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): https://github.com/alliedmodders/sourcemod/blob/a9a1939f75fb5e63eba9d937da62c747f38dd16c/extensions/dhooks/DynamicHooks/hook.cpp#L264-L295

KyleSanderson commented 1 year ago

Are you open to making this a pull request?

chrb22 commented 1 year ago

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 commented 1 year ago

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