From 1a06b40a58ccbd9b3bca2aa875cb5710e2617a40 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 06:40:55 -0400 Subject: [PATCH 1/7] [Fix] (MG_Remote, Wire): let an empty SEG_STAGE take a blob that fits by rebasing its cursors to zero, so the wrap skip is only ever charged against bytes still in flight --- MobileGL/MG_Remote/Wire/PipeWireCodec.cpp | 32 ++++++++++++++++++ MobileGL/MG_Remote/Wire/PipeWireCodec.h | 11 ++++++ MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp | 37 +++++++++++++++++++++ 3 files changed, 80 insertions(+) diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp index 0b13148c..00b7264f 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp @@ -717,6 +717,12 @@ namespace MobileGL::MG_Remote::Wire { } for (int attempt = 0; attempt < 2; ++attempt) { + // THE WRAP SKIP MAY ONLY BE CHARGED AGAINST BYTES THAT ARE STILL IN FLIGHT. When + // there are none the allocator starts over at offset zero, so a blob the segment + // can hold whole is never refused (see RebaseEmptyStage). On attempt 1 this runs + // AFTER ReclaimStagedBytes, which is the case the finding describes: 8 MiB + // allocated, then retired, then a 28 MiB request that used to abort. + RebaseEmptyStage(); const Uint64 offset = m_stageHead % m_stageCapacity; // A run is always contiguous: one that would straddle the end skips the remainder, // exactly as the ring's wrap pad does, and the skipped bytes are reclaimed with @@ -936,6 +942,32 @@ namespace MobileGL::MG_Remote::Wire { return m_emitSeq; } + // THE EMPTY-STAGE REBASE. Head and tail are monotonic byte counts, so "every staged byte + // has retired" reads head == tail, NOT head == tail == 0, and `head % capacity` is left + // wherever the last run ended. Charging a wrap skip against that offset then costs the + // unused suffix a second time: with head == tail == 64 in a 256 KiB stage, the allocator's + // test became `2*capacity - 64 <= capacity`, which is false at EVERY occupancy, so a blob + // that fits the segment whole was refused with `Fatal{RingOverrun, "SEG_STAGE"}` - whose + // own message then reported `0 bytes still in flight`. ReclaimStagedBytes cannot help, + // because an already-empty tail has nothing left to move. + // + // THE MARK QUEUE COMES WITH IT. A mark holds the ABSOLUTE head cursor it was pushed at and + // ReclaimStagedBytes assigns that value straight to m_stageTail. Every mark not yet + // consumed has StageCursor <= head == tail - the head is monotonic and marks are pushed in + // order - so each of them names a region that is already reclaimed and zero is the + // truthful rebasing of it. Without that, one reclaim after a rebase would put the tail + // AHEAD of the head and StagedBytesInFlight() would underflow to about 2^64. + void PipeWireEncoder::RebaseEmptyStage() { + if (m_stageHead != m_stageTail || m_stageHead == 0) { + return; + } + m_stageHead = 0; + m_stageTail = 0; + for (SizeT i = 0; i < m_stageMarks.size(); ++i) { + m_stageMarks[i].StageCursor = 0; + } + } + void PipeWireEncoder::ReclaimStagedBytes() { if (m_control == nullptr) { return; diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.h b/MobileGL/MG_Remote/Wire/PipeWireCodec.h index 1f87f2df..ea2360bb 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.h +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.h @@ -290,6 +290,17 @@ namespace MobileGL::MG_Remote::Wire { // RingControl - see ReclaimStagedBytes above. Uint8* StageAllocate(Uint64 size); + // AN EMPTY STAGE STARTS OVER AT ZERO, so that the wrap skip is only ever charged + // against bytes that are really still in flight. Head and tail are monotonic, so once + // everything has retired they are EQUAL BUT NOT ZERO, and `head % capacity` is + // wherever the last run happened to end - a wrap skip charged against that offset + // costs the suffix a second time and refused a blob the whole segment could hold, with + // a message that reported zero bytes in flight while it did so. Rebasing also rewrites + // the marks still held: a mark stores an ABSOLUTE head cursor and a later reclaim + // assigns it to m_stageTail, so leaving a stale one behind would drive the tail past + // the head and underflow StagedBytesInFlight(). + void RebaseEmptyStage(); + Transport::RingControl* m_control = nullptr; Transport::RingProducer* m_cmd = nullptr; Transport::RingProducer* m_stage = nullptr; diff --git a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp index e29613c1..200bfdbd 100644 --- a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp +++ b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp @@ -1135,6 +1135,43 @@ TEST_F(PipeWireCodecTest, StagedBytesAreReclaimedOnlyBehindRetiredSeq) { ASSERT_TRUE(wire.PumpOne(&applied)); wire.Encoder().ReclaimStagedBytes(); EXPECT_EQ(wire.Encoder().StagedBytesInFlight(), 0u); + + // ---- AND AN EMPTY STAGE TAKES THE WHOLE SEGMENT --------------------------------- + // The verifier's extension of this case, kept (wave1-codex-verify.md §1). The 64-byte + // run has retired and in-flight bytes are ZERO, so every byte of SEG_STAGE is free - + // but head and tail are monotonic and both sit at 64, so `head % capacity` is 64 and the + // allocator used to charge a `capacity - 64` wrap skip against a capacity that had + // nothing in it. The test then read `2*capacity - 64 <= capacity`, false at every + // occupancy, and a blob the segment holds WHOLE aborted with + // `Fatal{RingOverrun, "SEG_STAGE"} ... with 0 bytes still in flight`. + // + // I made it red once, by doing X: X = deleting the `RebaseEmptyStage()` call at the top + // of StageAllocate's attempt loop (PipeWireCodec.cpp). The case then dies with SIGABRT + // inside PipeWireEncoder::StageAllocate on that message, exactly as the verifier + // recorded it. + // + // THE EXACT MAXIMUM. `need = Align8(size)` and the first bound is `need > capacity`, so + // an empty stage takes a blob of exactly the capacity the encoder adopted - here + // Wire2::kStageBytes, and in a real session the whole SEG_STAGE view, i.e. + // MOBILEGL_IPC_STAGE_MB (32 MiB by default; SessionRings.h keeps SEG_STAGE un-ringed and + // un-rounded, so there is no control page to subtract). + const Uint64 maxRecordBefore = wire.Encoder().MaxRecordBytesSeen(); + std::vector whole(Wire2::kStageBytes, 0x5A); + const MGPBlobRef full = wire.Encoder().StageBytes(whole.data(), whole.size()); + EXPECT_EQ(full.Offset, 0u) << "an empty stage must hand a whole-capacity blob offset zero"; + EXPECT_EQ(full.Size, Wire2::kStageBytes); + EXPECT_EQ(full.Seg, static_cast(kSegStage)); + EXPECT_EQ(wire.Encoder().StagedBytesInFlight(), Wire2::kStageBytes); + const void* back = wire.Segments().Resolve(full.Seg, full.Offset, full.Size); + ASSERT_NE(back, nullptr); + EXPECT_EQ(back, wire.StageBase()); + + // R-10's max-record counter DOES NOT SEE IT, and that is the point of R-10's carrier + // rule: EncodeRecord feeds m_maxRecordBytes from `layout.TotalBytes` - header + payload + + // tails, all of it SEG_CMD - while the blob leaves only {Seg, Offset, Size} in the + // record. A quarter-megabyte of staging moved the counter by zero bytes. SEG_STAGE has + // its own bound and its own named Fatal, and MaxRecordBytesSeen() is not it. + EXPECT_EQ(wire.Encoder().MaxRecordBytesSeen(), maxRecordBefore); } // ---- M2 / M3: SEG_STAGE's cursors and the mark queue -------------------------------------- From ba6dc4f7f11e0b8e29a6015d074976363c2241a5 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 06:41:00 -0400 Subject: [PATCH 2/7] [Fix] (MG_Remote, Wire): require every client-server content blob to declare SEG_STAGE and refuse any other carrier by name, which also puts the audit's run bookkeeping out of reach --- MobileGL/MG_Remote/Wire/PipeWireCodec.cpp | 46 ++++++++++++++- MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp | 63 +++++++++++++++++++++ 2 files changed, 108 insertions(+), 1 deletion(-) diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp index 00b7264f..d79af8ff 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp @@ -457,6 +457,36 @@ namespace MobileGL::MG_Remote::Wire { WireOpName(op), static_cast(blob.Size)); std::abort(); } + // R-2.3's SECOND HALF: "inside SOME segment" IS NOT THE RULE. Contract table 1 gives + // every client->server content blob - groups A, B and C, all nineteen rows - the ONE + // carrier SEG_STAGE, and R-10 sends blobs there whole. Until this arm existed the only + // test was that the run resolved, so `CreateSamplerState.Parameters={Seg=SEG_REPLY,...}` + // was accepted and APPLIED: a server-owned segment, whose reuse is the reply pool's + // business and has nothing to do with stage retirement, carrying bytes the applier + // then read. It also went unpoisoned - NoteResolvedRun skipped every non-stage carrier + // - so rule C's only mechanical control read zero on exactly the record that needed it. + // + // The segment is checked BEFORE the resolve, deliberately: a forged SEG_REPLY run that + // happens to lie inside a mapped reply pool must be refused for naming the wrong + // carrier, not left to pass or fail on whether that pool is mapped at all. + // + // NOT A NEW FATAL FAMILY. The review suggested `Fatal{BlobNotStaged}`; this is + // ProtocolCorruption like every other R-2 honesty arm, because the families are the + // vocabulary the operator and the CI greps share (ProtocolCorruption, AbiMismatch, + // UnmigratedVerb, UnmigratedPipeInput, UnsetCallMask, RingOverrun) and a one-off + // seventh name would be a token nothing else in the tree recognises. The SEGMENT is in + // the message, which is what has to be greppable. + if (blob.Seg != kSegStage) { + MGLOG_F("MGPipe: Fatal{ProtocolCorruption, \"%s.blob\"} seg=%u offset=%llu " + "size=%llu is not SEG_STAGE(%u); every client->server content blob is " + "staged whole in SEG_STAGE (contract table 1 groups A/B/C, R-10) and no " + "other segment may carry one", + WireOpName(op), static_cast(blob.Seg), + static_cast(blob.Offset), + static_cast(blob.Size), + static_cast(kSegStage)); + std::abort(); + } if (segments.Resolve(blob.Seg, blob.Offset, blob.Size) == nullptr) { MGLOG_F("MGPipe: Fatal{ProtocolCorruption, \"%s.blob\"} seg=%u offset=%llu size=%llu " "does not lie inside that segment (R-2.3)", @@ -1115,8 +1145,22 @@ namespace MobileGL::MG_Remote::Wire { } void PipeWireDecoder::NoteResolvedRun(MGPWireOp op, const MGPBlobRef& blob) { + // UNREACHABLE NOW, AND LOUD RATHER THAN SILENT. This used to `return`, and that made + // the audit's bookkeeping quietly optional: a record naming a non-SEG_STAGE carrier + // was applied AND recorded nothing, so PoisonedStageBytes() stayed zero and rule C's + // only mechanical control was dark on exactly the record it existed to catch. The one + // caller is ResolveOrFatal, which runs RequireDeclaredBlob first, and that now refuses + // both an undeclared blob and a non-SEG_STAGE one by name. If either ever arrives here + // the audit has stopped covering the carrier, which is the same failure as no audit at + // all - the reason the run-count overflow just below is a Fatal too. if (blob.Size == 0 || blob.Seg != kSegStage) { - return; + MGLOG_F("MGPipe: Fatal{ProtocolCorruption, \"%s\"} the audit was asked to record a " + "resolved run with seg=%u size=%llu; only declared SEG_STAGE(%u) runs " + "reach the poison fill (R-2.5)", + WireOpName(op), static_cast(blob.Seg), + static_cast(blob.Size), + static_cast(kSegStage)); + std::abort(); } if (m_resolvedCount >= sizeof(m_resolved) / sizeof(m_resolved[0])) { // LOUD, NOT A SILENT DROP. This array is what the 0xDD fill covers, and a poison diff --git a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp index 200bfdbd..d6a7878a 100644 --- a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp +++ b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp @@ -1500,6 +1500,69 @@ TEST_F(PipeWireCodecTest, ARunThatLeavesItsSegmentIsFatal) { EXPECT_NE(r.Log.find("does not lie inside that segment"), std::string::npos) << r.Log; } +TEST_F(PipeWireCodecTest, AContentBlobCarriedOutsideSegStageIsFatalAtTheDecoder) { + // R-2.3's second half, and the verifier's finding-3 fixture kept as its own case + // (wave1-codex-verify.md §3). Contract table 1 row 17 puts CreateSamplerState's bytes in + // SEG_STAGE; here they sit in a mapped SEG_REPLY - the SERVER-owned reply pool, whose + // reuse has nothing to do with stage retirement - and the record names that segment. It + // used to be ACCEPTED and APPLIED, because the only test was that the run resolved + // somewhere: the verifier's probe printed `seg=3 accepted=1 poisoned=0`. + // + // The audit is armed, so the second half of the finding is nailed down too: with the + // poison ON, the record must DIE rather than be applied with PoisonedStageBytes() left at + // zero. NoteResolvedRun used to return silently for any non-stage carrier, which made + // rule C's only mechanical control dark on exactly the record it exists to catch; it is + // now a Fatal of its own and unreachable behind this arm. + // + // I made it red once, by doing X: X = deleting the `blob.Seg != kSegStage` arm in + // CheckBlobIsHonest (PipeWireCodec.cpp). The child then exits 0 instead of aborting and + // this case fails on DiedOfAbort - the verifier's `accepted=1` state. + const ChildResult r = RunInChild([] { + Wire2 wire; + std::vector replyBytes(4096, 0); + SamplerParameters params{}; + params.borderColorForm = BorderColorForm::Int; + std::memcpy(replyBytes.data(), ¶ms, sizeof(params)); + wire.Segments().Install(kSegReply, SegmentView{replyBytes.data(), replyBytes.size()}); + wire.Decoder().SetAuditPoison(true); + + MGPSamplerDesc desc{}; + desc.Cso = MakeHandle(88); + desc.Parameters.Seg = static_cast(kSegReply); + desc.Parameters.Offset = 0; + desc.Parameters.Size = sizeof(SamplerParameters); + ForgeAndDecode(wire, MGPWireOp::CreateSamplerState, &desc, sizeof(desc), nullptr, 0); + }); + ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log; + EXPECT_NE(r.Log.find("is not SEG_STAGE"), std::string::npos) << r.Log; + EXPECT_NE(r.Log.find("CreateSamplerState.blob"), std::string::npos) << r.Log; + EXPECT_NE(r.Log.find("seg=3"), std::string::npos) << r.Log; +} + +TEST_F(PipeWireCodecTest, TheEncoderRefusesTheNonStageCarrierTheDecoderCallsFatal) { + // THE ENCODER MUST NOT ACCEPT A RECORD THE DECODER FATALS ON - the same symmetry + // TheEncoderRefusesThePerStageSpirvRunTheDecoderCallsFatal states one arm over. Under + // `inproc` a SEG_REPLY pointer resolves, so an emitter that staged into the reply pool + // would get a valid seq here and a Fatal on a peer, which is the asymmetry EncodeRecord's + // own honesty loop exists to prevent. + // + // I made it red once, by doing X: X = deleting the `blob.Seg != kSegStage` arm in + // CheckBlobIsHonest. EncodeRecord then returns a real seq and the child exits 0. + const ChildResult r = RunInChild([] { + Wire2 wire; + std::vector replyBytes(4096, 0); + wire.Segments().Install(kSegReply, SegmentView{replyBytes.data(), replyBytes.size()}); + MGPSamplerDesc desc{}; + desc.Cso = MakeHandle(88); + desc.Parameters.Seg = static_cast(kSegReply); + desc.Parameters.Offset = 0; + desc.Parameters.Size = sizeof(SamplerParameters); + (void)wire.Encoder().EncodeRecord(MGPWireOp::CreateSamplerState, &desc, sizeof(desc)); + }); + ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log; + EXPECT_NE(r.Log.find("is not SEG_STAGE"), std::string::npos) << r.Log; +} + TEST_F(PipeWireCodecTest, AHalfDeclaredBlobIsFatalRatherThanReadAsAbsent) { // The shape a MONOLITH emitter produces - Seg None, Offset a host address, Size 0. Reading // it as "absent" would silently drop the bytes of every record an unconverted emitter sent. From bb6ba28241d4d8d657d16e97ea8c1f275da9cefe Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 06:41:01 -0400 Subject: [PATCH 3/7] [Fix] (MG_Remote, Wire): send the codec's MapPersistent arm through the same adoption-tier gate the applier uses, so MOBILEGL_IPC_ADOPT_TIER=0/1 die on their named diagnostic --- MobileGL/MG_Remote/Wire/PipeWireCodec.cpp | 16 ++++ MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp | 86 +++++++++++++++++++++ 2 files changed, 102 insertions(+) diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp index d79af8ff..f6c0b846 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp @@ -31,6 +31,9 @@ #include #include #include +// R-6's tier gate, and the ONE spelling of it (b1's file, unchanged by this package): the +// MapPersistent arm below asks it the same question MGPipeApplyMapPersistent asks. +#include #include #include #include @@ -1379,6 +1382,19 @@ namespace MobileGL::MG_Remote::Wire { // // DECLINED is a real answer, not a failure: the three frontend sites already // tolerate it (BufferObject.cpp:238, :603-606, :657-660). + // + // THE TIER IS CONSULTED HERE, AND IT IS THE SAME CONJUNCTION THE MONOLITH APPLIER + // USES (PipeApply.cpp's `Transport != Monolith && AdoptTierIsEmulate()`). The arm + // used to decline UNCONDITIONALLY and AdoptTier had no reference anywhere on the + // codec path, so MOBILEGL_IPC_ADOPT_TIER=0 and =1 - which contract §5 promises + // "parse and are Fatal at use, naming P11" - decoded as an ordinary DECLINED and + // the operator got a run that looked like a working T0. AdoptTierIsEmulate returns + // true at T2 and ABORTS at T0/T1 on its own named diagnostic, so the return value + // is deliberately not a branch: P5 declines at every tier it survives (R-6), and + // the two forbidden ones never get this far. + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) { + (void)MG_Remote::Client::AdoptTierIsEmulate(); + } PostReply(op, seq, ReplySink::kStatusDeclined, nullptr, 0); return true; diff --git a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp index d6a7878a..e61acbdb 100644 --- a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp +++ b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp @@ -34,6 +34,8 @@ #include "Includes.h" +// MG_Config::Transport and MG_Config::Ipc.AdoptTier: the two knobs R-6's tier gate reads. +#include #include #include #include @@ -516,6 +518,37 @@ TEST_F(PipeWireCodecTest, KReplySlotMapPersistentIsAConstantDecline) { EXPECT_TRUE(wire.Answers().All[0].Bytes.empty()); } +TEST_F(PipeWireCodecTest, TierTwoUnderSplitTransportStillDeclinesRatherThanRefusing) { + // The POSITIVE half of the two AdoptTier death cases below. Without it, those two could + // be satisfied by an arm that aborted on every tier, which is the opposite mistake to the + // one wave1-codex-verify.md §4 found. T2 is the only tier P5 implements and R-6 says the + // answer there is DECLINED - a real answer, not a failure - even when the transport is + // the split one that makes the tier question live at all. + const MG_Config::TransportMode savedTransport = MG_Config::Transport; + const Uint32 savedTier = MG_Config::Ipc.AdoptTier; + struct Restore { + MG_Config::TransportMode T; + Uint32 A; + ~Restore() { + MG_Config::Transport = T; + MG_Config::Ipc.AdoptTier = A; + } + } restore{savedTransport, savedTier}; + MG_Config::Transport = MG_Config::TransportMode::InProcess; + MG_Config::Ipc.AdoptTier = 2u; + + Wire2 wire; + const MGPHandleOnly handle = HandleOnly(5, MGPipeKind::Buffer); + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::MapPersistent, &handle, sizeof(handle)), + kInvalidSeq); + bool applied = false; + ASSERT_TRUE(wire.PumpOne(&applied)); + EXPECT_TRUE(applied); + ASSERT_EQ(wire.Answers().All.size(), 1u); + EXPECT_EQ(wire.Answers().All[0].Status, ReplySink::kStatusDeclined); + EXPECT_TRUE(wire.Answers().All[0].Bytes.empty()); +} + TEST_F(PipeWireCodecTest, KNeedsAckRespecifyCarriesItsRedefinitionScope) { // Contract table 1 row 19b. Without the carrier every per-level glTexImage*D would take // the whole-resource arm on the far side and eat the other levels' pending uploads, so @@ -1818,6 +1851,59 @@ TEST_F(PipeWireCodecTest, ASecondProcessResolverIsFatalRatherThanASilentRace) { EXPECT_NE(r.Log.find("already installed"), std::string::npos) << r.Log; } +// ---- R-6 / contract §5: the two forbidden adoption tiers die ON THE WIRE PATH TOO -------- +// +// wave1-codex-verify.md §4: `AdoptTier` had ZERO references anywhere on the codec path, so +// MOBILEGL_IPC_ADOPT_TIER=0 and =1 - which contract §5 promises "parse and are Fatal at use, +// naming P11" - decoded as an ordinary DECLINED. The verifier set each forbidden tier inside +// KReplySlotMapPersistentIsAConstantDecline and watched its successful-decline assertions +// still pass, on BOTH tiers. +// +// THESE ARE FORKED, NOT EXPECT_DEATH, for the reason at the top of this file - and forking is +// what lets the case REQUIRE THE DIAGNOSTIC rather than any abort: r.Log is searched for the +// exact sentence AdoptTierIsEmulate prints. ID-46 finding 10 is an empty death regex; the +// EXPECT_NE lines below are the opposite of that, and a crash for any other reason fails the +// case on the log it prints. +// +// I made both red once, by doing X: X = restoring the unconditional decline in +// PipeWireCodec.cpp's MapPersistent arm (deleting the AdoptTierIsEmulate call). Both children +// then exit 0 having posted a clean DECLINED, and both cases fail on DiedOfAbort. + +TEST_F(PipeWireCodecTest, AdoptTierZeroIsFatalOnTheWirePathAndNamesP11) { + const ChildResult r = RunInChild([] { + // The child dies; nothing needs restoring. The transport half is the same conjunction + // MGPipeApplyMapPersistent uses - a monolith TRANSPORT mints like push (ID-42) and is + // not the arm this record can arrive on. + MG_Config::Transport = MG_Config::TransportMode::InProcess; + MG_Config::Ipc.AdoptTier = 0u; + Wire2 wire; + const MGPHandleOnly handle = HandleOnly(5, MGPipeKind::Buffer); + (void)wire.Encoder().EncodeRecord(MGPWireOp::MapPersistent, &handle, sizeof(handle)); + bool applied = false; + (void)wire.PumpOne(&applied); + }); + ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log; + EXPECT_NE(r.Log.find("MOBILEGL_IPC_ADOPT_TIER=0 names adoption tier T0, which P11 implements"), + std::string::npos) + << r.Log; +} + +TEST_F(PipeWireCodecTest, AdoptTierOneIsFatalOnTheWirePathAndNamesP11) { + const ChildResult r = RunInChild([] { + MG_Config::Transport = MG_Config::TransportMode::InProcess; + MG_Config::Ipc.AdoptTier = 1u; + Wire2 wire; + const MGPHandleOnly handle = HandleOnly(5, MGPipeKind::Buffer); + (void)wire.Encoder().EncodeRecord(MGPWireOp::MapPersistent, &handle, sizeof(handle)); + bool applied = false; + (void)wire.PumpOne(&applied); + }); + ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log; + EXPECT_NE(r.Log.find("MOBILEGL_IPC_ADOPT_TIER=1 names adoption tier T1, which P11 implements"), + std::string::npos) + << r.Log; +} + #else TEST_F(PipeWireCodecTest, TheFatalArmsNeedFork) { From 2e2b41ab49aeaa7fb894642132ef71ffae65fb00 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 06:58:34 -0400 Subject: [PATCH 4/7] [Fix, Test] (workflows, scripts/ci): make the three CI negative controls assert their own failure reason instead of accepting any non-zero ctest exit, count the arming baseline by PASSED rather than by not-skipped, and put both control bodies in files a smoke test can drive against a stubbed ctest --- .github/workflows/test.yml | 130 ++++++++---------- scripts/ci/control_smoke_test.sh | 106 +++++++++++++++ scripts/ci/junit_tally.py | 49 +++++++ scripts/ci/redcheck_control_smoke_test.sh | 71 ++++++++++ scripts/ci/retrace_pull_library_control.sh | 117 ++++++++++++++++ scripts/ci/split_negative_controls.sh | 151 +++++++++++++++++++++ scripts/ci/testdata/stub_ctest.sh | 119 ++++++++++++++++ 7 files changed, 667 insertions(+), 76 deletions(-) create mode 100755 scripts/ci/control_smoke_test.sh create mode 100755 scripts/ci/junit_tally.py create mode 100755 scripts/ci/redcheck_control_smoke_test.sh create mode 100644 scripts/ci/retrace_pull_library_control.sh create mode 100755 scripts/ci/split_negative_controls.sh create mode 100755 scripts/ci/testdata/stub_ctest.sh diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c1d81e2e..896e8ad9 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1074,57 +1074,32 @@ jobs: # Split entries SKIP and ctest reports green whatever the knob says, so an unconditional # control would be red for the whole of P5 for a reason that is not a defect. # - # So the expected state is DERIVED rather than assumed, from the same fact the lanes derive - # it from: MG_IntegrationTest/CMakeLists.txt puts MGITEST_REMOTE_CLIENT_PRESENT=1 into the - # Split entries' ENVIRONMENT exactly when MG_Remote carries no c0 signature stub, and that - # string is in the generated ctest include files this artifact ships. When it is there the - # controls MUST fire; when it is not, the step says so loudly and does not pretend. + # So the expected state is DERIVED FROM BEHAVIOUR rather than assumed. The first version read + # MGITEST_REMOTE_CLIENT_PRESENT out of the generated *_tests.cmake, which was a restatement of + # the CMake source probe review finding M-1 falsified; the arming condition is a runtime fact + # inside each test process (MG_Config::Transport, ClientSession::Active() and + # ImplementedVerbCount(), read by Harness/SplitRuntimePeek), so the only honest way to ask it + # from a shell is to look at what the entries DID. When entries passed, the controls MUST + # fire; when every one of them skipped, the step says so loudly and does not pretend. + # + # THE BODY OF THIS STEP IS scripts/ci/split_negative_controls.sh, and the move is the point + # rather than tidiness. A `run:` block executes nowhere but on a runner, so these lines were + # unreviewable and untestable: when the wave-1 cross-family review said they were broken, + # CONFIRMING it needed a hand-made copy of them (wave1-codex-verify.md 8), and a copy is not + # the thing. scripts/ci/control_smoke_test.sh now drives the very file this step runs. + # + # What that smoke test pins, and what ID-46 finding 8 found missing: each control asserts its + # OWN failure reason. A non-zero ctest exit used to be enough, so a timeout, a setup abort or + # any unrelated assertion printed "turned N selected entries red, as it must" and this step + # went green. The arming run's `|| true` had the matching defect - it counted a case that ran + # and FAILED as evidence the lane was live, so the controls could be measured against a + # baseline that was already red. - name: Negative controls - the verb barrier and the persistent-map push must be load-bearing working-directory: build-split env: MOBILEGL_ITEST_REQUIRE_GPU: "1" - run: | - # THE ARMED STATE IS DERIVED FROM BEHAVIOUR, not from a marker string in the generated - # ctest files. The first version read MGITEST_REMOTE_CLIENT_PRESENT out of - # *_tests.cmake, which was a restatement of the CMake source probe review finding M-1 - # falsified; the arming condition is now a runtime fact inside each test process, so the - # only honest way to ask it from a shell is to look at what the entries DID. - ctest -L integration-split -j 4 --no-tests=error --output-junit "${RUNNER_TEMP}/isplit.xml" || true - armed=$(python3 - "${RUNNER_TEMP}/isplit.xml" <<'PY' - import sys, xml.etree.ElementTree as ET - ran = 0 - for case in ET.parse(sys.argv[1]).getroot().iter('testcase'): - if case.find('skipped') is None and case.get('status') not in ('notrun', 'disabled'): - ran += 1 - print(ran) - PY - ) - echo "split entries that actually ran: ${armed}" - if [ "${armed}" -lt 1 ]; then - echo "::warning::every DirectGLES.Split. entry SKIPPED, so neither negative control can fire. The arming condition is a runtime fact - MG_Config::Transport, ClientSession::Active() and ImplementedVerbCount(), read by Harness/SplitRuntimePeek - and it becomes true on the commit that lands the last of c1/s1/v1. This step becomes a gate then, with no edit; it is not a green that asserted anything today." - exit 0 - fi - run_control() { - name="$1"; filter="$2"; shift 2 - matched=$(ctest -N -L integration-split -R "${filter}" | grep -cE '^ *Test *#[0-9]+:') - if [ "${matched}" -lt 1 ]; then - echo "::error::${name} selected ${matched} tests; its filter no longer matches anything" - exit 1 - fi - if env "$@" ctest --output-on-failure -L integration-split -R "${filter}" --no-tests=error; then - echo "::error::${name} left ${matched} split entries GREEN, so the knob it turns is not load-bearing and the gate it controls proves nothing." - exit 1 - fi - echo "${name} turned ${matched} selected entries red, as it must" - } - # E1: R-1's lockstep verb barrier. Without it the client keeps pulling fields from a live - # GLContext while the server runs ahead, so the server reads future values. - run_control "negative control E1 (MOBILEGL_IPC_VERB_BARRIER=0)" \ - 'DirectGLES\.Split\.(Triangle|ClearThenReadPixels)' MOBILEGL_IPC_VERB_BARRIER=0 - # E3(a): the persistent-map push. 0 is admitted by ConfigLoader on purpose and is - # documented there as this control. - run_control "negative control E3(a) (MOBILEGL_IPC_PERSISTENT_BLOCK_KB=0)" \ - 'DirectGLES\.Split\.PersistentCoherentMapScenario' MOBILEGL_IPC_PERSISTENT_BLOCK_KB=0 + CONTROL_TMPDIR: ${{ runner.temp }} + run: bash "${GITHUB_WORKSPACE}/scripts/ci/split_negative_controls.sh" - name: Upload split lane logs if: always() @@ -1959,37 +1934,25 @@ jobs: # The rerun replays into the same case directory, so the good run's images are put aside and # restored whichever way the control goes; "Upload actual image" below runs `if: always()` # and would otherwise ship the deliberately-wrong run's output under the good run's name. + # THE BODY OF THIS STEP IS scripts/ci/retrace_pull_library_control.sh, for the reason the + # split lane's control gives: a `run:` block cannot be executed off a runner, so these lines + # could not be tested until they ran in CI. scripts/ci/control_smoke_test.sh drives that file. + # + # Two holes ID-46 finding 8(b) found in this block, both CONFIRMED against the REAL ctest in a + # REAL build tree, both closed in the script: it had NO selection guard at all - unlike the + # split lane's run_control - so a case/backend regex matching nothing exited 8 through + # `--no-tests=error` and was read as "the pull library turned it red"; and only "non-zero + # ctest" was checked after the nm identity check, so a loader failure, a missing fixture or a + # timeout passed it. The red must now carry run_trace_case.cmake's own sentence. - name: Negative control - the PULL library must red this split retrace working-directory: build-retrace/tools/trace_replay - run: | - set +e - GOOD_OUTPUT="${RUNNER_TEMP}/split-verified-output" - rm -rf "${GOOD_OUTPUT}" - if [ -d "${{ matrix.case }}" ]; then cp -a "${{ matrix.case }}" "${GOOD_OUTPUT}"; fi - # The pull library, unpacked from build-linux's artifact, over the frozen path every - # case has baked in. It defines no MG_Remote symbol, so ConfigLoader has no transport - # parser and MOBILEGL_TRANSPORT=inproc is accepted and ignored - the exact shape of "the - # split lane ran monolith". - cp "${GITHUB_WORKSPACE}/pull-runtime/build-linux/libMobileGL.so" \ - "${GITHUB_WORKSPACE}/build-linux/libMobileGL.so" - if nm --defined-only "${GITHUB_WORKSPACE}/build-linux/libMobileGL.so" | grep -q -i MG_Remote; then - echo "::error::the control's own library defines MG_Remote symbols, so it is not a pull build and this control would prove nothing" - exit 1 - fi - export MOBILEGL_TRANSPORT=inproc - ctest -V --no-tests=error --timeout 10800 \ - -R "^MobileGLTraceReplay\.${{ matrix.case }}\.${{ matrix.backend }}$" - control_rc=$? - set -e - if [ -d "${GOOD_OUTPUT}" ]; then - rm -rf "${{ matrix.case }}"; mv "${GOOD_OUTPUT}" "${{ matrix.case }}" - echo "restored the verified run's output over the control's" - fi - if [ "${control_rc}" -eq 0 ]; then - echo "::error::a PULL library passed the split retrace. OpenRA scores ssim 1.000000 under a monolith library too (measured), so the picture is not and cannot be this lane's gate - run_trace_case.cmake's transport-resolution assertion is, and it has stopped working. Every green in this job is then a monolith run under a name that says split." - exit 1 - fi - echo "the pull library turned the split retrace red, as it must (ctest exit ${control_rc})" + env: + CONTROL_TMPDIR: ${{ runner.temp }} + PULL_LIBRARY: ${{ github.workspace }}/pull-runtime/build-linux/libMobileGL.so + FROZEN_LIBRARY: ${{ github.workspace }}/build-linux/libMobileGL.so + run: >- + bash "${GITHUB_WORKSPACE}/scripts/ci/retrace_pull_library_control.sh" + '${{ matrix.case }}' '${{ matrix.backend }}' # The refusal census, recorded rather than gated. run_trace_case.cmake already REDS the case # on any Fatal{, so reaching here means the count is zero - but the number and the distinct @@ -2314,6 +2277,21 @@ jobs: python3 scripts/gen_pipe_field_ownership.py --check python3 scripts/gen_pipe_field_ownership.py --self-test + # R-16 APPLIED TO THE NEGATIVE CONTROLS THEMSELVES. The split lane's E1/E3(a) controls and the + # retrace lane's pull-library control are gates, and until ID-46 finding 8 neither could be + # made red by anyone: their bodies were `run:` blocks, which execute only on a runner. Both + # bodies now live in scripts/ci/, and this step runs them against a stubbed ctest that + # reproduces the finding - a NON-EMPTY selection failing with UNRELATED_CONTROL_FAILURE, and a + # case/backend regex matching no tests - and requires each control to report FAILED. The same + # stub, failing with the diagnostics the scenarios really emit, must make them report PASSED. + # + # NO BRANCH GUARD: this asks "do the negative controls still reject a red that is not theirs", + # which is a question every branch can answer and none of which depends on the TEMPORARY + # feat/disaggregated trigger at the top of this file. It costs a couple of seconds and needs + # no build. + - name: The split and retrace negative controls still reject a red that is not theirs (R-16) + run: bash scripts/ci/control_smoke_test.sh + # A GATE as of P3a (G5). "pool 与延迟释放原样搬" (ROADMAP.md:19) is meant literally: the # buffer pool, the deferred-release drain and the three persistently mapped rings move # VERBATIM, and ARCHITECTURE.md:515 says why - their retire happens only inside Present, so diff --git a/scripts/ci/control_smoke_test.sh b/scripts/ci/control_smoke_test.sh new file mode 100755 index 00000000..d26fc2fc --- /dev/null +++ b/scripts/ci/control_smoke_test.sh @@ -0,0 +1,106 @@ +#!/bin/bash +# R-16 FOR THE CI NEGATIVE CONTROLS THEMSELVES: a control-run smoke test. +# +# BRIEF-P5 13 (R-16) says a negative control must assert its own failure reason and that every gate +# carries a line saying "I made it red once, by doing X". The two controls this file exercises ARE +# gates, and until ID-46 finding 8 nobody could make either of them red, because a workflow `run:` +# block only executes on a runner. The wave-1 verification agent had to hand-copy the blocks into +# throwaway harnesses to show they were broken (wave1-codex-verify.md 8). This file is that +# experiment, kept: it runs the REAL control scripts - the same files .github/workflows/test.yml +# invokes, not copies of them - against a stubbed ctest, and checks that each one passes exactly +# when it should. +# +# The case that matters is the first one. A stubbed ctest reports a NON-EMPTY selection and then +# fails with UNRELATED_CONTROL_FAILURE: a reason that has nothing to do with the knob the control +# turns. Before ID-48's fix both controls printed their success message and the step exited 0. They +# must now report FAILED. +# +# usage: control_smoke_test.sh +set -u + +HERE="$(cd "$(dirname "$0")" && pwd)" +WORK="$(mktemp -d)" +trap 'rm -rf "${WORK}"' EXIT + +STUB_DIR="${WORK}/stub" +mkdir -p "${STUB_DIR}" +cp "${HERE}/testdata/stub_ctest.sh" "${STUB_DIR}/ctest" +chmod +x "${STUB_DIR}/ctest" + +passes=0 +failures=0 + +# expect