Windows gets incorrect arguments on post-only detours

[nosoop]

We've discussed this at some point before, just getting an issue in for recordkeeping purposes (and because I got bit by this again recently, heh). No rush on this one.

Detours that use only a post-hook on Windows get incorrect arguments. Hotfix is to add a pre-hook.

Issue is present on 2.2.0-detours10.

[4:56 AM] nosoop: Peace-Maker: can't seem to get a valid thisptr with dynhooks on windows, linux gets it correctly; got a live example here: https://github.com/nosoop/SM-TFWorkshopSoundsFix -- gamedata function entry is CSoundscapeSystem::Init() and plugin callback is OnSoundscapeInitPost
[4:59 AM] nosoop: SM 1.9.0.6261 and detours6 if either of those matter
[11:54 AM] <Peace-Maker> nosoop: I thought I considered that case in particular https://bitbucket.org/Peace_Maker/dhooks2/commits/5a2f2ba517bd075771c775304f95f2833a7fdf07?at=dynhooks
[11:54 AM] <Peace-Maker> but it looks like that only works if there is a pre hook registered. So plain fact that you added a pre hook on the function should fix the this pointer in the post hook. You shouldn't have to cache the this pointer yourself in your plugin
[11:56 AM] <Peace-Maker> There are similar problems with arguments on windows where the compiler reuses the stack space of the arguments after they're not used anymore in the rest of the function. that leads to different argument values in post hooks too
[11:58 AM] <Peace-Maker> The only fix would be to silently add a pre hook to every function - even if the plugin only wanted a post hook - and save the arguments (and this-pointer)
[11:58 AM] <Peace-Maker> It's on my todo™

[nosoop]

Scratch that, it's not exactly the same issue with the thisptr specifically; it's getting some different values in the argument stack that I'm having trouble explaining.

On prehook a pointer has the address 0x009ddbbc, whereas in post it's 0x019ddbbc.

The issue should be reproducible by uncommenting lines 152-155 in this release and taking / inflicting damage on a player.

[peace-maker]

Thanks for the heads up.

That quirk is actually "documented" in the release post.

Accessing parameter values in post-hooks

Some compiler optimizations can cause the memory locations of the arguments to be reused and overwritten in the function if the compiler comes to the conclusion that the argument isn't needed anymore (mostly used on Windows). This can cause problems in post-hooks, because the real argument values aren't available anymore and you find "garbage" values instead of the expected arguments. Some pointer types can even cause a crash when DHooks tries to dereference them and the value doesn't point to valid memory anymore. If you experience problems like this, the current workaround would be to save the arguments in a pre-hook and use them in the post-hook instead of calling DHooksGetParam*.

I'm assuming this needs some changes to DynamicHooks' design, which I've been holding off of yet.

[peace-maker]

All arguments are saved before calling the original function in detours now. Even if there is only a post hook registered.