From 30d72c5b4e623685928cc7c119f386c25d3d35c7 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sun, 6 Sep 2026 02:55:17 -0400 Subject: [PATCH] [Fix] (Pipe): refuse a stamp of 0 as fresh on both branches - a read before the first fill is Fatal{UnmigratedPipeInput, "@"}, not default storage - MGPipeInputFieldIsFresh (gen_pipe.py gen_filled, emitted into PipeFilled.inc) compared FilledGen == CurrentVerbSerial for a non-sticky field; before the first MGPipeFillForVerb both are 0, so 55 of the 63 fields read as fresh and served their default-constructed storage silently, with no log line and no abort. Only the four raw-pointer O-class accessors tripped, through their null-base checks. - D6 says the serial starts at 1 so that FilledGen == 0 means never filled, and names the window "@"; the predicate now refuses a stamp of 0 before consulting the sticky branch or the serial, so the window is the poison's case as documented. This is the window E's risk table expects the verify lane to find (an init-time read, the first link, anything reached from eglMakeCurrent). - Reproduced with the reviewer's pre-fill program (a live context with line width 7, no fill, gPipeInputs.GetLineWidth()): stored=0, rc=0 before; rc=134 with exactly Fatal{UnmigratedPipeInput, "GetLineWidth@"} after, with and without Features.PipeVerify set first. - The compare-at-read hook arms at the first fill and cannot see this window either; a static_assert next to it pins that a verify build always carries the poison, which is what covers the reads before arming. --- MobileGL/MG_Impl/Pipe/PipeFill.cpp | 11 ++++++++++- MobileGL/MG_Pipe/generated/PipeFilled.inc | 8 ++++++-- scripts/gen_pipe.py | 8 ++++++-- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/MobileGL/MG_Impl/Pipe/PipeFill.cpp b/MobileGL/MG_Impl/Pipe/PipeFill.cpp index 6537838b..e298c8f1 100644 --- a/MobileGL/MG_Impl/Pipe/PipeFill.cpp +++ b/MobileGL/MG_Impl/Pipe/PipeFill.cpp @@ -338,6 +338,13 @@ namespace MobileGL::MG_Pipe { PipeInputs g_snapshot{}; // the second arm PipeInputs g_readScratch{}; // where the compare-at-read re-read lands + // The read hook arms at the first fill (ArmVerify below), so it cannot see a read + // made before that. That window is covered by the poison instead: MGP_INPUT_CHECK + // precedes MGP_INPUT_VERIFY_READ in every accessor and a stamp of 0 is never fresh, + // so such a read is Fatal{UnmigratedPipeInput, "@"} before the hook + // could matter - which holds only while a verify build always carries the poison. + static_assert(MOBILEGL_PIPE_POISON, "the compare-at-read hook relies on the poison for reads before the first fill"); + struct VerifyState { Bool Parsed = false; Bool Enabled = false; @@ -515,7 +522,9 @@ namespace MobileGL::MG_Pipe { #endif #if MOBILEGL_PIPE_POISON MGPipeFilledState& filled = MGPipeFillAccess::Filled(inputs); - // Starts at 1, so FilledGen == 0 means "never filled". + // Starts at 1: FilledGen == 0 is "never filled", and MGPipeInputFieldIsFresh refuses + // it on both branches, so a read before this first bump is + // Fatal{UnmigratedPipeInput, "@"} rather than default storage. ++filled.CurrentVerbSerial; #endif MGPipeFillAccess::SetVerb(inputs, verb); diff --git a/MobileGL/MG_Pipe/generated/PipeFilled.inc b/MobileGL/MG_Pipe/generated/PipeFilled.inc index 91e82514..4c485cfd 100644 --- a/MobileGL/MG_Pipe/generated/PipeFilled.inc +++ b/MobileGL/MG_Pipe/generated/PipeFilled.inc @@ -310,8 +310,12 @@ struct MGPipeFilledState { std::abort(); } +// FilledGen == 0 is "never filled" on BOTH branches: before the first MGPipeFillForVerb the +// serial is 0 as well, and a read in that window is the poison's "@" case +// (P1 brief D6), never a fresh read of default-constructed storage. inline Bool MGPipeInputFieldIsFresh(const MGPipeFilledState& state, MGPipeInputField field) { const SizeT index = static_cast(field); - return kMGPipeInputFieldSticky[index] ? state.FilledGen[index] != 0 - : state.FilledGen[index] == state.CurrentVerbSerial; + const Uint64 gen = state.FilledGen[index]; + if (gen == 0) return false; + return kMGPipeInputFieldSticky[index] || gen == state.CurrentVerbSerial; } diff --git a/scripts/gen_pipe.py b/scripts/gen_pipe.py index a26d1ad9..7ae0fe56 100644 --- a/scripts/gen_pipe.py +++ b/scripts/gen_pipe.py @@ -807,10 +807,14 @@ def gen_filled(accessors, calls, sticky): std::abort(); } +// FilledGen == 0 is "never filled" on BOTH branches: before the first MGPipeFillForVerb the +// serial is 0 as well, and a read in that window is the poison's "@" case +// (P1 brief D6), never a fresh read of default-constructed storage. inline Bool MGPipeInputFieldIsFresh(const MGPipeFilledState& state, MGPipeInputField field) { const SizeT index = static_cast(field); - return kMGPipeInputFieldSticky[index] ? state.FilledGen[index] != 0 - : state.FilledGen[index] == state.CurrentVerbSerial; + const Uint64 gen = state.FilledGen[index]; + if (gen == 0) return false; + return kMGPipeInputFieldSticky[index] || gen == state.CurrentVerbSerial; }""") return "\n".join(out) + "\n"