Skip to content

Conversation

@Kasuromi
Copy link
Member

@Kasuromi Kasuromi commented May 3, 2022

Description

Removes FastNativeDetour in favor of native detour providers.
Creates a NativeDetourHelper for a managed API of obtaining OS-specific detour providers.
Implements detour providers for funchook & Dobby
Adds a configuration option to force NativeDetourHelper to use a specific provider

Motivation and Context

The native detour providers are added so we can support a wider set of architectures in the future by providing wrappers around native detour libraries.

Abstracting the creation of detours into a single helper class obsoletes FastNativeDetour and it's DetourGenerator, leading to their removal.

How Has This Been Tested?

So far this has been tested on a build of funchook & Dobby on a Windows 10 x64 environment on the game GTFO.
Native detours applied and disposed by IL2CPPChainloader work without issue on GTFO (unity v2019.4.21f1) and so do Harmony patches.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

TODO:

  • Add paths to the funchook & Dobby binaries for UNIX platforms
  • Add checks in NativeDetourHelper for unsupported platforms

@Kasuromi Kasuromi marked this pull request as ready for review May 5, 2022 20:34
@js6pak
Copy link
Member

js6pak commented May 17, 2022

Do we still need to use monomod DynDllImport? Can't we just use normal DllImport and NativeLibrary.SetDllImportResolver?

if (!typeof(Delegate).IsAssignableFrom(typeof(T)))
throw new InvalidOperationException($"Type {typeof(T)} not a delegate type.");

return GenerateTrampoline(typeof(T).GetMethod("Invoke")).CreateDelegate(typeof(T)) as T;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason to do this whole dance instead just TrampolinePtr.AsDelegate<T>()?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was reused from the original FastNativeDetour's GenerateTrampoline implementation, but it can be rewritten as is seen on the doorstop 4 support branch

@ghorsington
Copy link
Contributor

Do we still need to use monomod DynDllImport? Can't we just use normal DllImport and NativeLibrary.SetDllImportResolver?

Firstly, I don't think NativeLibrary is supported on netstandard2.1, as this PR targets the current BepInEx master.

Secondly, does NativeLibrary have a ready attribute-based API like [DynDllImport]? Right now the main reason to use DynDll is because it makes changing the resolve path dynamically easy while still keeping DllImport-like approach (see current unix stream handler as an example). It would be nice to have a similar easy approach with NativeLibrary should we move to .NET 6 (as part of #391).

@js6pak
Copy link
Member

js6pak commented May 17, 2022

Secondly, does NativeLibrary have a ready attribute-based API like [DynDllImport]?

You can just keep using normal [DllImport]

And with SetDllImportResolver you can do literally anything (changing name, pointing to a specific file etc)
image

@Kasuromi
Copy link
Member Author

It would be nice to have a similar easy approach with NativeLibrary should we move to .NET 6 (as part of #391).

Should I retarget this PR to #391?

@ghorsington
Copy link
Contributor

It would be nice to have a similar easy approach with NativeLibrary should we move to .NET 6 (as part of #391).

Should I retarget this PR to #391?

I merged this PR into #391 (after simplifying the code using .NET 6/C#10 features).

@ghorsington
Copy link
Contributor

This was merged into master via #391

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.

3 participants