sylveon / member_thunk Goto Github PK

Also, should stop relying on default calling convention (specifying any calling convention keyword on x64/ARM/ARM64 will make it use the normal convention for that architecture)

Will let users know exactly what is implementation details that shouldn't be relied on.

Currently, while noexcept member functions are supported, noexcept function pointers aren't.

struct Foo
{
	void bar() noexcept {}
};

Foo foo;
auto thunk = member_thunk::make<void(*)(std::size_t)>(&foo, &Foo::bar); // ok
auto thunk = member_thunk::make<void(*)(std::size_t) noexcept>(&foo, &Foo::bar); // associated constraints are not satisfied

Obviously, this should only enable when the called member function is noexcept too.

https://docs.microsoft.com/en-us/cpp/build/arm64-windows-abi-conventions

May you add some examples?
Thanks!

Once microsoft/STL#795 releases in VS preview, we should remove this

Currently, member_thunk::create requires a const_cast if you pass a const function and a const this pointer.

Currently only supports single inheritance member function pointers, need to support multi, virtual and unknown MFPs (different size and/or signature in content)

What calling conventions should we support?

https://docs.microsoft.com/en-us/windows/win32/api/winnt/nf-winnt-rtladdfunctiontable

It might be undesirable to create a thunk and leave it world-writable. We should allow a way to directly allocate a page, create thunks in it, and the set that page as execute-only once done.

The api would look something like this, not unlike a stack allocator

member_thunk::thunk_page page; // allocates a page in read-write

const auto thunk1 = page.create_thunk(this, &Foo::bar);
const auto thunk2 = page.create_thunk(this, &Foo::buz);

page.finalize(); // page set to read-execute

Once thunk_page::finalize is called, trying to create any more thunks will throw an exception.

thunk_page would have ownership of the page therefore being a move-only type, and frees the page once out of scope.

It should automatically grow if required. This can be done by either using a "linked-list" form, where a thunk_page owns the next thunk_page and forwards all calls to that once it's full, or by using VirtualAlloc to reserve an entire 64k of memory, and then committing pages as required, but would also introduce a theorical cap to the number of thunks (but does not throw away virtual memory, as the allocation granularity is 64k anyways).

This would also allow to optimize calls to FlushInstructionCache and SetProcessValidTargets to only run within finalize rather than for each new thunk, as well as skipping the alignment machinery (page is already 4k aligned, and everything is already 16 or 32 bytes, so consecutive stack allocations are already aligned)

This can introduce potential use-after-free (or use-before-executable) issues, so should be used with care.

https://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-optimization-manual.pdf

Assembly/Compiler Coding Rule 12. (M impact, H generality) All branch targets should be 16-byte aligned

Should help the prefetcher, and also required for #7 because SetProcessValidCallTargets wants 16-byte alignment. (Should we pad thunk objects to the nearest multiple of 16 with nops too?)

HeapAlloc can't provide custom alignment, but VirtualAlloc2 (which can) allocates in blocks of 4KB, so is hugely innefficient (although it allows us to put back write protection once the thunk is generated). Can continue using HeapAlloc but pad allocations, or implement own heap on top of VirtualAlloc2 pages.

ARM should be fine with that alignment too

Can we make member_thunk::make(this, &Foo::bar) valid and deduce the return type?

https://docs.microsoft.com/en-us/cpp/build/overview-of-arm-abi-conventions

There currently is a workaround for microsoft/STL#1183 here:

It should be removed when possible

https://learn.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-setprocessdynamicenforcedcetcompatibleranges

See https://godbolt.org/z/f3ji39 and https://godbolt.org/z/q28aVX. The new requirement should be std::is_pointer_v<T> || (std::is_integral_v<T> && sizeof(T) == sizeof(void*))

Avoid frame pointers

This allows control flow guard in the thunks themselves rather than just for the caller.

Wouldn't want to suppress any special C++ behaviors related to constructors or destructors. (does it even have the same ABI with a pointer-sized struct? might need is_scalar instead)