Consolidate the duplicated VtableHook into one shared copy

audio_hook.cpp carried its own VtableHook class identical to the shared
hook/src/vtable_hook.hpp -- two copies of the same delicate vtable-swap unhook
logic to keep in sync. Drop the audio copy and use the shared one (it's in
namespace coop::hook, so the in-file references resolve to it), leaving a short
note on why WASAPI methods are vtable-swapped rather than inline-hooked. Update
the shared header's comment to name both users (audio + DirectInput).

Last item from the review pass -- the Roadmap's Current-work section is now empty
(done work lives in git history); only Future work remains. Validated by
audio_hook_test (the vtable swap still hooks GetService/GetBuffer/ReleaseBuffer
end to end).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2026-06-24 02:28:05 +02:00
parent 5b2334f6e6
commit e4aafa08db
3 changed files with 8 additions and 73 deletions

View File

@@ -108,14 +108,6 @@ default** and covers anything the hooked path doesn't.
## Roadmap
### Current work
From an in-depth review pass. Each item is fixed test-first (a failing test, then the fix) and lands
as its own commit; "verify" items are confirmed real before any change, and dropped if not.
Code quality:
- **Consolidate `VtableHook`** — remove the duplicate copy in `audio_hook.cpp`.
### Future work
- **Per-game profiles** — persist each game's subsystem / capture-mode / audio choices and re-apply

View File

@@ -14,6 +14,7 @@
#include "debug_log.hpp"
#include "hook_registry.hpp"
#include "rate_estimator.hpp"
#include "vtable_hook.hpp"
namespace coop::hook
{
@@ -41,69 +42,11 @@ using GetServiceFn = HRESULT(STDMETHODCALLTYPE*)(IAudioClient*, REFIID, void**);
using GetBufferFn = HRESULT(STDMETHODCALLTYPE*)(IAudioRenderClient*, UINT32, BYTE**);
using ReleaseBufferFn = HRESULT(STDMETHODCALLTYPE*)(IAudioRenderClient*, UINT32, DWORD);
// Hooks one COM vtable slot by overwriting its function pointer; the original is
// called through the saved pointer. We use this instead of SafetyHook's inline
// hooks for the WASAPI COM methods because, on x86, MMDevApi/AudioSes prologues
// use dynamic stack alignment (`and esp,-8`) with EBP-relative argument access,
// which SafetyHook's trampoline relocation mishandles: the relocated prologue
// leaves EBP wrong, so the original reads garbage arguments and faults (it froze
// 32-bit FMOD games the instant audio init ran through the hook). Swapping the
// vtable entry leaves the original code untouched, so it runs with a pristine
// stack regardless of prologue shape. Every instance of a COM coclass shares one
// vtable, so a single swap intercepts all of them (the same property the old
// inline approach relied on). See the project's stdcall-x86 note.
class VtableHook
{
public:
bool install(void* com_object, unsigned index, void* detour)
{
if (m_vtable != nullptr)
{
return true; // already installed (shared vtable covers every instance)
}
auto** vtable = *reinterpret_cast<void***>(com_object);
DWORD old_protect = 0;
if (!VirtualProtect(&vtable[index], sizeof(void*), PAGE_READWRITE, &old_protect))
{
return false;
}
m_original = vtable[index];
vtable[index] = detour; // aligned pointer store -> atomic vs. a concurrent caller
VirtualProtect(&vtable[index], sizeof(void*), old_protect, &old_protect);
m_vtable = vtable;
m_index = index;
return true;
}
void remove()
{
if (m_vtable == nullptr)
{
return;
}
DWORD old_protect = 0;
if (VirtualProtect(&m_vtable[m_index], sizeof(void*), PAGE_READWRITE, &old_protect))
{
m_vtable[m_index] = m_original;
VirtualProtect(&m_vtable[m_index], sizeof(void*), old_protect, &old_protect);
}
m_vtable = nullptr;
// Deliberately keep m_original valid: a detour already running on the audio thread
// (it doesn't hold our setup lock) may still call original() after we restore the
// slot. The original function lives in the loaded audio module, so the pointer stays
// valid; nulling it here would race that in-flight detour into a null call (a rapid
// hook/unhook crash the mock-game stress test caught). A re-install re-reads it.
m_index = 0;
}
template <typename Fn> Fn original() const { return reinterpret_cast<Fn>(m_original); }
explicit operator bool() const { return m_vtable != nullptr; }
private:
void** m_vtable = nullptr;
unsigned m_index = 0;
void* m_original = nullptr;
};
// The WASAPI COM methods are hooked by SWAPPING their vtable slot -- coop::hook::VtableHook from the
// shared vtable_hook.hpp -- rather than inline-hooking: on x86 the MMDevApi/AudioSes prologues do
// dynamic stack alignment (`and esp,-8`) with EBP-relative args that SafetyHook's trampoline
// relocation mishandles (it froze 32-bit FMOD games the instant audio init ran through the hook).
// Swapping the slot leaves the original code untouched. See the project's stdcall-x86 note.
// The scalar audio format we forward; resolved from the game's WAVEFORMATEX.
struct CapturedFormat

View File

@@ -5,8 +5,8 @@
// mishandles -> the original runs with garbage args and faults. Swapping the vtable entry leaves
// the original code untouched, so it runs with a pristine stack regardless of prologue shape.
// Every instance of a COM coclass shares one vtable, so a single swap intercepts all of them.
// See the project's stdcall-x86 note. (audio_hook.cpp has its own equivalent; this is the shared
// copy for the input-side COM hooks.)
// See the project's stdcall-x86 note. Shared by the audio render-hook (audio_hook.cpp) and the
// input-side DirectInput hook (mkb_hook.cpp).
#pragma once
#include <windows.h>