[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

@@ -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;

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;

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<std::uint8_t> 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<Uint32>(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 --------------------------------------