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()