Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add dynamic detour support #1

Open
wants to merge 104 commits into
base: master
Choose a base branch
from

Conversation

peace-maker
Copy link
Collaborator

Enable plugins to detour any function in the game binaries from sourcepawn just like they can for virtual functions with DHooks already.

This uses Ayuto's https://github.com/Ayuto/DynamicHooks library as a base.

Documentation in this post:
https://forums.alliedmods.net/showthread.php?p=2588686#post2588686

We should bump the version to something without the -detoursX suffix after merge.

The custom return value was lost when calling the original function.
Save and restore our own return value, if we're about to call the original function.
ecx might get cleared before the original function returns leading to garbage in the post handler.
The esp pointer wasn't removed from the map after the function was called and the original return address was retrieved.
If the same function was called again with the same esp this would fail due to there already being an (old) return address associated with the esp.
If the compiler decided to pass an argument in a register on an internal function instead of pushing it on the stack to save time, allow us to specify the register the parameter is going to be in.

DHookAddParam received another parameter to set the register.
The post hook would have cleared the esp value from the return address map, but there seems to be a case where the post hook isn't called, but the function gets called with the same esp again. This is probably just masking a different error :(
…s on the stack

If a function was optimized to only pass one parameter in a register, but still pass other parameters on the stack, save the registers at the correct offset in the buffer.
…ignore it

The this pointer was always passed to the plugin callback if the calling convention was a thiscall. Even if the plugin author set the this pointer type to ThisPointer_Ignore.
A "Functions" section is parsed in gamedata files that allow you to define the signature of functions including metadata like the calling convention and specifying the register an argument is passed in.
A new native DHookCreateFromConf can be used to setup a hook or detour from one of that function sections in the "Functions" section.
The this-pointer is always pushed first on the stack.
Just return null if a user tries to access an invalid argument that wasn't defined when detouring the function.
Some registers were upper case and HookParamType_CBaseEntity didn't follow the naming scheme of the other types.
Encourage passing the callback when hooking instead of when setting the hook up.
peace-maker and others added 30 commits June 9, 2020 23:30
The previous fix in ac44af9 did not remove the hook itself instead of only not calling the removal callback. So yes, the removal callback wouldn't be called anymore, but the hook wouldn't be removed causing the normal hook callback to fire after the plugin unloaded.

Drifter321#3
Vtable hooks are wrapped in a `DynamicHook` methodmap and detours in a `DynamicDetour` methodmap. Parameters and return values pushed to the callbacks are of methodmap types `DHookParam` and `DHookReturn` to easily access the values.

Most functions are mapped 1:1 with the exception of creating vtable hooks, where the callback function has to be passed in the hook function instead of during setup.

`HookRaw` dropped the `removecb` parameter, since it was never called. There currently is no way to know when to unhook the raw object.

This change doesn't affect the legacy API at all.
Save the value of arguments in a seperate buffer for the post callback.
Compiler optimizations might cause the registers or stack space to be reused
and overwritten during function execution if the value isn't needed anymore
at some point. This leads to different values in the post hook.
When the callback signature is missing a parameter that gets pushed due to the hook setup, print a nicer error message telling the user they tried to call e.g. DHookGetParam(hReturn, 1).
Forgot to increase the buffer offset.
Automatic msvc detection ftw!
Save all arguments before calling the original function
When the server is shutdown the map ends which destroys all entities. DHooks listens on OnEntityDestroyed and adds all vhooks on that entity to a list to call the removal callback on the next frame. SourceMod unloads all plugins first before unloading extensions. All plugins are unloaded on the same frame, so the removal callbacks aren't worked down on the next frame. The second plugin to be unloaded runs through the list of removed entity vhooks and checks which one belongs to itself. The first plugin was unloaded already though and left a dangling pointer in that list which is accessed by the second plugin unloading.

Clear the plugin callback reference as well. Hopefully no hook is fired on the same frame after entity removal.. Will have to revisit in that case.
Allows to compile against SourceMod 1.11+ which uses an updated AMTL without ke::Vector.
The GameData methodmap was added in SourceMod 1.10. Strip all references from the include file to support compilation with older SourceMod versions.
This helps with debugging crashes.
Also update the author list in the extension info.
Some servers (e.g. l4d2) ship an outdated libstdc++.so.6 which lacks the features we need.
We setup a Recall (changing the parameters of the called function from within the hook handler) when a plugin wants to supercede, but never call the function again. This causes a copy of the current context to be pushed, but never properly removed by actually calling the function.

This is wrong, since the original function should be skipped, so don't setup a Recall in SourceHook.

This fixes a long standing issue causing a growth of the internal context stack in SourceHook. This caused very long delays on unhook during iteration of the context stack. Another issue was that the hook result further up the call stack would be misaligned due to the additional unused context pushed in DoRecall.
 Drifter321#2
Use the known-rwx code allocator of SourcePawn like we do for the other code parts of the detour hooking logic.

There seem to be problems with trying to make the C++ heap executable on CentOS 7+, so avoid using executable heap memory in the first place.

Fixes #11
Fixes #22
If the function is optimized to expect some parameter through a register instead of the stack, DHookParam.IsNull might validate the wrong parameter. First we store the stack parameters, then the register parameters in orgParams.
The return buffer wasn't constructed correctly.
* #23 `DHookGetParamAddress` + `DHookParam.GetAddress`

* Update dhooks.inc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants