From 5e9b1cde4cfae711c303bc5d607b338658d73eb9 Mon Sep 17 00:00:00 2001 From: BlackMark Date: Wed, 24 Jun 2026 01:47:19 +0200 Subject: [PATCH] Pin the whole SharedBlock layout with static_asserts (ABI tripwire) The cross-process block's layout is a wire protocol shared by the x64 host and the x86 hook, but only three front offsets were asserted, and hook_selftest's dump_layout merely printed the rest. A field reordered/resized inside HookStatus (which precedes control/video/mkb) would silently shift everything with no compile-time tripwire and, if the developer forgot to bump kProtocolVersion, ship a silent host<->DLL mismatch. - protocol.hpp now static_asserts sizeof(SharedBlock) and every sub-channel offset (status/control/video/mkb) plus each sub-struct size (HookStatus/HookControl/ VideoShare/AudioStreamInfo/HookEntry/MkbRing). protocol.hpp is compiled for both arches, so a cross-bitness divergence fails to compile on the one that disagrees. - hook_selftest's dump_layout now ASSERTS the same numbers instead of only printing, so hook_selftest_x86 confirms the x86 layout at runtime too. Verified: x64 and x86 builds both compile (identical layout) and both selftests pass; sizeof(SharedBlock)=3936 on both. Co-Authored-By: Claude Opus 4.8 --- README.md | 2 -- common/include/coop/protocol.hpp | 23 ++++++++++++++++++----- tests/hook_selftest.cpp | 13 +++++++++++++ 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 8c6bd07..5ede1bc 100644 --- a/README.md +++ b/README.md @@ -114,8 +114,6 @@ From an in-depth review pass. Each item is fixed test-first (a failing test, the as its own commit; "verify" items are confirmed real before any change, and dropped if not. Cross-process / ABI: -- **Tie `kProtocolVersion` to the layout** — `static_assert` `sizeof(SharedBlock)` + the full offset - set, and make `hook_selftest`'s `dump_layout` assert instead of only printing. - **`log_ring` torn-text window** — verify the MPSC overwrite race; fix or bound it. - **`narrow`/`widen` not inverse** — make the audio-override name round-trip lossless (or document). diff --git a/common/include/coop/protocol.hpp b/common/include/coop/protocol.hpp index cf3658b..4b64976 100644 --- a/common/include/coop/protocol.hpp +++ b/common/include/coop/protocol.hpp @@ -270,14 +270,27 @@ static_assert(std::atomic::is_always_lock_free, static_assert(std::atomic::is_always_lock_free, "status counters need a lock-free 64-bit atomic for cross-process use"); -// The x64 host and the x86 hook map this same block, so its layout must be -// byte-identical across bitness. These offsets (verified equal on both arches) -// lock the front of the block -- the seqlock + pad state the input hot path reads; -// a future field reorder that diverges between x86 and x64 fails to compile on the -// arch that disagrees. (Fixed-width POD + no pointers is what keeps it stable.) +// The x64 host and the x86 hook map this same block, so its layout must be byte-identical across +// bitness, and any change is a wire-protocol change. These pin the WHOLE block -- total size, every +// sub-channel offset, and each sub-struct's size -- so an accidental field reorder/resize fails to +// COMPILE (on whichever arch disagrees, since protocol.hpp is built for both) until the numbers here +// AND kProtocolVersion (the runtime handshake in ipc_client) are both revisited. The version gate only +// guards against a *known* change; this is the tripwire for an *unintended* one. (Fixed-width POD + no +// pointers is what keeps it stable.) If one of these fails: confirm the layout change is intended, +// bump kProtocolVersion, and update the number here. +static_assert(sizeof(SharedBlock) == 3936, "SharedBlock size changed -- this is a wire-protocol change"); static_assert(offsetof(SharedBlock, sequence) == 12, "cross-bitness: sequence offset moved"); static_assert(offsetof(SharedBlock, pads) == 16, "cross-bitness: pad-state offset moved"); static_assert(offsetof(SharedBlock, status) == 96, "cross-bitness: status offset moved"); +static_assert(offsetof(SharedBlock, control) == 1816, "cross-bitness: control offset moved"); +static_assert(offsetof(SharedBlock, video) == 1840, "cross-bitness: video offset moved"); +static_assert(offsetof(SharedBlock, mkb) == 1880, "cross-bitness: mkb offset moved"); +static_assert(sizeof(HookStatus) == 1720, "HookStatus size changed -- wire-protocol change"); +static_assert(sizeof(HookControl) == 24, "HookControl size changed -- wire-protocol change"); +static_assert(sizeof(VideoShare) == 40, "VideoShare size changed -- wire-protocol change"); +static_assert(sizeof(AudioStreamInfo) == 32, "AudioStreamInfo size changed -- wire-protocol change"); +static_assert(sizeof(HookEntry) == 56, "HookEntry size changed -- wire-protocol change"); +static_assert(sizeof(MkbRing) == 2056, "MkbRing size changed -- wire-protocol change"); // --- Seqlock helpers ------------------------------------------------------- diff --git a/tests/hook_selftest.cpp b/tests/hook_selftest.cpp index f70a123..841076a 100644 --- a/tests/hook_selftest.cpp +++ b/tests/hook_selftest.cpp @@ -97,6 +97,19 @@ void dump_layout() offsetof(HookStatus, audio_streams), offsetof(HookStatus, hook_entries)); std::printf("LAYOUT VideoShare sizeof=%zu present_calls=%zu HookControl sizeof=%zu\n", sizeof(VideoShare), offsetof(VideoShare, present_calls), sizeof(HookControl)); + std::printf("LAYOUT off mkb=%zu MkbRing sizeof=%zu AudioStreamInfo sizeof=%zu HookEntry sizeof=%zu\n", + offsetof(SharedBlock, mkb), sizeof(MkbRing), sizeof(AudioStreamInfo), sizeof(HookEntry)); + // Assert the layout, not just print it -- and this runs in the x86 build too (hook_selftest_x86), + // so the same numbers the static_asserts pin at compile time are confirmed at runtime on both + // arches. The reference values match the static_asserts in protocol.hpp. + check(sizeof(SharedBlock) == 3936, "layout: sizeof(SharedBlock)"); + check(offsetof(SharedBlock, sequence) == 12 && offsetof(SharedBlock, pads) == 16, "layout: seqlock front"); + check(offsetof(SharedBlock, status) == 96 && offsetof(SharedBlock, control) == 1816, "layout: status/control"); + check(offsetof(SharedBlock, video) == 1840 && offsetof(SharedBlock, mkb) == 1880, "layout: video/mkb"); + check(sizeof(HookStatus) == 1720 && sizeof(HookControl) == 24 && sizeof(VideoShare) == 40, + "layout: sub-struct sizes"); + check(sizeof(AudioStreamInfo) == 32 && sizeof(HookEntry) == 56 && sizeof(MkbRing) == 2056, + "layout: more sub-struct sizes"); } int main()