Reloaded-Project / Reloaded.Hooks

Advanced native function hooks for x86, x64. Welcome to the next level!
GNU Lesser General Public License v3.0
213 stars 33 forks source link

Default to standard x64 calling convention if none is supplied #3

Closed NotAdam closed 4 years ago

NotAdam commented 4 years ago

This changes the behaviour around hooks where a default calling convention isn't supplied, though I'm not 100% sure if this is a best-fit fix, but didn't want to also write a bunch of extra code to pass a default through a Hook constructor if it's not necessary.

I figure that this is a reasonable default for x64 given it's not the hellscape that x86 is so a default option won't cause any unexpected breakages (or unlikely to), but still allows for the previous behaviour. x86 has been left as is given the aformentioned hellscape that is x86 calling conventions.

If you'd prefer this to be done via a constructor overload (or something else) I can move things around.

Sewer56 commented 4 years ago

I figure that this is a reasonable default for x64 given it's not the hellscape that x86 is so a default option won't cause any unexpected breakages (or unlikely to), but still allows for the previous behaviour.

Sure works for me no problem. I don't work with x64 applications very often but from experience the default convention is used very often anyway in compiled programs; as it already does register parameter passing.

NotAdam commented 4 years ago

I don't work with x64 applications very often but from experience the default convention is used very often anyway in compiled programs; as it already does register parameter passing.

Yeah, that's why I figured this would be a good way of fixing this particular thing so I don't break a bunch of third party code in the process of migrating to this lib.

Thanks for the merge. When you get a chance would you be able to publish a new nuget package?

Sewer56 commented 4 years ago

My current plan is to update immediately after the first .NET 5 preview that implements UnmanagedCallersOnly attribute, since I added function pointer support in the net5 branch: https://github.com/Reloaded-Project/Reloaded.Hooks/tree/net5

It seems that at the current moment, this attribute is ignored and all functions are treated as stdcall under x86 as opposed to cdecl, which is what this library creates wrappers for.

x64 is working fine though obviously due to default convention.

Sewer56 commented 4 years ago

Forgot to mention.

Sadly I wouldn't get much use out of function pointer support myself though, because of https://github.com/dotnet/runtime/issues/39176 ; which makes me unable to release Reloaded-II under .NET 5, even though everything is working fine on my end. ¯\(ツ)

NotAdam commented 4 years ago

I did see the .NET 5 stuff, there's a few things coming soon that look pretty good for this kind of thing.

That's an interesting issue though, because we're looking at moving to .net 5 (and ditching EasyHook) and that bug repro is how I was looking to implement it which is slightly problematic. Likely we'll be stuck on framework until that's fixed then, thanks for the heads up.

Can probs just submodule for now until then, main thing was ditching EasyHook because it's completely broken when it comes to hooking

Sewer56 commented 4 years ago

For the record, you're still a-ok if you are getting .NET code execution from the target program's primary thread (e.g. hook some function the program will execute down the road). But obviously, that's no good, as depending on your use case, you often will want to hook something executed at startup.

In the meantime if interested you're still a-ok moving to Core 3.1 though as this is a .NET 5 regression, you shouldn't need to change any code to migrate from 3.1 to 5 if they ever decide to fix it. Well, that is outside of updating the target framework in your .csproj of course. I did upgrade Reloaded-II to .NET 5 after all, but had to downgrade again due to the issue.

NotAdam commented 4 years ago

That wouldn't really work in our case, ideally we can start the process suspended, then load hostfx and then you're able to hook things pre init. Only real fix I guess would be to hijack the main thread and run our own code first, then set rip back to the game entrypoint but that's pretty gross.

Worst case scenario, that'd work but the less I have to touch the easier it becomes to manage. Will sit on it for now anyway though, appreciate the insight. See what happens when .NET 5 comes out lol

Sewer56 commented 4 years ago

ideally we can start the process suspended, then load hostfx and then you're able to hook things pre init

Reloaded-II does exactly this, so feel free to use it as reference.

Only real fix I guess would be to hijack the main thread and run our own code first,

Yeah hijacking the entry point is possible. Personally I didn't like that idea though, I agree it sounds way too hacky for our own good; especially since my own goal is to support any program too.

See what happens when .NET 5 comes out lol

I bet we'll be stuck waiting in limbo given it was milestoned for 6.0.0; unless somehow you can convince enough people to updoot the issue to encourage the developers working for a certain multi billion dollar corporation.

Dormanil commented 3 years ago

Looks like they are backporting the fix for the issue you found into .NET 5. https://github.com/dotnet/runtime/pull/44267