From b2daab61b76eecda3385c15bfb833418ef702754 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sun, 18 Oct 2026 05:17:54 -0400 Subject: [PATCH] [Refactor, Test] (docs/ci, docs/Disaggregated, MG_Backend/MGPipe, MG_Impl/Pipe, MG_IntegrationTest, MG_IntegrationTest/Harness, MG_IntegrationTest/Scenarios, MG_Pipe, MG_Test/Pipe, MobileGL, scripts/data, tools/trace_replay): delete the MOBILEGL_PIPE_POISON_OMIT knob - the PoisonOmitted and VerifySplitPoisonOmitted entries set the harness marker MGITEST_POISON_OMIT; the re-executed child arms the pair through Harness/PoisonOmissionArm.h - MGPipeSetPoisonOmission, the stamp withhold, the name lookups and the server half (now one atomic word) compile only with MOBILEGL_PIPE_VERIFY - three PipeInputsTest cases withhold the stamp test-side; PoisonOmitKnobArmsTheOmission and BadPoisonOmitKnobIsFatalNamingTheKnob are retired - skip census: the two retired cases leave the unit-strict and unit-verify names --- MobileGL/Config.h | 6 - MobileGL/ConfigLoader.cpp | 1 - MobileGL/MG_Backend/MGPipe/PipeInputs.cpp | 101 ++++++------- MobileGL/MG_Backend/MGPipe/PipeInputs.h | 11 +- MobileGL/MG_Impl/Pipe/PipeFill.cpp | 98 +++++-------- MobileGL/MG_Impl/Pipe/PipeFill.h | 15 +- MobileGL/MG_IntegrationTest/CMakeLists.txt | 23 +-- .../Harness/PoisonOmissionArm.cpp | 39 +++++ .../Harness/PoisonOmissionArm.h | 34 +++++ .../Scenarios/PoisonOmissionScenario.cpp | 133 ++++++++++-------- MobileGL/MG_Pipe/FillPoints.def | 4 +- MobileGL/MG_Pipe/PipeMutation.h | 2 +- MobileGL/MG_Test/Pipe/PipeInputsTest.cpp | 78 +++------- .../design/09-build-switches-counters.md | 2 +- docs/ci/ci-cleanup-log.md | 1 + scripts/data/skip_census_baseline.json | 4 - tools/trace_replay/run_trace_case.cmake | 8 +- 17 files changed, 276 insertions(+), 284 deletions(-) create mode 100644 MobileGL/MG_IntegrationTest/Harness/PoisonOmissionArm.cpp create mode 100644 MobileGL/MG_IntegrationTest/Harness/PoisonOmissionArm.h diff --git a/MobileGL/Config.h b/MobileGL/Config.h index d1a56f64..02e9cead 100644 --- a/MobileGL/Config.h +++ b/MobileGL/Config.h @@ -469,12 +469,6 @@ namespace MobileGL::MG_Config { // negative control, MOBILEGL_PIPE_VERIFY_CORRUPT, has no field here: only a verify // build reads it, when the comparator arms (MG_Impl/Pipe/PipeFill.cpp, ArmVerify). Bool PipeVerifyFatal = true; - // MOBILEGL_PIPE_POISON_OMIT: :; the filler skips the STAMP (not - // the value) of that field for that verb, an omission indistinguishable from a - // forgotten FillPoints.def row, so that verb's read of it is - // Fatal{UnmigratedPipeInput} (negative control B). Unknown name is - // Fatal{PipeVerifyBadKnob}. - String PipePoisonOmit; // MOBILEGL_PIPE_STATS: dump the boundary counters (bytes, calls, roundtrips, // texture pulls, upload shapes, residual-block bytes, index mirror bytes). Bool PipeStats = false; diff --git a/MobileGL/ConfigLoader.cpp b/MobileGL/ConfigLoader.cpp index b1d2925e..f63eed9f 100644 --- a/MobileGL/ConfigLoader.cpp +++ b/MobileGL/ConfigLoader.cpp @@ -452,7 +452,6 @@ namespace MobileGL::MG_ConfigLoader { // Defaults ON: read as a tri-state so only an explicitly falsy value turns it off. features.PipeVerifyFatal = QueryEnvQuirkOverride("MOBILEGL_PIPE_VERIFY_FATAL") != MG_Config::QuirkOverride::ForceOff; - QueryEnvVariable("MOBILEGL_PIPE_POISON_OMIT", features.PipePoisonOmit, ""); features.PipeStats = QueryEnvFlag("MOBILEGL_PIPE_STATS"); features.PipeIndexMirrorMb = QueryEnvUint32("MOBILEGL_PIPE_INDEX_MIRROR_MB", 64, 0, 4096); features.PipeStatsPeriod = QueryEnvUint32("MOBILEGL_PIPE_STATS_PERIOD", 120, 1, 1000000); diff --git a/MobileGL/MG_Backend/MGPipe/PipeInputs.cpp b/MobileGL/MG_Backend/MGPipe/PipeInputs.cpp index 649b48bd..958076f3 100644 --- a/MobileGL/MG_Backend/MGPipe/PipeInputs.cpp +++ b/MobileGL/MG_Backend/MGPipe/PipeInputs.cpp @@ -6,13 +6,14 @@ // SPDX-License-Identifier: LGPL-3.0-only // End of Source File Header -// The backend-side half of the PipeInputs block: the poison Fatal with its verb name, the -// name lookups the runtime knobs need, and - in a verify build - the per-field equality, -// the entry comparator and the corruption injector. Compiled only under MOBILEGL_PIPE_PUSH -// (CMakeLists.txt appends it to SOURCE_FILES there), so the pull build never sees it. Spells -// no MG_State global: everything that reads the live context lives in MG_Impl/Pipe/PipeFill.cpp. +// The backend-side half of the PipeInputs block: the poison Fatal with its verb name and - +// in a verify build - the per-field equality, the entry comparator, the corruption injector, +// the name lookups its knob and the poison omission need, and the omission's server half. +// Spells no MG_State global: everything that reads the live context lives in +// MG_Impl/Pipe/PipeFill.cpp. #include +#include #include #include @@ -29,22 +30,6 @@ namespace MobileGL::MG_Pipe { MGPipeInputPoisonFatal(field, MGPipeVerbName(verb)); } - Optional MGPipeFindInputField(const char* name) { - if (name == nullptr) return std::nullopt; - for (SizeT i = 0; i < kMGPipeInputFieldCount; ++i) { - if (std::strcmp(kMGPipeInputFieldNames[i], name) == 0) return static_cast(i); - } - return std::nullopt; - } - - Optional MGPipeFindVerb(const char* name) { - if (name == nullptr) return std::nullopt; - for (SizeT i = 0; i < kMGPipeVerbCount; ++i) { - if (std::strcmp(kMGPipeVerbNames[i], name) == 0) return static_cast(i); - } - return std::nullopt; - } - // The stamp is the poison's (m_filled, the generations): a build without MOBILEGL_PIPE_POISON // compiles no read check that could ask any of this. Every transport build has the poison. #if MOBILEGL_PIPE_POISON @@ -55,46 +40,24 @@ namespace MobileGL::MG_Pipe { Uint64 g_residualPulls = 0; #if MOBILEGL_PIPE_VERIFY - // ---- MOBILEGL_PIPE_POISON_OMIT, server half ---- + // ---- negative control B, server half (MGPipeServerSetPoisonOmission) ---- // - // The knob withholds the stamp of one (verb, field) pair, a negative control for the + // The omission withholds the stamp of one (verb, field) pair, a negative control for the // poison check. Under a transport the backend reads the server's stamp, which re-stamps // every field the verb's class can answer, so the server must withhold the pair too or - // the control cannot go red. Parsed here because this file is in the server library and - // MG_Impl is not; a malformed value is the client's Fatal{PipeVerifyBadKnob}, and this - // side simply does not arm. Verify builds only, so the shipped stamp stays branch-free. - struct ServerPoisonOmission { - Bool Parsed = false; - String Value; - Bool Armed = false; - MGPipeVerb Verb = MGPipeVerb::kVerbCount; - MGPipeInputField Field = MGPipeInputField::kFieldCount; - }; - ServerPoisonOmission g_serverOmission; + // the control cannot go red. The harness arms it while the apply thread is already + // running, so the pair is ONE atomic word: the verb in the high half, the field in the + // low one, and no verb at all (0xffff) until something is armed. Verify builds only, so + // the shipped stamp stays branch-free. + constexpr Uint32 kNoServerOmission = ~Uint32{0}; + std::atomic g_serverOmission{kNoServerOmission}; - const ServerPoisonOmission& ServerOmission() { - const String& knob = MG_Config::Features.PipePoisonOmit; - if (g_serverOmission.Parsed && knob.size() == g_serverOmission.Value.size() && - (knob.empty() || knob == g_serverOmission.Value)) { - return g_serverOmission; - } - g_serverOmission = ServerPoisonOmission{}; - g_serverOmission.Parsed = true; - g_serverOmission.Value = knob; - const auto colon = knob.find(':'); - if (colon == String::npos || colon == 0 || colon + 1 >= knob.size()) return g_serverOmission; - const auto verb = MGPipeFindVerb(knob.substr(0, colon).c_str()); - const auto field = MGPipeFindInputField(knob.substr(colon + 1).c_str()); - if (!verb || !field) return g_serverOmission; - g_serverOmission.Armed = true; - g_serverOmission.Verb = *verb; - g_serverOmission.Field = *field; - return g_serverOmission; + constexpr Uint32 PackServerOmission(MGPipeVerb verb, MGPipeInputField field) { + return (static_cast(verb) << 16) | static_cast(field); } Bool ServerOmitsStamp(MGPipeVerb verb, MGPipeInputField field) { - const ServerPoisonOmission& omission = ServerOmission(); - return omission.Armed && omission.Verb == verb && omission.Field == field; + return g_serverOmission.load(std::memory_order_acquire) == PackServerOmission(verb, field); } #endif // MOBILEGL_PIPE_VERIFY @@ -453,12 +416,10 @@ namespace MobileGL::MG_Pipe { filled.FilledGen[i] = serial & (Uint64{0} - bit); } #if MOBILEGL_PIPE_VERIFY - // Negative control B's server half (ServerPoisonOmission above): the one withheld pair. + // Negative control B's server half (g_serverOmission above): the one withheld pair. { - const ServerPoisonOmission& omission = ServerOmission(); - if (omission.Armed && omission.Verb == verb) { - filled.FilledGen[static_cast(omission.Field)] = 0; - } + const Uint32 omission = g_serverOmission.load(std::memory_order_acquire); + if ((omission >> 16) == static_cast(verb)) filled.FilledGen[omission & 0xffffu] = 0; } #endif MGPipeStampAccess::SetServerStamped(inputs, true); @@ -475,6 +436,12 @@ namespace MobileGL::MG_Pipe { MGPipeStampAccess::SetServerStamped(gPipeInputs, false); } +#if MOBILEGL_PIPE_VERIFY + void MGPipeServerSetPoisonOmission(MGPipeVerb verb, MGPipeInputField field) { + g_serverOmission.store(PackServerOmission(verb, field), std::memory_order_release); + } +#endif + Uint64 MGPipeResidualPullCount() { return g_residualPulls; } @@ -691,5 +658,21 @@ namespace MobileGL::MG_Pipe { return true; }); } + + Optional MGPipeFindInputField(const char* name) { + if (name == nullptr) return std::nullopt; + for (SizeT i = 0; i < kMGPipeInputFieldCount; ++i) { + if (std::strcmp(kMGPipeInputFieldNames[i], name) == 0) return static_cast(i); + } + return std::nullopt; + } + + Optional MGPipeFindVerb(const char* name) { + if (name == nullptr) return std::nullopt; + for (SizeT i = 0; i < kMGPipeVerbCount; ++i) { + if (std::strcmp(kMGPipeVerbNames[i], name) == 0) return static_cast(i); + } + return std::nullopt; + } #endif // MOBILEGL_PIPE_VERIFY } // namespace MobileGL::MG_Pipe diff --git a/MobileGL/MG_Backend/MGPipe/PipeInputs.h b/MobileGL/MG_Backend/MGPipe/PipeInputs.h index d7c9ef39..7c7d4e34 100644 --- a/MobileGL/MG_Backend/MGPipe/PipeInputs.h +++ b/MobileGL/MG_Backend/MGPipe/PipeInputs.h @@ -46,10 +46,6 @@ namespace MobileGL::MG_Pipe { [[noreturn]] void MGPipeInputPoisonFatalForVerb(MGPipeInputField field, MGPipeVerb verb); // kMGPipeVerbNames[verb], or "" for kVerbCount (no verb has been filled yet). const char* MGPipeVerbName(MGPipeVerb verb); - // Name lookups for the runtime knobs (MOBILEGL_PIPE_VERIFY_CORRUPT names a field, - // MOBILEGL_PIPE_POISON_OMIT a Verb:Field pair). Empty on an unknown name. - Optional MGPipeFindInputField(const char* name); - Optional MGPipeFindVerb(const char* name); // The server-side read check (ARCHITECTURE.md §9.2.1). Inside a server-stamped verb // (PipeInputs::ServerStampedVerb) a stale read is judged by the field's ownership class: @@ -996,6 +992,13 @@ namespace MobileGL::MG_Pipe { // dereferenced, the snapshot is only ever compared). Returns false for a forwarded field, // which has nothing to corrupt. Bool MGPipeApplyVerifyCorruption(PipeInputs& snapshot, MGPipeInputField field); + // PipeInputs.cpp. Name lookups for MOBILEGL_PIPE_VERIFY_CORRUPT (a field) and + // MGPipeSetPoisonOmission (a verb and a field). Empty on an unknown name. + Optional MGPipeFindInputField(const char* name); + Optional MGPipeFindVerb(const char* name); + // PipeInputs.cpp. Negative control B's server half, armed by MGPipeSetPoisonOmission: the + // server's verb stamp withholds the stamp of `field` at `verb` too. + void MGPipeServerSetPoisonOmission(MGPipeVerb verb, MGPipeInputField field); // PipeFill.cpp. Writes the MOBILEGL_PIPE_VERIFY_FATAL=0 summary line ("N divergence(s) // survived") NOW, while the role's log file is still open. MobileGL::Destroy calls it right // before MG_Util::Debug::Close(): the line used to come from a namespace-scope static's diff --git a/MobileGL/MG_Impl/Pipe/PipeFill.cpp b/MobileGL/MG_Impl/Pipe/PipeFill.cpp index 4a5848bd..2fd7d8d3 100644 --- a/MobileGL/MG_Impl/Pipe/PipeFill.cpp +++ b/MobileGL/MG_Impl/Pipe/PipeFill.cpp @@ -8,9 +8,9 @@ // The client side of the PipeInputs block (ARCHITECTURE.md 9.2 phase A): the only place in // the push arm that reads MG_State::pGLContext. Holds the per-verb filler, the F-class -// forwarders, IsLive, the MOBILEGL_PIPE_POISON_OMIT knob and - in a verify build - the -// second arm (SnapshotFromGLContext), the entry compare, the compare-at-read hook and the -// MOBILEGL_PIPE_VERIFY_CORRUPT knob, which only this build reads. +// forwarders, IsLive and - in a verify build - the second arm (SnapshotFromGLContext), the +// entry compare, the compare-at-read hook, the MOBILEGL_PIPE_VERIFY_CORRUPT knob, which only +// this build reads, and the poison omission (MGPipeSetPoisonOmission). #include #include // C-1: the vertex-elements CSO's death path raises the backend notice from here, between the @@ -457,62 +457,40 @@ namespace MobileGL::MG_Pipe { return null; } +#if MOBILEGL_PIPE_VERIFY [[noreturn]] void BadKnob(const char* knob, const char* value, const char* why) { MGLOG_F("MGPipe: Fatal{PipeVerifyBadKnob, \"%s=%s\": %s}", knob, value, why); std::abort(); } - // ---- MOBILEGL_PIPE_POISON_OMIT (negative control B, P1 brief D6) ---- + // ---- negative control B (P1 brief D6), armed by MGPipeSetPoisonOmission ---- // The filler skips the STAMP (never the value) of one (verb, field) pair: an omission // indistinguishable from a forgotten FillPoints.def row, so that verb's read of the - // field is Fatal{UnmigratedPipeInput, "Field@Verb"} and no other verb is affected. + // field is Fatal{UnmigratedPipeInput, "Field@Verb"} and no other verb is affected. The + // GL thread arms it and the GL thread's fill reads it; the server's stamp has its own. struct PoisonOmission { Bool Armed = false; MGPipeVerb Verb = MGPipeVerb::kVerbCount; MGPipeInputField Field = MGPipeInputField::kFieldCount; }; PoisonOmission g_omission; - Bool g_omissionKnobParsed = false; - String g_omissionKnobValue; // the value the last parse saw - // Parsed on the first fill and again only when the value changes. A lane loads - // Features once, before any fill, so that is one parse per process there; a forked - // test child that sets Features after its parent already filled gets its own parse, - // which is what puts the parser and its Fatal{PipeVerifyBadKnob} under a unit test. - // An empty value never clears an omission a test armed through MGPipeSetPoisonOmission. - void ParsePoisonOmissionKnob() { - const String& knob = MG_Config::Features.PipePoisonOmit; - // THE UNSET KNOB IS TWO LENGTH LOADS AND A BRANCH, inline, and that is the whole - // change (P5d r3, package C): this runs at every verb - 852 draws a frame on the - // profiled workload - and `knob == g_omissionKnobValue` is an out-of-line String - // compare even when both sides are empty, which is every shipping configuration. - // Sizes first, contents only when a non-empty value is involved; the re-parse - // condition is unchanged (the value differing from what the last parse latched). - if (g_omissionKnobParsed && knob.size() == g_omissionKnobValue.size() && - (knob.empty() || knob == g_omissionKnobValue)) { - return; + // The stamp the fill's walk is about to overwrite for the omitted pair, put back after the + // branch-free stamp of the whole walk: the value is copied either way, only the stamp is + // withheld. Index kMGPipeInputFieldCount is "nothing withheld at this verb". + struct WithheldStamp { + SizeT Index = kMGPipeInputFieldCount; + Uint64 Gen = 0; + void PutBack(MGPipeFilledState& filled) const { + if (Index < kMGPipeInputFieldCount) filled.FilledGen[Index] = Gen; } - g_omissionKnobParsed = true; - g_omissionKnobValue = knob; - if (knob.empty()) return; - const auto colon = knob.find(':'); - if (colon == String::npos || colon == 0 || colon + 1 >= knob.size()) { - BadKnob("MOBILEGL_PIPE_POISON_OMIT", knob.c_str(), "expected :"); - } - const String verbName = knob.substr(0, colon); - const String fieldName = knob.substr(colon + 1); - const auto verb = MGPipeFindVerb(verbName.c_str()); - if (!verb) BadKnob("MOBILEGL_PIPE_POISON_OMIT", knob.c_str(), "no such verb in kMGPipeVerbNames"); - const auto field = MGPipeFindInputField(fieldName.c_str()); - if (!field) BadKnob("MOBILEGL_PIPE_POISON_OMIT", knob.c_str(), "no such field in kMGPipeInputFieldNames"); - MGPipeSetPoisonOmission(verbName.c_str(), fieldName.c_str()); + }; + WithheldStamp StampToWithhold(const MGPipeFilledState& filled, MGPipeVerb verb) { + if (!g_omission.Armed || g_omission.Verb != verb) return {}; + const auto index = static_cast(g_omission.Field); + return {index, filled.FilledGen[index]}; } - [[maybe_unused]] Bool IsOmitted(MGPipeVerb verb, MGPipeInputField field) { - return g_omission.Armed && g_omission.Verb == verb && g_omission.Field == field; - } - -#if MOBILEGL_PIPE_VERIFY // ---- the MOBILEGL_PIPE_VERIFY comparator (P1 brief D8) ---- // Two mechanisms, both active only when Features.PipeVerify is set: the ENTRY compare // once per verb (the pushed block against a second snapshot of the live context, @@ -1906,25 +1884,17 @@ namespace MobileGL::MG_Pipe { return published; } - void MGPipeSetPoisonOmission(const char* verb, const char* field) { - if (verb == nullptr || field == nullptr) { - g_omission = PoisonOmission{}; - return; - } +#if MOBILEGL_PIPE_VERIFY + Bool MGPipeSetPoisonOmission(const char* verb, const char* field) { const auto v = MGPipeFindVerb(verb); const auto f = MGPipeFindInputField(field); - if (!v || !f) BadKnob("MOBILEGL_PIPE_POISON_OMIT", verb, "unknown verb or field"); - g_omission.Armed = true; - g_omission.Verb = *v; - g_omission.Field = *f; -#if MOBILEGL_PIPE_POISON + if (!v || !f) return false; + g_omission = PoisonOmission{true, *v, *f}; + MGPipeServerSetPoisonOmission(*v, *f); MGLOG_I("MGPipe: poison omission armed - %s@%s", field, verb); -#else - MGLOG_W_ONCE("MGPipe: poison omission %s@%s requested but the poison is not compiled in " - "(MOBILEGL_PIPE_POISON=0): no stamp exists to omit", - field, verb); -#endif + return true; } +#endif // ---- liveness ---- Bool PipeInputs::IsLive() const { @@ -3039,7 +3009,6 @@ namespace MobileGL::MG_Pipe { // every verb) and the guard below then fires Fatal{RoleViolation, "gPipeInputs"} on // the first unbarriered verb, by name. const Bool runAhead = ClientRunsAhead(); - ParsePoisonOmissionKnob(); #if MOBILEGL_PIPE_VERIFY ArmVerify(); #else @@ -3390,18 +3359,19 @@ namespace MobileGL::MG_Pipe { MGPipeFillAccess::CopyField(inputs, *ctx, static_cast(copies[k])); } #if MOBILEGL_PIPE_POISON - // The value is copied either way; only the stamp is withheld for the omitted pair, - // which is put back to what it held after the branch-free stamp of the whole walk. const Uint8* const walk = g_fillPlan.Walk[planClass].data(); const SizeT walkCount = g_fillPlan.WalkCount[planClass]; const Uint64 serial = filled.CurrentVerbSerial; - const Bool omit = g_omission.Armed && g_omission.Verb == verb; - const SizeT omitted = omit ? static_cast(g_omission.Field) : 0; - const Uint64 omittedGen = omit ? filled.FilledGen[omitted] : 0; +#if MOBILEGL_PIPE_VERIFY + // Negative control B: the omitted pair's stamp goes back after the walk (WithheldStamp). + const WithheldStamp withheld = StampToWithhold(filled, verb); +#endif for (SizeT k = 0; k < walkCount; ++k) { filled.FilledGen[walk[k] & ~ResidualFillPlan::kCopy] = serial; } - if (omit) filled.FilledGen[omitted] = omittedGen; +#if MOBILEGL_PIPE_VERIFY + withheld.PutBack(filled); +#endif #endif } if (payloadBytes != 0 && MG_Util::PipeStats::Enabled()) { diff --git a/MobileGL/MG_Impl/Pipe/PipeFill.h b/MobileGL/MG_Impl/Pipe/PipeFill.h index 091aaca9..5176d7db 100644 --- a/MobileGL/MG_Impl/Pipe/PipeFill.h +++ b/MobileGL/MG_Impl/Pipe/PipeFill.h @@ -81,14 +81,6 @@ namespace MobileGL::MG_Pipe { // (TrackerWalk.ABaseInstanceSurvivesTheFirstWalkOnAFreshContext). Uint32 MGPipePendingBaseInstance(); - // PipeFill.cpp. Negative control B (P1 brief D6): the filler withholds the STAMP - never - // the value - of `field` at `verb`, so that verb's read of it is - // Fatal{UnmigratedPipeInput, "Field@Verb"} while every other verb is unaffected. The - // MOBILEGL_PIPE_POISON_OMIT knob (":") calls this once, on the first - // fill; tests call it directly. Both null clears the omission. An unknown name is - // Fatal{PipeVerifyBadKnob}. - void MGPipeSetPoisonOmission(const char* verb, const char* field); - // PipeFill.cpp. How many times set_vertex_attrib_defaults' applier failed to reproduce // the value the call carried, so the client wrote the mirror itself // (EmitVertexAttribDefaults). It is the ONE observable of that repair: the window it @@ -119,6 +111,13 @@ namespace MobileGL::MG_Pipe { // is the branch that survives P13, which is why it is its own function rather than the // filler's loop. void SnapshotFromGLContext(PipeInputs& snapshot, const MGPipeFieldMask& mask); + + // PipeFill.cpp. Negative control B (P1 brief D6): from now on the filler withholds the STAMP - + // never the value - of `field` at `verb`, and so does the server's verb stamp, so that verb's + // read of it is Fatal{UnmigratedPipeInput, "Field@Verb"} while every other verb is unaffected. + // Names from kMGPipeVerbNames / kMGPipeInputFieldNames; false, arming nothing, for an unknown + // one. Call it on the GL thread. The integration harness arms it (Harness/PoisonOmissionArm.h). + Bool MGPipeSetPoisonOmission(const char* verb, const char* field); #endif } // namespace MobileGL::MG_Pipe #define MGP_FILL(Verb) ::MobileGL::MG_Pipe::MGPipeValidateForVerb(::MobileGL::MG_Pipe::MGPipeVerb::Verb) diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index e1fd0061..9d6fe5ff 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -56,6 +56,7 @@ add_executable(MobileGLIntegrationTest Harness/P4aSeamPeek.cpp Harness/P4aFinalFixPeek.cpp Harness/PersistentMapPeek.cpp + Harness/PoisonOmissionArm.cpp Harness/SplitRuntimePeek.cpp # ---- P3b/P4b wave 2-D package D2 (own block; D1 edits this file elsewhere) -------------- # The client texture emitter's own counters, for TextureViewAliasScenario's half of the @@ -1309,7 +1310,9 @@ if (MOBILEGL_PIPE_VERIFY) # Negative control B (G5). The omission skips the STAMP of one field for one verb while still # copying its value, which is indistinguishable from a fill row nobody wrote; the scenario - # forks, so the resulting std::abort() is a datum in waitpid() rather than a dead lane. + # forks, so the resulting std::abort() is a datum in waitpid() rather than a dead lane. The + # pair is a harness marker, MGITEST_POISON_OMIT: the library has no knob for it, and the + # re-executed child arms it through the verify build's setter (Harness/PoisonOmissionArm.h). # P13 W5: the pair is the pack state at ReadPixels, which monolith's record arm still reads from # the fill (around its neutral-pack readback). GetActiveTextureUnit at GenerateMipmap was the # frontend arm's read; the record arm resolves the texture from the verb's own handle and never @@ -1317,13 +1320,13 @@ if (MOBILEGL_PIPE_VERIFY) mgl_itest_join_environment(MGL_ITEST_GLES_POISON_OMIT_ENVIRONMENT "MOBILEGL_TRANSPORT=monolith" "MOBILEGL_BACKEND_TYPE=DirectGLES" "MOBILEGL_PIPE_VERIFY=1" - "MOBILEGL_PIPE_POISON_OMIT=ReadPixels:GetPixelStoreParameters" + "MGITEST_POISON_OMIT=ReadPixels:GetPixelStoreParameters" "MOBILEGL_LOG_FILE_PATH=${CMAKE_CURRENT_BINARY_DIR}/pipe-poison-omit-DirectGLES.log" ${MGL_ITEST_COMMON_ENV}) mgl_itest_join_environment(MGL_ITEST_VULKAN_POISON_OMIT_ENVIRONMENT "MOBILEGL_TRANSPORT=monolith" "MOBILEGL_BACKEND_TYPE=DirectVulkan" "MOBILEGL_PIPE_VERIFY=1" - "MOBILEGL_PIPE_POISON_OMIT=ReadPixels:GetPixelStoreParameters" + "MGITEST_POISON_OMIT=ReadPixels:GetPixelStoreParameters" "MOBILEGL_LOG_FILE_PATH=${CMAKE_CURRENT_BINARY_DIR}/pipe-poison-omit-DirectVulkan.log" ${MGL_ITEST_VULKAN_ENV}) @@ -3663,15 +3666,13 @@ if (MOBILEGL_BUILD_DISAGGREGATED AND MOBILEGL_PIPE_VERIFY) # PoisonOmissionScenario's ReadChildLog reads BOTH role halves of the child's log, because # under a transport the poisoned read can fire on either side of the seam. # - # A DIFFERENT PAIR FROM THE MONOLITH ENTRIES', and the monolith pair is not "also fine - # here": GenerateMipmap:GetActiveTextureUnit is inert on this arm - the split backend's - # generate_mipmap resolves its texture from the verb's handle and never reads the active - # unit, so the child exited 0 with that stamp withheld (measured, both backends). - # ReadPixels:GetPixelStoreParameters is read on the server under the server's stamp, - # which is the stamp the knob has to withhold on this arm (PipeInputs.cpp's - # ServerPoisonOmission). PoisonOmissionScenario.cpp's kOmittedPairs carries both. + # THE MONOLITH ENTRIES' PAIR. ReadPixels:GetPixelStoreParameters is read on the server under + # the server's stamp, which is the stamp the omission has to withhold on this arm + # (PipeInputs.cpp's MGPipeServerSetPoisonOmission). GenerateMipmap:GetActiveTextureUnit is + # inert here: generate_mipmap resolves its texture from the verb's handle and never reads + # the active unit, so the child exited 0 with that stamp withheld (measured, both backends). mgl_itest_lane("${mglItestVerifyBackend}.VerifySplitPoisonOmitted." "PoisonOmissionScenario.OmittedFieldAbortsOnThatVerb" - LABELS "integration-verify-split" TIMEOUT ${MGL_ITEST_VERIFY_TIMEOUT} LOCK "pipe-poison-omit-split-${mglItestVerifyBackend}.log" ENV "${mglItestVerifyArmEnv}\;MOBILEGL_PIPE_POISON_OMIT=ReadPixels:GetPixelStoreParameters\;MOBILEGL_LOG_FILE_PATH=${CMAKE_CURRENT_BINARY_DIR}/pipe-poison-omit-split-${mglItestVerifyBackend}.log") + LABELS "integration-verify-split" TIMEOUT ${MGL_ITEST_VERIFY_TIMEOUT} LOCK "pipe-poison-omit-split-${mglItestVerifyBackend}.log" ENV "${mglItestVerifyArmEnv}\;MGITEST_POISON_OMIT=ReadPixels:GetPixelStoreParameters\;MOBILEGL_LOG_FILE_PATH=${CMAKE_CURRENT_BINARY_DIR}/pipe-poison-omit-split-${mglItestVerifyBackend}.log") # B's AMBIENT HALF. `Verify.PoisonOmissionScenario.WithoutOmissionCompletes` runs the # scenario with nothing withheld, and the mechanism filter took it out of the ambient set # above along with the rest of the scenario. Registered back by hand, with no knob, so diff --git a/MobileGL/MG_IntegrationTest/Harness/PoisonOmissionArm.cpp b/MobileGL/MG_IntegrationTest/Harness/PoisonOmissionArm.cpp new file mode 100644 index 00000000..f8278229 --- /dev/null +++ b/MobileGL/MG_IntegrationTest/Harness/PoisonOmissionArm.cpp @@ -0,0 +1,39 @@ +// MobileGL - MobileGL/MG_IntegrationTest/Harness/PoisonOmissionArm.cpp +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header + +#include "PoisonOmissionArm.h" + +#if !defined(__ANDROID__) +#include +#include +#if MOBILEGL_PIPE_VERIFY +#define MGITEST_POISON_OMISSION_ARM_LIVE 1 +#endif +#endif + +namespace MGITest { + +#if defined(MGITEST_POISON_OMISSION_ARM_LIVE) + bool PoisonOmissionArmAvailable() { + return true; + } + + bool ArmPoisonOmission(const char* verb, const char* field) { + return MobileGL::MG_Pipe::MGPipeSetPoisonOmission(verb, field); + } +#else + bool PoisonOmissionArmAvailable() { + return false; + } + + bool ArmPoisonOmission(const char*, const char*) { + return false; + } +#endif + +} // namespace MGITest diff --git a/MobileGL/MG_IntegrationTest/Harness/PoisonOmissionArm.h b/MobileGL/MG_IntegrationTest/Harness/PoisonOmissionArm.h new file mode 100644 index 00000000..e7b19f8e --- /dev/null +++ b/MobileGL/MG_IntegrationTest/Harness/PoisonOmissionArm.h @@ -0,0 +1,34 @@ +// MobileGL - MobileGL/MG_IntegrationTest/Harness/PoisonOmissionArm.h +// Copyright (c) 2025-2026 MobileGL-Dev +// Licensed under the GNU Lesser General Public License v3.0: +// https://www.gnu.org/licenses/gpl-3.0.txt +// https://www.gnu.org/licenses/lgpl-3.0.txt +// SPDX-License-Identifier: LGPL-3.0-only +// End of Source File Header +// +// NEGATIVE CONTROL B'S OMISSION, ARMED FROM THE HARNESS. PoisonOmissionScenario's re-executed +// child withholds the stamp of one (verb, field) pair before it runs the sequence, so that verb's +// read of the field must be Fatal{UnmigratedPipeInput, "@"}. The library has no knob +// for it: only a -DMOBILEGL_PIPE_VERIFY=ON build compiles the setter (MG_Impl/Pipe/PipeFill.h, +// MGPipeSetPoisonOmission), and this file is the one place that calls it. +// +// A SEPARATE TRANSLATION UNIT for PipeApplyPeek's reason, verbatim: the scenario sources include +// the GL headers with prototypes and the pipe headers are not meant to meet them in one file. +// +// Both entry points return false, touching nothing, where the setter is out of reach: a build +// without MOBILEGL_PIPE_VERIFY, or Android, where this module links the shipping libMobileGL.so +// built -fvisibility=hidden. + +#pragma once + +namespace MGITest { + + // True when this build can arm the omission at all. + bool PoisonOmissionArmAvailable(); + + // Withholds the stamp of `field` at `verb` (kMGPipeVerbNames / kMGPipeInputFieldNames spellings) + // for the rest of this process, in the client's fill and in the server's verb stamp. Call it on + // the thread that issues the GL calls. False when the build cannot, or a name is unknown. + bool ArmPoisonOmission(const char* verb, const char* field); + +} // namespace MGITest diff --git a/MobileGL/MG_IntegrationTest/Scenarios/PoisonOmissionScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/PoisonOmissionScenario.cpp index 14a5d2b8..88a83cf3 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/PoisonOmissionScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/PoisonOmissionScenario.cpp @@ -11,22 +11,26 @@ // The per-verb poison is the half of P1 that makes a forgotten fill row loud instead of silent: the // filler stamps a generation on every field it copies for a verb, and an accessor whose stamp is not // this verb's aborts with Fatal{UnmigratedPipeInput, "@"}. A mechanism that can only be -// observed when someone forgets a row is a mechanism nobody can trust, so MOBILEGL_PIPE_POISON_OMIT -// forges the mistake on purpose: it names one (verb, field) pair whose STAMP the filler skips while -// still copying the value, which is indistinguishable from a row that was never written. +// observed when someone forgets a row is a mechanism nobody can trust, so the control forges the +// mistake on purpose: the harness marker MGITEST_POISON_OMIT names one (verb, field) pair, and the +// child arms it through the verify build's setter (Harness/PoisonOmissionArm.h), so the filler skips +// that pair's STAMP while still copying the value - indistinguishable from a row never written. // // The scenario asserts both halves of "on THAT verb, and only there": // -// OmittedFieldAbortsOnThatVerb - with MOBILEGL_PIPE_POISON_OMIT=GenerateMipmap:GetActiveTextureUnit, -// a draw must still complete (GetActiveTextureUnit is not in kDraw's -// mask, and the draw's own fields are stamped normally) and the -// following glGenerateMipmap must abort naming exactly that pair. -// WithoutOmissionCompletes - the identical sequence with the knob unset runs to completion with -// no Fatal at all. Without this half, "it aborted" would say nothing -// about WHY: a poison that fired on every verb would look just as red. +// OmittedFieldAbortsOnThatVerb - with MGITEST_POISON_OMIT=ReadPixels:GetPixelStoreParameters, the +// draw and the glGenerateMipmap must still complete (the stamp is +// withheld at ReadPixels alone, and their own fields are stamped +// normally) and the following glReadPixels must abort naming +// exactly that pair. +// WithoutOmissionCompletes - the identical sequence with the marker unset runs to completion +// with no Fatal at all. Without this half, "it aborted" would say +// nothing about WHY: a poison that fired on every verb would look +// just as red. // -// The knob is process-wide, so the two cases cannot share a lane: the first runs in the PoisonOmitted. -// entries, the second in the ambient Verify. entries (the first skips when the knob is unset). +// The omission is process-wide, so the two cases cannot share a lane: the first runs in the +// PoisonOmitted. entries, the second in the ambient Verify. entries (the first skips when the marker +// is unset). // // WHY THE SEQUENCE RUNS IN A SEPARATE PROCESS, AND WHY THAT PROCESS IS fork()+execve() AND NOT fork() // ALONE. The poison reports with MGLOG_F and then std::abort(), in the middle of a GL command - so the @@ -56,6 +60,7 @@ #include "../Harness/PipeStatsWindow.h" #include "../Harness/HeadlessGL.h" +#include "../Harness/PoisonOmissionArm.h" #include "../Harness/ScenarioFixture.h" #if !defined(_WIN32) && !defined(__APPLE__) && !defined(__ANDROID__) && __has_include() @@ -81,28 +86,23 @@ extern char** environ; namespace MGITest { namespace { - // What the PoisonOmitted. ctest entries name, which CI requires to pass by name. Each pair is - // spelled here so the assertion below is about the exact string the poison contracts to - // print (ARCHITECTURE.md 9.2: Fatal{UnmigratedPipeInput, "@"}). + // What the PoisonOmitted. and VerifySplitPoisonOmitted. ctest entries put in + // MGITEST_POISON_OMIT, and CI requires both to pass by name. The pair is spelled here so the + // assertion below is about the exact string the poison contracts to print (ARCHITECTURE.md + // 9.2: Fatal{UnmigratedPipeInput, "@"}). // - // TWO PAIRS SINCE P7 WAVE 3, ONE PER ARM, because the first one is structurally inert on - // the split arm. GenerateMipmap:GetActiveTextureUnit is the monolith control: the monolith - // backend resolves the texture to mip through the active unit. Under a transport it does - // not - since P5c hd the server's generate_mipmap resolves the texture from the verb's own - // handle (MGPipeApplier().VerbMipRes) and never reads the active unit - so omitting that - // stamp is omitting a stamp nobody reads, and the child exits 0 (measured on - // integration-verify-split, both backends). ReadPixels:GetPixelStoreParameters is the - // split control: the server's read_pixels backend call DOES read the pack half through the - // accessor, under the server's own stamp, which honours the knob in a verify build - // (MG_Backend/MGPipe/PipeInputs.cpp's ServerPoisonOmission). + // ONE PAIR FOR BOTH ARMS SINCE P13 W5. The record arm resolves glGenerateMipmap's texture + // from the verb's own handle and never reads the active unit, so the old monolith pair, + // GenerateMipmap:GetActiveTextureUnit, withheld a stamp nobody reads. ReadPixels' pack + // state is read on both arms: by the monolith record arm around its neutral-pack readback, + // and by the server's read_pixels under the server's own stamp, which withholds the pair + // too (MG_Backend/MGPipe/PipeInputs.cpp, MGPipeServerSetPoisonOmission). // - // AND EACH PAIR SAYS WHICH ROLE HAS TO FIRE (`FiresOnTheServer`), which is the whole point - // of the split pair and was not checked at all until P7 wave 3's V1 fix round. The reader - // below concatenates the child's two role halves and the assertion searched the union, so - // `VerifySplitPoisonOmitted.` would have been just as green if the stamp the knob withheld - // had been the CLIENT's - which is the case it exists to rule out, because the mechanism - // under test is MG_Backend/MGPipe/PipeInputs.cpp's server-side stamp honouring the knob. - // The monolith pair has one role and one log half, so it keeps the union. + // AND THE PAIR SAYS WHICH ROLE HAS TO FIRE (`FiresOnTheServer`, P7 wave 3's V1 fix round). + // The reader below concatenates the child's two role halves, so `VerifySplitPoisonOmitted.` + // would be just as green if the withheld stamp had been the CLIENT's - the case it exists to + // rule out. A build without MOBILEGL_BUILD_DISAGGREGATED has one role and one log half, so + // there the union is all there is. struct OmittedPair { const char* Verb; const char* Field; @@ -110,7 +110,6 @@ namespace MGITest { bool FiresOnTheServer; // the Fatal must be in the child's SERVER half, not merely present }; constexpr OmittedPair kOmittedPairs[] = { - {"GenerateMipmap", "GetActiveTextureUnit", {"@DrawArrays", "@DrawArrays"}, false}, {"ReadPixels", "GetPixelStoreParameters", {"@DrawArrays", "@GenerateMipmap"}, true}, }; constexpr const char* kFatalPrefix = "Fatal{UnmigratedPipeInput"; @@ -120,6 +119,11 @@ namespace MGITest { constexpr const char* kChildMarker = "MGITEST_POISON_OMISSION_CHILD"; constexpr const char* kWorkerFilter = "--gtest_filter=PoisonOmissionScenario.TheSequenceThePoisonControlsRun"; + // The pair to withhold, as :, set by the PoisonOmitted. ctest entries and + // inherited by the child. A HARNESS marker (hence MGITEST_): the library never reads it, + // and the worker arms the omission itself (Harness/PoisonOmissionArm.h). + constexpr const char* kOmissionMarker = "MGITEST_POISON_OMIT"; + constexpr const char* kVS = R"(#version 330 core in vec2 aPos; void main() { gl_Position = vec4(aPos, 0.0, 1.0); } @@ -265,14 +269,14 @@ void main() { o_color = vec4(0.25, 0.5, 0.75, 1.0); } glFinish(); std::fprintf(stderr, "[itest] poison worker: the draw completed\n"); - // The verb the omission names. Under MOBILEGL_PIPE_POISON_OMIT this must abort. + // A texture verb between the draw and the readback; the omission does not name it. glBindTexture(GL_TEXTURE_2D, texture); glGenerateMipmap(GL_TEXTURE_2D); glFinish(); std::fprintf(stderr, "[itest] poison worker: glGenerateMipmap returned\n"); - // The split arm's verb (kOmittedPairs[1]): a readback, which is a round trip, so - // the server has applied it - and read the pack state - before this returns. + // The verb the omission names (kOmittedPairs): a readback, which is a round trip, + // so the server has applied it - and read the pack state - before this returns. unsigned char centre[4] = {}; glReadPixels(gl.Width() / 2, gl.Height() / 2, 1, 1, GL_RGBA, GL_UNSIGNED_BYTE, centre); std::fprintf(stderr, "[itest] poison worker: glReadPixels returned\n"); @@ -382,6 +386,18 @@ void main() { o_color = vec4(0.25, 0.5, 0.75, 1.0); } } if (!Ready()) return; + // The omission the marker names, armed on this thread - the one that issues the GL calls + // - before the sequence's first verb. Without the marker nothing is armed, which is + // WithoutOmissionCompletes' half. + if (const char* marker = std::getenv(kOmissionMarker); marker != nullptr && *marker != '\0') { + const std::string pair(marker); + const std::string::size_type colon = pair.find(':'); + ASSERT_NE(colon, std::string::npos) << kOmissionMarker << "=" << pair << " is not :"; + ASSERT_TRUE(ArmPoisonOmission(pair.substr(0, colon).c_str(), pair.substr(colon + 1).c_str())) + << "could not arm the omission " << kOmissionMarker << "=" << pair + << ": the build has no setter (-DMOBILEGL_PIPE_VERIFY=ON compiles it) or a name is unknown"; + } + RunSequence(); #if MGITEST_POISON_HAVE_FORK @@ -404,21 +420,22 @@ void main() { o_color = vec4(0.25, 0.5, 0.75, 1.0); } TEST_F(PoisonOmissionScenario, OmittedFieldAbortsOnThatVerb) { if (!Ready()) return; - if (!StringKnobIsSet("MOBILEGL_PIPE_POISON_OMIT")) { - GTEST_SKIP() << "this case is the poison's negative control and needs " - "MOBILEGL_PIPE_POISON_OMIT=: for the whole process, which " - "is what the PoisonOmitted. ctest entries set"; + if (!StringKnobIsSet(kOmissionMarker)) { + GTEST_SKIP() << "this case is the poison's negative control and needs " << kOmissionMarker + << "=:, which is what the PoisonOmitted. ctest entries set"; } - const std::string knob = std::getenv("MOBILEGL_PIPE_POISON_OMIT"); + const std::string marker = std::getenv(kOmissionMarker); const OmittedPair* pair = nullptr; for (const OmittedPair& candidate : kOmittedPairs) { - if (knob == std::string(candidate.Verb) + ":" + candidate.Field) pair = &candidate; + if (marker == std::string(candidate.Verb) + ":" + candidate.Field) pair = &candidate; } if (pair == nullptr) { - GTEST_SKIP() << "MOBILEGL_PIPE_POISON_OMIT is " << knob << ", but this case only knows " - << "how to provoke " << kOmittedPairs[0].Verb << ":" << kOmittedPairs[0].Field << " and " - << kOmittedPairs[1].Verb << ":" << kOmittedPairs[1].Field; + GTEST_SKIP() << kOmissionMarker << " is " << marker << ", but this case only knows how to provoke " + << kOmittedPairs[0].Verb << ":" << kOmittedPairs[0].Field; } + ASSERT_TRUE(PoisonOmissionArmAvailable()) + << kOmissionMarker << " is set, but only a -DMOBILEGL_PIPE_VERIFY=ON build compiles the setter " + << "the child arms the omission with (Harness/PoisonOmissionArm.h), so this control cannot run here"; const std::string expectedPair = std::string(pair->Field) + "@" + pair->Verb; int status = 0; @@ -452,10 +469,10 @@ void main() { o_color = vec4(0.25, 0.5, 0.75, 1.0); } << childLog; #if MOBILEGL_BUILD_DISAGGREGATED // WHICH ROLE, for the split pair (P7 wave 3, V1). The union above answers "did the - // poison fire", which is all the monolith pair can be asked; this pair exists to prove - // that the SERVER'S OWN verb stamp honours MOBILEGL_PIPE_POISON_OMIT - // (MG_Backend/MGPipe/PipeInputs.cpp's ServerPoisonOmission) - the thing that was NOT - // true before wave 3 and made the control silently green. A client-side firing would + // poison fire", which is all a one-role build can be asked; this pair exists to prove + // that the SERVER'S OWN verb stamp honours the omission + // (MG_Backend/MGPipe/PipeInputs.cpp's MGPipeServerSetPoisonOmission) - the thing that was + // NOT true before wave 3 and made the control silently green. A client-side firing would // satisfy the union and prove the opposite of what the entry claims, so the half is // named here. if (pair->FiresOnTheServer) { @@ -481,11 +498,11 @@ void main() { o_color = vec4(0.25, 0.5, 0.75, 1.0); } } // The ambient half of the pair, in the ambient Verify. lanes: the same sequence with the - // knob UNSET must run to completion and log no Fatal at all. The PoisonOmitted. entries are - // the armed half (knob set, the verb must abort), and CI requires those entries by name. + // marker UNSET must run to completion and log no Fatal at all. The PoisonOmitted. entries are + // the armed half (marker set, the verb must abort), and CI requires those entries by name. // - // It does not skip when MOBILEGL_PIPE_POISON_OMIT is set: run by hand with the knob - // exported, it goes red, and the note in its failure says that is the poison firing. + // It does not skip when MGITEST_POISON_OMIT is set: run by hand with the marker exported, + // it goes red, and the note in its failure says that is the poison firing. TEST_F(PoisonOmissionScenario, WithoutOmissionCompletes) { if (!Ready()) return; @@ -493,7 +510,7 @@ void main() { o_color = vec4(0.25, 0.5, 0.75, 1.0); } GTEST_SKIP() << "the poison is only compiled into the push/verify builds; in an ordinary " "build there is nothing for this control to be a control OF"; } - const bool omissionArmed = StringKnobIsSet("MOBILEGL_PIPE_POISON_OMIT"); + const bool omissionArmed = StringKnobIsSet(kOmissionMarker); int status = 0; std::string reason; @@ -501,10 +518,10 @@ void main() { o_color = vec4(0.25, 0.5, 0.75, 1.0); } const std::string childLog = ReadChildLog(); const std::string note = - omissionArmed - ? std::string(" NOTE: MOBILEGL_PIPE_POISON_OMIT is set in this process, so this failure is " - "expected: the poison IS armed, and this entry going red is the proof.") - : std::string(); + omissionArmed ? std::string(" NOTE: ") + kOmissionMarker + + " is set in this process, so this failure is expected: the poison IS armed, and " + "this entry going red is the proof." + : std::string(); ASSERT_TRUE(WIFEXITED(status)) << "with no omission armed, a draw followed by glGenerateMipmap must complete; the child " << DescribeStatus(status) diff --git a/MobileGL/MG_Pipe/FillPoints.def b/MobileGL/MG_Pipe/FillPoints.def index 3e90b9fa..7f233b9b 100644 --- a/MobileGL/MG_Pipe/FillPoints.def +++ b/MobileGL/MG_Pipe/FillPoints.def @@ -10,8 +10,8 @@ // MGP_FILL_VERB_LIST: every GLFunctionsTable function pointer in declaration order, with its // class (gen_pipe.py refuses any other set). MGP_FILL_FIELD_LIST: the fields a class may read; // sticky fields (MGP_COVERAGE_STICKY_LIST) are implicit. Fix Fatal{UnmigratedPipeInput} by -// adding a row, never by making it sticky. A row is removed only on a MOBILEGL_PIPE_POISON_OMIT -// run of the full CTS caselist on both devices; a corpus that never reaches a path proves nothing. +// adding a row, never by making it sticky. A row is removed only after a poison build without it +// ran the full CTS caselist on both devices; a corpus that never reaches a path proves nothing. // gen_pipe.py's block regexes end at a blank line: keep the empty line after each macro. // clang-format off diff --git a/MobileGL/MG_Pipe/PipeMutation.h b/MobileGL/MG_Pipe/PipeMutation.h index f195b1ba..517b7c98 100644 --- a/MobileGL/MG_Pipe/PipeMutation.h +++ b/MobileGL/MG_Pipe/PipeMutation.h @@ -15,7 +15,7 @@ // materialising a queued clear, overriding a sampler's filter) would leave the copy stale, so // the mutator calls MGP_NOTE_MUTATION(Field) where it moves the value. The notice refreshes // that one field when it belongs to the verb in flight. It never touches the poison stamp, so a -// stamp withheld by MOBILEGL_PIPE_POISON_OMIT stays withheld. +// stamp the fill withheld (negative control B) stays withheld. #include namespace MobileGL::MG_Pipe { // MG_Impl/Pipe/PipeFill.cpp (the client side, the only place that may spell pGLContext). diff --git a/MobileGL/MG_Test/Pipe/PipeInputsTest.cpp b/MobileGL/MG_Test/Pipe/PipeInputsTest.cpp index ecaa280b..aa284f54 100644 --- a/MobileGL/MG_Test/Pipe/PipeInputsTest.cpp +++ b/MobileGL/MG_Test/Pipe/PipeInputsTest.cpp @@ -75,10 +75,8 @@ namespace { void SetUp() override { m_previous = MG_State::pGLContext.Take(); MG_State::pGLContext = MakeUnique(); - MGPipeSetPoisonOmission(nullptr, nullptr); } void TearDown() override { - MGPipeSetPoisonOmission(nullptr, nullptr); MG_State::pGLContext = Move(m_previous); } SharedPtr m_previous; @@ -88,11 +86,20 @@ namespace { Bool Fresh(MGPipeInputField field) { return MGPipeInputFieldIsFresh(gPipeInputs->FilledState(), field); } + + // Negative control B's omission, made here rather than by the library: fill `verb`, then put + // the stamp of `field` back to what it held before the fill. The value is copied and only the + // stamp is withheld, which is what a FillPoints.def row nobody wrote leaves behind. + void FillWithholdingTheStamp(MGPipeVerb verb, MGPipeInputField field) { + auto& filled = const_cast(gPipeInputs->FilledState()); + const Uint64 before = filled.FilledGen[static_cast(field)]; + MGPipeValidateForVerb(verb); + filled.FilledGen[static_cast(field)] = before; + } #endif [[maybe_unused]] constexpr const char* kOmittedFatal = "Fatal{UnmigratedPipeInput, \"GetActiveTextureUnit@GenerateMipmap\"}"; - [[maybe_unused]] constexpr const char* kOmissionKnob = "GenerateMipmap:GetActiveTextureUnit"; #if MGTEST_HAVE_FORK struct ChildResult { @@ -139,7 +146,7 @@ namespace { // Negative control B, layer 1 (P1 brief D6 / G5): the omitted (verb, field) pair is the // ONLY thing that goes stale - every other bit of the verb class's mask is fresh, every // field outside it is not, and the verb after it neither heals the field (not in kDraw's -// mask) nor loses one of its own. +// mask) nor loses one of its own. The omission is the test's own (FillWithholdingTheStamp). TEST_F(PipeInputsTest, OmittingOneFieldForOneVerbLeavesExactlyThatFieldStale) { #if !MOBILEGL_PIPE_POISON GTEST_SKIP() << "poison not compiled in (MOBILEGL_PIPE_POISON=0)"; @@ -148,8 +155,7 @@ TEST_F(PipeInputsTest, OmittingOneFieldForOneVerbLeavesExactlyThatFieldStale) { EXPECT_TRUE(Fresh(MGPipeInputField::GetActiveTextureUnit)); EXPECT_TRUE(Fresh(MGPipeInputField::GetTextureUnitObject)); - MGPipeSetPoisonOmission("GenerateMipmap", "GetActiveTextureUnit"); - MGPipeValidateForVerb(MGPipeVerb::GenerateMipmap); + FillWithholdingTheStamp(MGPipeVerb::GenerateMipmap, MGPipeInputField::GetActiveTextureUnit); EXPECT_TRUE(Fresh(MGPipeInputField::GetTextureUnitObject)); EXPECT_FALSE(Fresh(MGPipeInputField::GetActiveTextureUnit)); // The value was still copied: only the stamp is withheld. @@ -173,15 +179,15 @@ TEST_F(PipeInputsTest, OmittingOneFieldForOneVerbLeavesExactlyThatFieldStale) { // A sticky field stays fresh across every verb. EXPECT_TRUE(Fresh(MGPipeInputField::RecordError)); - // And the omission is scoped to its verb: a different verb of the same class keeps it. + // And the omission is scoped to its fill: the next verb of the same class stamps it again. MGPipeValidateForVerb(MGPipeVerb::BindImageTexture); EXPECT_TRUE(Fresh(MGPipeInputField::GetActiveTextureUnit)); #endif } // Negative control B, layer 2 (G5): the read itself. The child fills GenerateMipmap with the -// omission and reads gPipeInputs->GetActiveTextureUnit(); the parent expects SIGABRT and the -// exact Fatal line, and that nothing else was fatal. +// stamp of GetActiveTextureUnit withheld and reads gPipeInputs->GetActiveTextureUnit(); the +// parent expects SIGABRT and the exact Fatal line, and that nothing else was fatal. TEST_F(PipeInputsTest, ReadingAnOmittedFieldAbortsNamingTheVerb) { #if !MOBILEGL_PIPE_POISON GTEST_SKIP() << "poison not compiled in (MOBILEGL_PIPE_POISON=0)"; @@ -190,10 +196,9 @@ TEST_F(PipeInputsTest, ReadingAnOmittedFieldAbortsNamingTheVerb) { #else ASSERT_FALSE(g_logPath.empty()) << "main() did not set MOBILEGL_LOG_FILE_PATH"; const ChildResult r = RunInChild([] { - MGPipeSetPoisonOmission("GenerateMipmap", "GetActiveTextureUnit"); MGPipeValidateForVerb(MGPipeVerb::DrawArrays); (void)gPipeInputs->GetRenderStateParameters(); // a filled field of the preceding draw: must not abort - MGPipeValidateForVerb(MGPipeVerb::GenerateMipmap); + FillWithholdingTheStamp(MGPipeVerb::GenerateMipmap, MGPipeInputField::GetActiveTextureUnit); (void)gPipeInputs->GetTextureUnitObject(0); // the sibling field: filled, must not abort (void)gPipeInputs->GetActiveTextureUnit(); // the omitted field: Fatal ::_exit(3); // reached only if the poison failed @@ -253,53 +258,6 @@ TEST_F(PipeInputsTest, ReadingBeforeAnyFillAbortsNamingNoVerb) { #endif } -// The MOBILEGL_PIPE_POISON_OMIT parser, through the Features field the loader fills: the -// child sets the knob after its parent filled without it, and the first fill after that -// parses it, logs the arming line and withholds the stamp exactly as the programmatic form. -TEST_F(PipeInputsTest, PoisonOmitKnobArmsTheOmission) { -#if !MOBILEGL_PIPE_POISON - GTEST_SKIP() << "poison not compiled in (MOBILEGL_PIPE_POISON=0)"; -#elif !MGTEST_HAVE_FORK - GTEST_SKIP() << "no fork() on this platform"; -#else - ASSERT_FALSE(g_logPath.empty()) << "main() did not set MOBILEGL_LOG_FILE_PATH"; - MGPipeValidateForVerb(MGPipeVerb::DrawArrays); // the parent's parse saw an empty knob - const ChildResult r = RunInChild([] { - MG_Config::Features.PipePoisonOmit = kOmissionKnob; - MGPipeValidateForVerb(MGPipeVerb::GenerateMipmap); - (void)gPipeInputs->GetTextureUnitObject(0); // the sibling field: filled, must not abort - (void)gPipeInputs->GetActiveTextureUnit(); // the omitted field: Fatal - ::_exit(3); - }); - ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log; - EXPECT_NE(r.Log.find("MGPipe: poison omission armed - GetActiveTextureUnit@GenerateMipmap"), std::string::npos) - << r.Log; - EXPECT_NE(r.Log.find(kOmittedFatal), std::string::npos) << r.Log; - EXPECT_EQ(r.Log.find("GetTextureUnitObject@"), std::string::npos) << r.Log; -#endif -} - -// An unknown verb in the knob is Fatal{PipeVerifyBadKnob} naming the knob and the value, -// before any stamp is withheld. -TEST_F(PipeInputsTest, BadPoisonOmitKnobIsFatalNamingTheKnob) { -#if !MGTEST_HAVE_FORK - GTEST_SKIP() << "no fork() on this platform"; -#else - ASSERT_FALSE(g_logPath.empty()) << "main() did not set MOBILEGL_LOG_FILE_PATH"; - const ChildResult r = RunInChild([] { - MG_Config::Features.PipePoisonOmit = "NoSuchVerb:GetActiveTextureUnit"; - MGPipeValidateForVerb(MGPipeVerb::GenerateMipmap); - ::_exit(3); - }); - ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log; - EXPECT_NE(r.Log.find("Fatal{PipeVerifyBadKnob, \"MOBILEGL_PIPE_POISON_OMIT=NoSuchVerb:GetActiveTextureUnit\": " - "no such verb in kMGPipeVerbNames}"), - std::string::npos) - << r.Log; - EXPECT_EQ(r.Log.find("poison omission armed"), std::string::npos) << r.Log; -#endif -} - // Negative control A at the block level (G4): a clean snapshot compares equal to the pushed // block over the verb's mask; corrupting one field of the snapshot makes the compare name // exactly that field, at the serial of the fill it belongs to. @@ -566,8 +524,7 @@ TEST_F(PipeInputsTest, TheMutationNoticeRefreshesTheValueButNotTheStamp) { GTEST_SKIP() << "poison not compiled in (MOBILEGL_PIPE_POISON=0)"; #else auto& ctx = *MG_State::pGLContext; - MGPipeSetPoisonOmission("DrawArrays", "GetSamplingResolutionGeneration"); - MGPipeValidateForVerb(MGPipeVerb::DrawArrays); + FillWithholdingTheStamp(MGPipeVerb::DrawArrays, MGPipeInputField::GetSamplingResolutionGeneration); ASSERT_FALSE(Fresh(MGPipeInputField::GetSamplingResolutionGeneration)); ctx.BumpSamplingResolutionGeneration(); EXPECT_FALSE(Fresh(MGPipeInputField::GetSamplingResolutionGeneration)) @@ -575,7 +532,6 @@ TEST_F(PipeInputsTest, TheMutationNoticeRefreshesTheValueButNotTheStamp) { // FenceSync is a kQuery verb: its mask holds no texture field at all, so the notice must // leave the generation unfilled and a read of it Fatal{UnmigratedPipeInput}. - MGPipeSetPoisonOmission(nullptr, nullptr); MGPipeValidateForVerb(MGPipeVerb::FenceSync); ASSERT_FALSE(Fresh(MGPipeInputField::GetSamplingResolutionGeneration)); ctx.BumpSamplingResolutionGeneration(); diff --git a/docs/Disaggregated/design/09-build-switches-counters.md b/docs/Disaggregated/design/09-build-switches-counters.md index 2faed9b0..f0092259 100644 --- a/docs/Disaggregated/design/09-build-switches-counters.md +++ b/docs/Disaggregated/design/09-build-switches-counters.md @@ -47,7 +47,7 @@ CMake：`MOBILEGL_BUILD_DISAGGREGATED`（OFF）、`MOBILEGL_BUILD_DISAGGREGATED_ | 变量 | 默认 | 说明 | |---|---|---| | `MOBILEGL_PIPE_PUSH` | `0x3fff` | 子系统位图（`MG_Pipe/MGPipe.h`，位永不复用）；P13 起位 0–13 固定开，清任何一位都在启动时具名拒绝；只剩位 63（"关 CSO 内容寻址"的行为对照）可设 | -| `MOBILEGL_PIPE_VERIFY` / `_VERIFY_FATAL` / `_VERIFY_CORRUPT` / `_POISON_OMIT` | 0 / 1 / 空 / 空 | 影子比对与两个阴性对照。比对器只编进 verify 构建，其他构建里 `MOBILEGL_PIPE_VERIFY=1` 只告警；`_VERIFY_CORRUPT` 不进 `Config.h`，只有 verify 构建在比对器武装时读它（`PipeFill.cpp` 的 `ArmVerify`，同样经环境与系统配置文件） | +| `MOBILEGL_PIPE_VERIFY` / `_VERIFY_FATAL` / `_VERIFY_CORRUPT` | 0 / 1 / 空 | 影子比对与它的阴性对照。比对器只编进 verify 构建，其他构建里 `MOBILEGL_PIPE_VERIFY=1` 只告警；`_VERIFY_CORRUPT` 不进 `Config.h`，只有 verify 构建在比对器武装时读它（`PipeFill.cpp` 的 `ArmVerify`，同样经环境与系统配置文件）。poison 的阴性对照没有运行时开关：集成测试的 harness 标记 `MGITEST_POISON_OMIT` 由重新 exec 的子进程经 verify 构建才有的 `MGPipeSetPoisonOmission` 武装（`Harness/PoisonOmissionArm.h`） | | `MOBILEGL_PIPE_STATS` / `_STATS_PERIOD` / `_STATS_FILE` | 0 / 120 / 空 | 边界计数器（附 B）；dump 按角色写 `.client.json` / `.server.json` | | `MOBILEGL_PIPE_INDEX_MIRROR_MB` | 64 | 索引镜像预算（P8）；`_LEGACY_MEMOS`、`_TEXEL_RETAIN_MB`、`_HANDLE_ABA_CONTROL` 随各自的臂在 P13 删除 | diff --git a/docs/ci/ci-cleanup-log.md b/docs/ci/ci-cleanup-log.md index 96bf6daf..53ec3c5d 100644 --- a/docs/ci/ci-cleanup-log.md +++ b/docs/ci/ci-cleanup-log.md @@ -62,6 +62,7 @@ them wrong. | 2026-10-18 | `Negative control A (corrupted snapshot)` | Test / `integration-verify` | it re-ran the five `DirectGLES.Verify.ClearThenReadPixelsScenario` entries with `MOBILEGL_PIPE_VERIFY_CORRUPT=GetRenderStateParameters` exported job-wide and accepted any red | the in-lane `DirectGLES/DirectVulkan.VerifyCorrupted.PipeVerifyArmingScenario.CorruptedFieldIsReported` entries, required by name (`Verify lane control entries`), which require `Fatal{PipeVerifyDiffer, "GetRenderStateParameters@...` in the log; the abort with `MOBILEGL_PIPE_VERIFY_FATAL` at its default is unit `PipeInputsTest.VerifyCorruptKnobNamesTheFieldAtEntry` on the verify runtime | | 2026-10-18 | `Negative control A2 (corrupted capability)` | Test / `integration-verify` | the same five-entry rerun with `MOBILEGL_PIPE_VERIFY_CORRUPT=IsCapabilityEnabled`, accepting any red; it proved that the comparator compares the capability field (P13 W3c moved the residual block's cross-check there) | unit `PipeInputsTest.EveryCorruptedFieldIsNamedByTheComparator` (`Unit tests on the verify runtime`): every field with storage, the capability field among them, corrupted in turn must be the field the comparator names | | 2026-10-18 | `Negative control A (corrupted snapshot)` and `Keep lane logs before controls` | Test / `integration-verify-split` | the first re-ran the ten `VerifySplit.ClearThenReadPixelsScenario` entries of both backends with the same knob and accepted any red; the second only kept the lane's logs from being overwritten by that rerun | the in-lane `VerifySplitCorrupted` and `VerifySplitReadCorrupted` entries of both backends, required by name (`Verify-split control entries`) | +| 2026-10-18 | unit `PipeInputsTest.{PoisonOmitKnobArmsTheOmission, BadPoisonOmitKnobIsFatalNamingTheKnob}`, and the `MOBILEGL_PIPE_POISON_OMIT` knob: `Features.PipePoisonOmit`, its parser on every verb (`MGPipeValidateForVerb`'s `ParsePoisonOmissionKnob`, compiled into every build) and the server stamp's own parse of it | Test / unit (every poison lane); product code | the knob existed only for negative control B, and the two cases pinned its parser and its `Fatal{PipeVerifyBadKnob}` | the PoisonOmitted and VerifySplitPoisonOmitted entries set the harness marker `MGITEST_POISON_OMIT` to the same pair, and the re-executed child arms it through `MGPipeSetPoisonOmission`, compiled only with `-DMOBILEGL_PIPE_VERIFY=ON` (`Harness/PoisonOmissionArm.h`); `OmittingOneFieldForOneVerbLeavesExactlyThatFieldStale`, `ReadingAnOmittedFieldAbortsNamingTheVerb` and `TheMutationNoticeRefreshesTheValueButNotTheStamp` withhold the stamp test-side and still run in every poison lane | ## Kept after investigation diff --git a/scripts/data/skip_census_baseline.json b/scripts/data/skip_census_baseline.json index ac8bddb7..e8206326 100644 --- a/scripts/data/skip_census_baseline.json +++ b/scripts/data/skip_census_baseline.json @@ -10503,14 +10503,12 @@ "PipeCatalogue.WireOpcodesAreThePositionsInTheCatalogue", "PipeInputsTest.AFrontendMutationInsideAVerbDoesNotDivergeAtRead", "PipeInputsTest.AFrontendMutationInsideAVerbRefreshesThePushedField", - "PipeInputsTest.BadPoisonOmitKnobIsFatalNamingTheKnob", "PipeInputsTest.BadVerifyCorruptKnobIsFatalNamingTheKnob", "PipeInputsTest.CorruptedSnapshotFieldIsNamedWithItsSerial", "PipeInputsTest.EveryCorruptedFieldIsNamedByTheComparator", "PipeInputsTest.EveryVerbFillsItsClassAndNothingElse", "PipeInputsTest.MutatedFieldIsNamedAtRead", "PipeInputsTest.OmittingOneFieldForOneVerbLeavesExactlyThatFieldStale", - "PipeInputsTest.PoisonOmitKnobArmsTheOmission", "PipeInputsTest.ReadingAFilledFieldCompletes", "PipeInputsTest.ReadingAnOmittedFieldAbortsNamingTheVerb", "PipeInputsTest.ReadingBeforeAnyFillAbortsNamingNoVerb", @@ -16801,14 +16799,12 @@ "PipeCatalogue.WireOpcodesAreThePositionsInTheCatalogue", "PipeInputsTest.AFrontendMutationInsideAVerbDoesNotDivergeAtRead", "PipeInputsTest.AFrontendMutationInsideAVerbRefreshesThePushedField", - "PipeInputsTest.BadPoisonOmitKnobIsFatalNamingTheKnob", "PipeInputsTest.BadVerifyCorruptKnobIsFatalNamingTheKnob", "PipeInputsTest.CorruptedSnapshotFieldIsNamedWithItsSerial", "PipeInputsTest.EveryCorruptedFieldIsNamedByTheComparator", "PipeInputsTest.EveryVerbFillsItsClassAndNothingElse", "PipeInputsTest.MutatedFieldIsNamedAtRead", "PipeInputsTest.OmittingOneFieldForOneVerbLeavesExactlyThatFieldStale", - "PipeInputsTest.PoisonOmitKnobArmsTheOmission", "PipeInputsTest.ReadingAFilledFieldCompletes", "PipeInputsTest.ReadingAnOmittedFieldAbortsNamingTheVerb", "PipeInputsTest.ReadingBeforeAnyFillAbortsNamingNoVerb", diff --git a/tools/trace_replay/run_trace_case.cmake b/tools/trace_replay/run_trace_case.cmake index 8a999759..cc83f782 100644 --- a/tools/trace_replay/run_trace_case.cmake +++ b/tools/trace_replay/run_trace_case.cmake @@ -237,8 +237,8 @@ endif() # * it carries none of the three MGPipe Fatals: Fatal{PipeVerifyDiffer (a push/pull divergence, # the thing the mode exists to find), Fatal{UnmigratedPipeInput (a backend read of a field the # verb's fill table does not list - fixed by adding the row to MG_Pipe/FillPoints.def, never by -# marking the field sticky), and Fatal{PipeVerifyBadKnob (a misspelt MOBILEGL_PIPE_VERIFY_CORRUPT -# or MOBILEGL_PIPE_POISON_OMIT). The third is in the regex on purpose even though D2 makes it +# marking the field sticky), and Fatal{PipeVerifyBadKnob (a misspelt MOBILEGL_PIPE_VERIFY_CORRUPT). +# The third is in the regex on purpose even though D2 makes it # abort the process: with MOBILEGL_PIPE_VERIFY_FATAL=0 the abort is exactly what does not # happen, and a typo'd knob would otherwise leave the negative-control lane looking healthy. # The Fatal check is not redundant with the replay's exit status: MOBILEGL_PIPE_VERIFY_FATAL=0 is @@ -286,8 +286,8 @@ if(DEFINED ENV{MOBILEGL_PIPE_VERIFY} AND NOT "$ENV{MOBILEGL_PIPE_VERIFY}" STREQU "divergence and is recorded, not silenced; Fatal{UnmigratedPipeInput, \"@\"} " "is a missing row in MG_Pipe/FillPoints.def's class table - add it, regenerate, rerun " "(never mark the field sticky); Fatal{PipeVerifyBadKnob, ...} is a misspelt " - "MOBILEGL_PIPE_VERIFY_CORRUPT / MOBILEGL_PIPE_POISON_OMIT - fix the spelling, the " - "vocabularies are kMGPipeInputFieldNames[] and kMGPipeVerbNames[].") + "MOBILEGL_PIPE_VERIFY_CORRUPT - fix the spelling, the vocabulary is " + "kMGPipeInputFieldNames[].") endif() message(STATUS "MGPipe verify: ${pipe_verify_case} armed, zero divergences, zero unmigrated reads") endif()