From a0836568ac78bbcffb4a46866fbb63b5107c6c22 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 18 Sep 2026 12:19:14 -0400 Subject: [PATCH] [MG_Pipe, MG_Remote, MG_Backend, MG_Test] (Disaggregated): P5e ID-128 - admission gains the escalation disjunct - The integrator's census over the merged tree left two markers nothing admitted, both DirectGLES.Split.ClientVertexArrayScenario: GetBoundVertexArray@DrawArrays. Disjunct 1 fails (draw_vbo is kWaitNone) and disjunct 2 fails (that row's retiring phase names P5e, correctly, for the ORDINARY draw path vi and mv retired). But these two entries pull the pair on a record the client IS parked behind: MGPipeBarriered escalates a draw carrying client vertex arrays (ID-83), ID-82 puts client arrays outside P5e entirely, and the read site (SyncClientSideVertexArraysForDrawArrays) aborts by its own name if it is ever applied unbarriered. The pull is legal; no static table could see it. - Third disjunct: the record was barriered BY ESCALATION, i.e. wireSaysBarriered(op, payload, applierState) && MGPipeWaitClassFor(op) == kWaitNone. Both halves were already computed on every record - ApplyOne computes the predicate unconditionally for ID-103's reason - so this is one more bit stamped beside the barriered stamp, not new plumbing. - It is a SEPARATE flag rather than a refinement of the barriered stamp, because the two answer different questions: the stamp answers "is the client parked" (the allocator and registry guards ask that), this answers "and for a reason the static table cannot see" (only the strict knob asks). Its default is false - the answer that admits nothing - which is the opposite of the stamp's default and deliberately so. - The marker says WHICH disjunct: ADMITTED when the generated table answered, ADMITTED-ESCALATED when only the record did. That keeps the lane's comparison against --print-admitted exact instead of widening the static list with every pair that could ever escalate - and the tag sits in the slot the grammar already has for the reason, so Fatal{UnmigratedPipeInput filters are untouched. - Escalation also happens to be the truer justification for GetTransformFeedbackProgram@DrawArrays, which ID-125 admits by phase. Both now hold; ID-125's disjunct is kept, as ruled. - Red-once: AnEscalatedRecordsPullIsAdmittedAndSaysWhy. Dropping `&& !escalated` from the strict arm kills it - "FieldOwnershipTest.cpp:938: Failure / Actual: false / signal 6" with 'Fatal{UnmigratedPipeInput, "GetBoundVertexArray@DrawArrays"} [BARRIER-PULLED, MOBILEGL_IPC_STRICT_ERRORS=1, retires in P5e (Espryt unbarriered), P7 (Magma)]'. Its control is StrictErrorsTurnsABarrierPulledReadIntoANamedAbort, which drives the identical pull with the flag at its default and must keep aborting. --- MobileGL/MG_Backend/MGPipe/PipeInputs.cpp | 30 ++++++++++-- MobileGL/MG_Pipe/PipeApply.cpp | 13 ++++++ MobileGL/MG_Pipe/PipeApply.h | 26 +++++++++++ MobileGL/MG_Remote/Server/PipeApplier.cpp | 11 +++++ MobileGL/MG_Test/Pipe/FieldOwnershipTest.cpp | 48 ++++++++++++++++++++ 5 files changed, 124 insertions(+), 4 deletions(-) diff --git a/MobileGL/MG_Backend/MGPipe/PipeInputs.cpp b/MobileGL/MG_Backend/MGPipe/PipeInputs.cpp index ef1136e3..5405546f 100644 --- a/MobileGL/MG_Backend/MGPipe/PipeInputs.cpp +++ b/MobileGL/MG_Backend/MGPipe/PipeInputs.cpp @@ -93,7 +93,7 @@ namespace MobileGL::MG_Pipe { // which happens on the apply thread inside PipeApplier::ApplyOne, and g_residualPulls // beside it is a plain Uint64 for the same reason. A racing writer here would at worst // log a duplicate line, never lose one. - void AdmittedBarrierPullOnce(MGPipeInputField field, MGPipeVerb verb) { + void AdmittedBarrierPullOnce(MGPipeInputField field, MGPipeVerb verb, Bool escalated) { const SizeT fieldIndex = static_cast(field); const SizeT verbIndex = static_cast(verb); if (fieldIndex >= kMGPipeInputFieldCount || verbIndex >= kMGPipeVerbCount) return; @@ -102,9 +102,17 @@ namespace MobileGL::MG_Pipe { const Uint64 mask = Uint64{1} << (bit % 64); if ((seen[bit / 64] & mask) != 0) return; seen[bit / 64] |= mask; - MGLOG_W("MGPipe: Admitted{UnmigratedPipeInput, \"%s@%s\"} [BARRIER-PULLED, ADMITTED, " + // P5e (gl), ID-128: WHICH DISJUNCT ADMITTED IT, in the slot the grammar already has + // for the reason. `ADMITTED` means the generated table said so, and the lane can + // check that against `--print-admitted`. `ADMITTED-ESCALATED` means the table did + // NOT and the record was barriered by an escalation the table cannot see, which is a + // RUNTIME fact about that record's payload - so the lane must not look for it in a + // static list. Saying which is what keeps the lane's comparison exact instead of + // widening the list with every pair that could ever escalate. + MGLOG_W("MGPipe: Admitted{UnmigratedPipeInput, \"%s@%s\"} [BARRIER-PULLED, %s, " "retires in %s]", kMGPipeInputFieldNames[fieldIndex], MGPipeVerbName(verb), + escalated ? "ADMITTED-ESCALATED" : "ADMITTED", kMGPipeFieldRetiringPhase[fieldIndex]); } @@ -142,6 +150,9 @@ namespace MobileGL::MG_Pipe { // barriered, unadmitted -> Fatal under strict. A debt no phase has taken. // barriered, admitted -> ONE MGLOG_W per (field, verb), and the entry completes. // + // "Admitted" has three disjuncts and the third is a runtime one (ID-128): the generated + // table answers the first two, and the record's own escalation flag answers the third. + // // An admitted pull is a debt this phase deliberately leaves standing: the field's row is // BARRIER_PULLED, the verb's op is statically barriered so the client really is parked // behind the record, and the field is inside the verb's own may-read mask - so the value @@ -157,10 +168,21 @@ namespace MobileGL::MG_Pipe { MG_Util::PipeStats::AddCalls(MG_Util::PipeStats::CallClass::ResidualPulls, 1); } if (MG_Config::Ipc.StrictErrors) { - if (!MGPipeBarrierPullAdmitted(field, verb)) { + // THE THIRD DISJUNCT (P5e gl, ID-128), and it has to be asked at RUNTIME because + // it is a fact about this record's payload rather than about its opcode. The + // static table admits a pair when the verb's op waits (disjunct 1) or when the + // field's retiring phase is not this phase (disjunct 2); an escalated record is + // one the client parks behind for a reason only the payload knows - an open + // transform-feedback span, or a draw carrying client vertex arrays. Both are + // outside this phase by ruling (§5.7, ID-82), and the client-array read site + // aborts by its own name if it is ever applied unbarriered, so the pull is legal + // and P5e is not the phase that owes it. + const Bool statically = MGPipeBarrierPullAdmitted(field, verb); + const Bool escalated = MGPipeApplierCurrentRecordIsBarrieredByEscalation(); + if (!statically && !escalated) { StrictBarrierPullFatal(field, verb, "MOBILEGL_IPC_STRICT_ERRORS=1"); } - AdmittedBarrierPullOnce(field, verb); + AdmittedBarrierPullOnce(field, verb, !statically); } } diff --git a/MobileGL/MG_Pipe/PipeApply.cpp b/MobileGL/MG_Pipe/PipeApply.cpp index fb42b14f..2fca9c03 100644 --- a/MobileGL/MG_Pipe/PipeApply.cpp +++ b/MobileGL/MG_Pipe/PipeApply.cpp @@ -3031,6 +3031,11 @@ namespace MobileGL::MG_Pipe { // MGPipeBarriered in the sink's ApplyOne. namespace { thread_local Bool g_currentRecordBarriered = true; + // P5e (gl), ID-128. FALSE IS THE DEFAULT AND IT IS THE STRICT ANSWER: "this record was + // barriered by escalation" admits a pull that neither the wait class nor the retiring + // phase admits, so a reader that runs before any writer must say no. The stamp above + // defaults the other way because ITS safe answer is "the client is parked". + thread_local Bool g_currentRecordBarrieredByEscalation = false; } // namespace Bool MGPipeApplierCurrentRecordIsBarriered() { return g_currentRecordBarriered; } @@ -3039,6 +3044,14 @@ namespace MobileGL::MG_Pipe { g_currentRecordBarriered = barriered; } + Bool MGPipeApplierCurrentRecordIsBarrieredByEscalation() { + return g_currentRecordBarrieredByEscalation; + } + + void MGPipeApplierSetCurrentRecordBarrieredByEscalation(Bool escalated) { + g_currentRecordBarrieredByEscalation = escalated; + } + // ================================================================================ // w3: the shader CSO, the two program bindings and the default uniform block // diff --git a/MobileGL/MG_Pipe/PipeApply.h b/MobileGL/MG_Pipe/PipeApply.h index 8e47e222..a791c97b 100644 --- a/MobileGL/MG_Pipe/PipeApply.h +++ b/MobileGL/MG_Pipe/PipeApply.h @@ -948,6 +948,32 @@ namespace MobileGL::MG_Pipe { // the call site (it is the line ra rebases onto); c0e owns the storage so both compile. void MGPipeApplierSetCurrentRecordBarriered(Bool barriered); + // ---- P5e (gl), ID-128: WAS THIS RECORD BARRIERED BY ESCALATION? ---------------------- + // + // MGPipeBarriered is the static wait class PLUS two payload-derived escalations (ID-83): an + // open transform-feedback span makes every context verb inside it barriered, and a draw + // carrying client vertex arrays makes that draw barriered. "Escalated" is therefore exactly + // + // MGPipeBarriered(op, payload, st) && MGPipeWaitClassFor(op) == kWaitNone + // + // and BOTH HALVES ARE ALREADY COMPUTED on every record - ApplyOne computes the predicate + // unconditionally for ID-103's reason and the static class is a table - so this is a bit + // carried beside the existing stamp rather than new plumbing. + // + // IT IS A SEPARATE FLAG AND NOT A REFINEMENT OF THE ONE ABOVE, because the two answer + // different questions. The stamp above answers "is the client parked behind this record", + // which the allocator guard and the frontend-keyed registry guard ask. This one answers "and + // was it parked for a reason the STATIC table cannot see", which only the strict knob asks - + // and it asks because the escalations happen exactly for XFB-active and client-array draws, + // both of which CONTRACT-P5E puts outside this phase (§5.7 and ID-82). A pull on such a + // record is a debt some later phase owes, not a migration P5e skipped. + // + // FALSE IS THE DEFAULT, which is the opposite of the stamp's default and deliberately so: + // "not escalated" is the answer that admits nothing, so a reader that runs before any writer + // is strict rather than lax. + Bool MGPipeApplierCurrentRecordIsBarrieredByEscalation(); + void MGPipeApplierSetCurrentRecordBarrieredByEscalation(Bool escalated); + // THE CLIENT'S HALF OF THE WAIT RULE IS NOT LANDED YET, AND THIS CONSTANT IS THAT FACT. // MGPipeBarriered above describes what the client WILL do; until package ra changes // EmitAndWaitTails the client still blocks after every record, so the answer the server diff --git a/MobileGL/MG_Remote/Server/PipeApplier.cpp b/MobileGL/MG_Remote/Server/PipeApplier.cpp index 622bab8e..04af0ca0 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.cpp +++ b/MobileGL/MG_Remote/Server/PipeApplier.cpp @@ -1199,6 +1199,17 @@ namespace MobileGL::MG_Remote::Server { MG_Pipe::MGPipeApplierSetCurrentRecordBarriered( MGPipeApplierStampsBarriered(MG_Pipe::kMGPipeP5eClientWaitRuleLanded, MGPipeServerPublishesRunAhead(), wireSaysBarriered)); + // P5e (gl), ID-128: AND WHETHER IT WAS BARRIERED BY ESCALATION RATHER THAN BY ITS CLASS. + // The predicate above is the static wait class plus two payload-derived escalations + // (ID-83: an open transform-feedback span, a draw carrying client vertex arrays), so the + // difference between it and the table IS the escalation - both halves are already + // computed here, and the second flag costs one compare. The strict knob is the only + // reader: a pull on an escalated record is a debt the phase that owns XFB (§5.7) or + // client arrays (ID-82) owes, neither of which is this one. + MG_Pipe::MGPipeApplierSetCurrentRecordBarrieredByEscalation( + wireSaysBarriered && + MG_Pipe::MGPipeWaitClassFor(static_cast(record.kind)) == + MG_Pipe::kWaitNone); // ORDER IS THE CONTRACT'S: stamp, then apply. The stamp is what makes any server-side // read of gPipeInputs legal at all (PipeApplier.h's block 1), so a record applied // before it aborts on the FIRST field inside SyncRenderState. diff --git a/MobileGL/MG_Test/Pipe/FieldOwnershipTest.cpp b/MobileGL/MG_Test/Pipe/FieldOwnershipTest.cpp index feb0db9e..1dc143e2 100644 --- a/MobileGL/MG_Test/Pipe/FieldOwnershipTest.cpp +++ b/MobileGL/MG_Test/Pipe/FieldOwnershipTest.cpp @@ -94,11 +94,16 @@ namespace { MG_Config::Ipc.StrictErrors = false; MGPipeServerClearVerbBoundary(); MGPipeResetResidualPullCountForTesting(); + // P5e (gl), ID-128: the escalation flag is per-thread state PipeApplier::ApplyOne + // stamps on every record, so a case that set it would otherwise hand its answer to + // the next one - and the arm it selects is "admitted", so the leak is silent. + MGPipeApplierSetCurrentRecordBarrieredByEscalation(false); #endif } void TearDown() override { #if MOBILEGL_BUILD_DISAGGREGATED MGPipeServerClearVerbBoundary(); + MGPipeApplierSetCurrentRecordBarrieredByEscalation(false); MG_Config::Ipc.StrictErrors = false; #endif MG_State::pGLContext = Move(m_previous); @@ -898,6 +903,49 @@ TEST_F(FieldOwnershipTest, TheSameReadWithoutStrictErrorsSurvivesAndIsCounted) { EXPECT_EQ(r.Log.find("Fatal{"), std::string::npos) << r.Log; } +// P5e (gl) RED-ONCE, ID-128: THE THIRD DISJUNCT, AND IT IS A RUNTIME FACT ABOUT THE RECORD. +// +// GetBoundVertexArray@DrawArrays is this phase's debt on an unbarriered verb, so the static +// table rejects it and strict aborts - that is the case two blocks up, and it must keep doing +// that for the ORDINARY draw path vi and mv retired. But the two +// DirectGLES.Split.ClientVertexArrayScenario entries pull the same pair on a record the client +// IS parked behind: MGPipeBarriered escalates a draw carrying client vertex arrays (ID-83), and +// ID-82 puts client arrays outside P5e entirely - under run-ahead the client refuses them by +// name and staging is P8's. The read site (SyncClientSideVertexArraysForDrawArrays) aborts with +// Fatal{RoleViolation, "MGPipeSlots"} if it is ever applied unbarriered, so the pull is legal. +// +// ONE PAIR, TWO VERDICTS, DECIDED BY THE RECORD AND NOT BY THE TABLE. That is why the flag is +// stamped in ApplyOne beside the barriered stamp rather than folded into the generated table: +// no table indexed by (field, verb) can tell these two entries apart. +// +// THE RED: drop `&& !escalated` from CountBarrierPull's strict arm and this case dies of +// SIGABRT on Fatal{UnmigratedPipeInput, "GetBoundVertexArray@DrawArrays"}. +// +// THE CONTROL IS THE CASE TWO BLOCKS UP, NOT A SECOND CHILD HERE. +// StrictErrorsTurnsABarrierPulledReadIntoANamedAbort drives the IDENTICAL pull - same field, +// same verb, same knob - with the escalation flag at its `false` default, and asserts the abort. +// Reading the log delta of two forks in one case is what a shared log file cannot support. +TEST_F(FieldOwnershipTest, AnEscalatedRecordsPullIsAdmittedAndSaysWhy) { + const ChildResult escalated = RunInChild([] { + MG_Config::Ipc.StrictErrors = true; + // What PipeApplier::ApplyOne stamps for a draw whose payload carries kDrawClientArrays: + // MGPipeBarriered says barriered, the static wait class of draw_vbo says kWaitNone. + MGPipeApplierSetCurrentRecordBarrieredByEscalation(true); + MGPipeServerStampVerbBoundary(MGPipeVerb::DrawArrays); + (void)gPipeInputs.GetBoundVertexArray(); + if (MGPipeResidualPullCount() != 1) ::_exit(7); + }); + ASSERT_TRUE(ExitedWith(escalated, 0)) << DescribeStatus(escalated) << "\n" << escalated.Log; + EXPECT_EQ(escalated.Log.find("Fatal{UnmigratedPipeInput"), std::string::npos) << escalated.Log; + // THE MARKER SAYS WHICH DISJUNCT, and that is what keeps the lane's comparison exact: a pair + // admitted by the table is checkable against `--print-admitted`, one admitted by escalation + // is not in any static list and must not widen one. + EXPECT_NE(escalated.Log.find("Admitted{UnmigratedPipeInput, \"GetBoundVertexArray@DrawArrays\"}" + " [BARRIER-PULLED, ADMITTED-ESCALATED, retires in "), + std::string::npos) + << escalated.Log; +} + // THE SEVEN STICKY FORWARDS ARE NOT EXEMPT FROM THE DETECTOR, which is what this case has // always been for - it was written as "strict PROMOTES them to a Fatal" because before ID-125 // every barrier-pulled read under strict aborted.