diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp index a1ee9f27..0b13148c 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp @@ -61,27 +61,94 @@ namespace MobileGL::MG_Remote::Wire { static_assert(static_cast(MG_Pipe::kMGHostSpanSegNone) == kSegNone, "kMGHostSpanSegNone and SegmentId::kSegNone must be the same value"); - // The collision named in the file header, asserted rather than described. If a later edit - // moves either bit these fire and the mapping below is revisited; if someone deletes the - // mapping and stamps MGPipeCallFlags straight into the header, the RingTest wrap cases - // stay green and nine opcodes vanish, which is why this is a static_assert and not a - // comment. - static_assert(static_cast(MG_Pipe::kVarTail) == - static_cast(Transport::kRecPad), - "MGPipeCallFlags::kVarTail and RingRecordFlags::kRecPad share a bit AND a " - "field; the encoder must translate, never stamp. If this ever stops being " - "true, keep translating anyway - two flag spaces in one field is the hazard, " - "not this particular overlap"); - static_assert(static_cast(MG_Pipe::kHostSpan) == - static_cast(Transport::kRecBorrowSlot), - "MGPipeCallFlags::kHostSpan and RingRecordFlags::kRecBorrowSlot share a bit"); + // ---- the collision, asserted EXHAUSTIVELY rather than described ------------------- + // + // Naming the two overlaps somebody happened to notice is not enough: the review found a + // THIRD (kReplySlot == kRecVarTail) that four hand-written asserts had missed, and the + // fourth would have been found by a user. So the assertions below cover the whole of both + // enums - every bit, the exact overlap mask, and the completeness of the translation - and + // a new enumerator on either side breaks the build rather than being dropped in silence. + namespace FlagSpace { + constexpr Uint16 kCallAll = static_cast(MG_Pipe::kNeedsAck) | + static_cast(MG_Pipe::kHasBlob) | + static_cast(MG_Pipe::kVarTail) | + static_cast(MG_Pipe::kHostSpan) | + static_cast(MG_Pipe::kReplySlot) | + static_cast(MG_Pipe::kOptional); + constexpr Uint16 kRingAll = static_cast(Transport::kRecNeedsAck) | + static_cast(Transport::kRecHasBlob) | + static_cast(Transport::kRecPad) | + static_cast(Transport::kRecBorrowSlot) | + static_cast(Transport::kRecVarTail); + // The three call flags the encoder TRANSLATES, and the three it deliberately drops + // because MGPipeCallFlagsFor(op) recovers them from the opcode. + constexpr Uint16 kTranslated = static_cast(MG_Pipe::kNeedsAck) | + static_cast(MG_Pipe::kHasBlob) | + static_cast(MG_Pipe::kVarTail); + constexpr Uint16 kDropped = static_cast(MG_Pipe::kHostSpan) | + static_cast(MG_Pipe::kReplySlot) | + static_cast(MG_Pipe::kOptional); + } // namespace FlagSpace + + static_assert(FlagSpace::kCallAll == 0x3Fu, + "MGPipeCallFlags gained or lost an enumerator; re-derive the translation in " + "EncodeRecord and extend the overlap assertions below before assuming the " + "new bit is safe to drop"); + static_assert(FlagSpace::kRingAll == 0x1Fu, + "RingRecordFlags gained or lost an enumerator; the two spaces share one " + "16-bit field, so a new ring bit may now alias a call flag"); + static_assert((FlagSpace::kTranslated | FlagSpace::kDropped) == FlagSpace::kCallAll && + (FlagSpace::kTranslated & FlagSpace::kDropped) == 0, + "every MGPipeCallFlags bit must be either translated or deliberately " + "dropped; a seventh enumerator that needs a ring bit would otherwise be " + "dropped in silence"); + // FIVE OF THE SIX CALL-FLAG BITS ALIAS A RING BIT. Only kOptional (1<<5) is free, and it + // is free by luck rather than by design - RingRecordFlags simply has not reached 1<<5 yet. + static_assert((FlagSpace::kCallAll & FlagSpace::kRingAll) == FlagSpace::kRingAll, + "the overlap between the two flag spaces moved"); + // Named individually so a failure says WHICH pair, and so the three that actually bite are + // impossible to overlook while reading. static_assert(static_cast(MG_Pipe::kNeedsAck) == static_cast(Transport::kRecNeedsAck), - "the two kNeedsAck bits agree; the mapping below relies on it only for " - "readability, not for correctness"); + "kNeedsAck == kRecNeedsAck (harmless: the meanings agree)"); static_assert(static_cast(MG_Pipe::kHasBlob) == static_cast(Transport::kRecHasBlob), - "the two blob bits agree"); + "kHasBlob == kRecHasBlob (harmless: the meanings agree)"); + static_assert(static_cast(MG_Pipe::kVarTail) == + static_cast(Transport::kRecPad), + "kVarTail == kRecPad - THE DANGEROUS ONE. Stamping MGPipeCallFlags into the " + "header would make RingConsumer::Pop discard all nine kVarTail opcodes as " + "wrap fillers. The encoder must translate, never stamp; if this ever stops " + "being true, keep translating anyway - two flag spaces in one field is the " + "hazard, not this particular overlap"); + static_assert(static_cast(MG_Pipe::kHostSpan) == + static_cast(Transport::kRecBorrowSlot), + "kHostSpan == kRecBorrowSlot - a stamped host-span record would look like a " + "slot borrowed into the GPU timeline and retire on completedFrameSerial"); + static_assert(static_cast(MG_Pipe::kReplySlot) == + static_cast(Transport::kRecVarTail), + "kReplySlot == kRecVarTail - live in the REVERSE direction: the encoder " + "stamps kRecVarTail on nine opcodes, and a reader who believes " + "PipeWire.inc's 'MGPipeCallFlags of the call' comment reads those nine as " + "kReplySlot"); + static_assert((static_cast(MG_Pipe::kOptional) & FlagSpace::kRingAll) == 0, + "kOptional is the one call flag with no ring alias, and only because " + "RingRecordFlags has not reached 1<<5"); + + // ---- and the two headers really are one layout ----------------------------------- + // + // The decoder walks backwards from RingRecordView::payload to the MGPWireRecHeader in + // front of it, so the two structs being separately asserted to be eight bytes is not + // enough: if RingRecordHeader ever reorders its fields, every flag assert above still + // passes and every payload read shifts by the difference. + static_assert(sizeof(MGPWireRecHeader) == sizeof(Transport::RingRecordHeader)); + static_assert(offsetof(MGPWireRecHeader, Op) == offsetof(Transport::RingRecordHeader, kind), + "MGPWireRecHeader::Op and RingRecordHeader::kind are the same two bytes"); + static_assert(offsetof(MGPWireRecHeader, Flags) == + offsetof(Transport::RingRecordHeader, flags), + "MGPWireRecHeader::Flags and RingRecordHeader::flags are the same two bytes"); + static_assert(offsetof(MGPWireRecHeader, Size) == offsetof(Transport::RingRecordHeader, size), + "MGPWireRecHeader::Size and RingRecordHeader::size are the same four bytes"); // --------------------------------------------------------------------------------- // The catalogue, once. Name and payload type per opcode. @@ -427,6 +494,29 @@ namespace MobileGL::MG_Remote::Wire { } } + void CheckHostSpanIsHonest(const MGHostSpan& span, const SegmentTable& segments) { + CheckHostSpanIsHonest(span); + if (span.Size == 0) { + // Fully absent, and the arms above already proved Seg agrees with that. + return; + } + // ARM 4, WHICH THE OTHER OVERLOAD CANNOT DO. A span naming a real segment and a run + // past the end of it used to pass every check and reach WireVerbSink::OnDrawVbo, which + // this file's header promises is "a DECODED, VALIDATED argument list". It resolves to + // nullptr through MGPipeHostBytes - a draw from a null index pointer - or, for any + // consumer that adds Offset to its own SEG_STAGE base instead of going through the + // resolver, reads outside the segment. P5 emits no spans, so this was latent; P8 arms + // it, which is exactly when nobody will be reading this code. + if (segments.Resolve(span.Seg, span.Offset, span.Size) == nullptr) { + MGLOG_F("MGPipe: Fatal{ProtocolCorruption, \"host-span\"} seg=%u offset=%llu " + "size=%llu does not lie inside that segment (R-2.3 arm 4)", + static_cast(span.Seg), + static_cast(span.Offset), + static_cast(span.Size)); + std::abort(); + } + } + // --------------------------------------------------------------------------------- // The record layout: the tail arithmetic both sides run // --------------------------------------------------------------------------------- @@ -527,13 +617,27 @@ namespace MobileGL::MG_Remote::Wire { tails = 1; break; } - case MGPWireOp::ResourceSubData: - case MGPWireOp::BufferSubDataResident: { + case MGPWireOp::ResourceSubData: { const auto& p = *static_cast(payload); tail0 = TailBytesFor(p.RegionCount, sizeof(MGPSubRegion)); tails = 1; break; } + case MGPWireOp::BufferSubDataResident: { + // NO TAIL, because its catalogue row has no kVarTail (PipeCalls.def gives it + // kHasBlob|kOptional) and MGPipeApplyBufferSubDataResident takes no regions. The + // layout used to share ResourceSubData's arm, which meant a record with + // RegionCount = 2 was REQUIRED to carry 80 bytes the arm then dropped on the + // floor - and MGPipeBuildSubDataRecord is the shared builder that fills + // RegionCount for both halves, so that was one routing change away from being + // live. The resident path is the BUFFER half only and a buffer record declares no + // regions, so a non-zero count is a fault rather than a tail. + const auto& p = *static_cast(payload); + if (p.RegionCount != 0) { + WireProtocolFatalAt("BufferSubDataResident.RegionCount", p.RegionCount, 0); + } + break; + } case MGPWireOp::DrawVbo: { // MGPDrawRange[NumDraws], then a CONDITIONAL MGHostSpan. The span's start is // realigned to 8 because MGPDrawRange is twelve bytes: see WireRecordLayout's @@ -580,10 +684,72 @@ namespace MobileGL::MG_Remote::Wire { Bool PipeWireEncoder::Valid() const { return m_control != nullptr && m_cmd != nullptr; } + Uint8* PipeWireEncoder::StageAllocate(Uint64 size) { + if (m_stageBase == nullptr) { + const SegmentView view = m_segments != nullptr ? m_segments->Get(kSegStage) + : SegmentView{}; + if (view.Base == nullptr || view.Size == 0) { + WireProtocolFatal("PipeWireEncoder::StageBytes", "SEG_STAGE has no segment view"); + } + // The RingProducer c0's constructor takes is the authority on how many of the + // segment's bytes are really the staging area - the rest is whatever the session + // put in front of it. It is read, never written: SEG_STAGE's cursor triple belongs + // to nobody in P5 (see ReclaimStagedBytes' header). + Uint64 capacity = view.Size; + if (m_stage != nullptr && m_stage->Valid()) { + if (m_stage->Capacity() > view.Size) { + WireProtocolFatalAt("SEG_STAGE.capacity", m_stage->Capacity(), view.Size); + } + capacity = m_stage->Capacity(); + } + m_stageBase = static_cast(view.Base); + m_stageCapacity = capacity; + } + + const Uint64 need = Align8(size); + if (need > m_stageCapacity) { + MGLOG_F("MGPipe: Fatal{RingOverrun, \"SEG_STAGE\"} a %llu byte blob cannot fit a " + "%llu byte staging segment at any occupancy; P5 does not chunk (R-10) - " + "raise MOBILEGL_IPC_STAGE_MB or report the record to the integrator", + static_cast(size), + static_cast(m_stageCapacity)); + std::abort(); + } + + for (int attempt = 0; attempt < 2; ++attempt) { + 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 + // everything else behind them. + const Uint64 skip = offset + need > m_stageCapacity ? m_stageCapacity - offset : 0; + if ((m_stageHead + skip + need) - m_stageTail <= m_stageCapacity) { + const Uint64 at = (m_stageHead + skip) % m_stageCapacity; + m_stageHead += skip + need; + return m_stageBase + at; + } + if (attempt == 0) { + // One try at reclaiming what the server has already retired. A second failure + // means the bytes genuinely do not fit, which R-10 says P5 does not chunk and + // must instead prove it never needs to. + ReclaimStagedBytes(); + } + } + MGLOG_F("MGPipe: Fatal{RingOverrun, \"SEG_STAGE\"} a %llu byte blob does not fit a %llu " + "byte staging segment with %llu bytes still in flight (retiredSeq=%llu); P5 " + "does not chunk (R-10) - raise MOBILEGL_IPC_STAGE_MB or report the record to " + "the integrator", + static_cast(size), + static_cast(m_stageCapacity), + static_cast(m_stageHead - m_stageTail), + static_cast( + m_control != nullptr ? m_control->retiredSeq.load(std::memory_order_acquire) : 0)); + std::abort(); + } + MGPBlobRef PipeWireEncoder::StageBytes(const void* bytes, Uint64 size) { - if (!Valid() || m_stage == nullptr || m_segments == nullptr) { + if (!Valid() || m_segments == nullptr) { WireProtocolFatal("PipeWireEncoder::StageBytes", - "no SEG_STAGE producer or segment table installed"); + "no command producer or segment table installed"); } if (size == 0) { // "The record declared no blob" and "the record declared an empty blob" must not @@ -596,33 +762,9 @@ namespace MobileGL::MG_Remote::Wire { WireProtocolFatal("PipeWireEncoder::StageBytes", "non-zero size with a null source"); } - void* slot = m_stage->Reserve(static_cast(MGPWireOp::kInvalid), - Transport::kRecHasBlob, size); - if (slot == nullptr) { - // One try at reclaiming what the server has already retired, then give up: a - // second failure means the run genuinely does not fit SEG_STAGE, which R-10 says - // P5 does not chunk and must instead prove it never needs to. - ReclaimStagedBytes(); - slot = m_stage->Reserve(static_cast(MGPWireOp::kInvalid), - Transport::kRecHasBlob, size); - } - if (slot == nullptr) { - MGLOG_F("MGPipe: Fatal{RingOverrun, \"SEG_STAGE\"} a %llu byte blob does not fit a " - "%llu byte staging ring with %llu bytes free; P5 does not chunk (R-10) - " - "raise MOBILEGL_IPC_STAGE_MB or report the record to the integrator", - static_cast(size), - static_cast(m_stage->Capacity()), - static_cast(m_stage->FreeBytes())); - std::abort(); - } + Uint8* slot = StageAllocate(size); std::memcpy(slot, bytes, static_cast(size)); - - const SegmentView stageView = m_segments->Get(kSegStage); - if (stageView.Base == nullptr) { - WireProtocolFatal("PipeWireEncoder::StageBytes", "SEG_STAGE has no segment view"); - } - const Uint64 offset = - static_cast(static_cast(slot) - static_cast(stageView.Base)); + const Uint64 offset = static_cast(slot - m_stageBase); MGPBlobRef ref{}; ref.Seg = kSegStage; @@ -630,18 +772,14 @@ namespace MobileGL::MG_Remote::Wire { ref.Size = size; ref.Pad0 = 0; // The self-check that keeps the two halves of "SEG_STAGE" one thing: the segment view - // the decoder resolves through must cover the ring this producer just wrote into. A - // view installed over the CONTROL page, or over the ring plus its header, resolves to + // the decoder resolves through must cover the bytes this allocator just wrote into. A + // view installed over the CONTROL page, or over the segment plus a header, resolves to // a plausible pointer that is not these bytes. if (m_segments->Resolve(ref.Seg, ref.Offset, ref.Size) != slot) { WireProtocolFatal("PipeWireEncoder::StageBytes", - "the SEG_STAGE segment view does not cover the staging ring's " - "byte area; the two would resolve to different addresses"); + "the SEG_STAGE segment view does not cover the staging area; the " + "two would resolve to different addresses"); } - // The stage ring's own Publish: the decoder reads these bytes by OFFSET, never by - // popping the stage ring, so the head has to be visible before the command record - // that names them is. - m_stage->Publish(); return ref; } @@ -759,16 +897,32 @@ namespace MobileGL::MG_Remote::Wire { MGPBlobRef blob{}; std::memcpy(&blob, bytes + slots.Offset + i * sizeof(MGPBlobRef), sizeof(MGPBlobRef)); CheckBlobIsHonest(op, blob, *m_segments); + // THE ENCODER MUST NOT ACCEPT A RECORD THE DECODER FATALS ON. w1's ruling that + // CreateShaderState's modules travel inside the Reflection archive lived only + // in the decoder, so an emitter that declared a per-stage run got a valid seq + // here and a Fatal on a peer - exactly the asymmetry EncodeRecord's own + // comment says it exists to prevent. + if (op == MGPWireOp::CreateShaderState && i < 6 && blob.Size != 0) { + MGLOG_F("MGPipe: Fatal{ProtocolCorruption, \"CreateShaderState.Spirv[%u]\"} " + "declares %llu bytes at the ENCODER; under split the modules travel " + "inside the Reflection archive and the six per-stage runs stay " + "undeclared", + static_cast(i), + static_cast(blob.Size)); + std::abort(); + } } } if ((callFlags & static_cast(kHostSpan)) != 0 && layout.TailCount == 2 && - layout.TailBytes[1] != 0) { + layout.TailBytes[1] != 0 && m_segments != nullptr) { const Uint64 at = layout.TailOffset[1] - sizeof(MGPWireRecHeader); const Uint64 spans = layout.TailBytes[1] / sizeof(MGHostSpan); for (Uint64 i = 0; i < spans; ++i) { MGHostSpan span{}; std::memcpy(&span, bytes + at + i * sizeof(MGHostSpan), sizeof(MGHostSpan)); - CheckHostSpanIsHonest(span); + // All four arms, including the one that needs the table: a producer that + // computed an offset wrongly is caught here rather than on a peer. + CheckHostSpanIsHonest(span, *m_segments); } } @@ -778,49 +932,56 @@ namespace MobileGL::MG_Remote::Wire { ++m_emitSeq; // The stage mark: where SEG_STAGE stood once everything this record names had been // staged. ReclaimStagedBytes releases up to the newest mark the server has retired. - if (m_stage != nullptr) { - m_stageMarks.push_back(StageMark{m_emitSeq, m_stage->LocalHead()}); - } + m_stageMarks.push_back(StageMark{m_emitSeq, m_stageHead}); return m_emitSeq; } void PipeWireEncoder::ReclaimStagedBytes() { - if (m_stage == nullptr || m_control == nullptr) { + if (m_control == nullptr) { return; } const Uint64 retired = m_control->retiredSeq.load(std::memory_order_acquire); - Uint64 upTo = 0; - Bool found = false; + Uint64 upTo = m_stageTail; while (m_stageMarkFront < m_stageMarks.size() && m_stageMarks[m_stageMarkFront].Seq <= retired) { upTo = m_stageMarks[m_stageMarkFront].StageCursor; - found = true; ++m_stageMarkFront; } - // Compact rather than erase-from-front on every call: the list is at most as long as - // the number of records in flight, which the verb barrier keeps at one or two. - if (m_stageMarkFront != 0 && m_stageMarkFront == m_stageMarks.size()) { + + // COMPACT ON A THRESHOLD, NOT ONLY ON A FULL DRAIN. The queue used to be cleared only + // when front reached size(), so any pipelining deeper than "fully drained at every + // reclaim" - which is precisely the regime this design exists to survive when R-1's + // barrier retires family by family - left front < size() for ever and grew the vector + // 16 bytes per encoded record for the life of the context. Erasing the consumed prefix + // once it is half the queue is amortised O(1) and bounds the storage at twice the + // records actually in flight. + if (m_stageMarkFront == m_stageMarks.size()) { m_stageMarks.clear(); m_stageMarkFront = 0; + } else if (m_stageMarkFront != 0 && m_stageMarkFront * 2 >= m_stageMarks.size()) { + m_stageMarks.erase(m_stageMarks.begin(), + m_stageMarks.begin() + static_cast(m_stageMarkFront)); + m_stageMarkFront = 0; } - if (!found || upTo <= m_stageReclaimed) { - return; + + // SEG_STAGE is CLIENT-OWNED memory (contract table 1: "client stages, server copies") + // and the server only reads it, so the client is both the allocator and the thing that + // frees. What it may not do is free ahead of retiredSeq - the whole content of R-11 on + // this side - and what it may ALSO not do is write RingControl's stage tails, which + // Ring.h makes consumer-owned. So the reclaim watermark is this local counter and the + // shared triple is untouched. + if (upTo > m_stageTail) { + m_stageTail = upTo; } - m_stageReclaimed = upTo; - // SEG_STAGE is CLIENT-OWNED memory (contract table 1: "client stages, server copies"), - // and the server only ever reads it, so the client is both the producer and the thing - // that frees. What it may not do is free ahead of retiredSeq, which is the whole - // content of R-11 on this side. - m_control->stageAppliedTail.store(upTo, std::memory_order_release); - m_control->stageRetiredTail.store(upTo, std::memory_order_release); } - Uint64 PipeWireEncoder::StagedBytesInFlight() const { - if (m_stage == nullptr) { - return 0; - } - return m_stage->LocalHead() - m_stageReclaimed; - } + Uint64 PipeWireEncoder::StagedBytesInFlight() const { return m_stageHead - m_stageTail; } + + // THE STORAGE, NOT THE LIVE COUNT. `size() - front` is the number of marks still in + // flight, and it stays at one or two even while the vector behind it grows for ever - so a + // control written against it would have gone green through exactly the leak it was meant + // to catch. What leaks is the container, so that is what this reports. + SizeT PipeWireEncoder::StageMarksHeld() const { return m_stageMarks.size(); } void PipeWireEncoder::Publish() { if (m_cmd == nullptr) { @@ -830,13 +991,21 @@ namespace MobileGL::MG_Remote::Wire { // notify-then-publish loses the wakeup. The doorbell itself belongs to the SESSION // (s1) - the codec does not own a Doorbell and must not, or a unit case could not // drive encoder -> ring -> decoder without one. - if (m_stage != nullptr) { - m_stage->Publish(); - } + // + // SEG_STAGE needs no publish: the decoder reads those bytes by OFFSET, never by + // popping a ring, and the release store on SEG_CMD's head below is what orders the + // staged writes before the record that names them. m_cmd->Publish(); if (m_control != nullptr) { + // submittedSeq is the one watermark the PRODUCER owns (Ring.h's five-watermark + // block). Nobody waits on it; it answers "how far ahead of the server is the + // client right now". m_control->submittedSeq.store(m_emitSeq, std::memory_order_release); } + // The reclaim has a trigger in this package, rather than depending on a c1 barrier + // that does not exist yet: every publish is a chance to notice what the server has + // already retired, and it costs one acquire load. + ReclaimStagedBytes(); } Uint64 PipeWireEncoder::EmitSeq() const { return m_emitSeq; } @@ -851,8 +1020,18 @@ namespace MobileGL::MG_Remote::Wire { ReplySink* replies) : m_control(control), m_segments(segments), m_replies(replies) { m_auditPoison = MG_Config::Ipc.Audit; + // Once, at construction, beside the resolver it is modelled on - not per record from + // the apply thread. The thunk is inert without a decoder on the calling thread, so an + // early install changes nothing for a monolith caller of MGPipeApplyWireRecord. + InstallApplyHook(); } + void PipeWireDecoder::InstallApplyHook() { + MG_Pipe::gMGPipeWireRecordApply = &MGPipeWireRecordApplyThunk; + } + + void PipeWireDecoder::UninstallApplyHook() { MG_Pipe::gMGPipeWireRecordApply = nullptr; } + Bool PipeWireDecoder::Valid() const { return m_control != nullptr && m_segments != nullptr; } Uint64 PipeWireDecoder::AppliedSeq() const { return m_applySeq; } @@ -867,6 +1046,34 @@ namespace MobileGL::MG_Remote::Wire { Uint64 PipeWireDecoder::PoisonedStageBytes() const { return m_poisonedBytes; } + Bool PipeWireDecoder::LastAcceptanceKnown() const { return m_lastAcceptanceKnown; } + + Bool PipeWireDecoder::LastAcceptance() const { return m_lastAcceptance; } + + Uint64 PipeWireDecoder::AcceptedRecords() const { return m_accepted; } + + Uint64 PipeWireDecoder::DeclinedRecords() const { return m_declined; } + + void PipeWireDecoder::PostReply(MGPWireOp op, Uint64 seq, Int32 status, const void* bytes, + Uint64 size) { + // THE ONE GATE ON SEG_REPLY. s1 sizes ReplyPool from MGPipeCallFlagsFor - table 0 says + // that table is what "every package" reads - so writing SEG_REPLY[seq % slots] for a + // record the pool reserved no slot for silently overwrites a waiter's answer. And + // because the slot header stamps the WRITER's seq for self-check, the waiter's check + // then fails for ever: the barrier HANGS rather than returning something wrong, which + // is the harder failure to diagnose of the two. + if ((MGPipeCallFlagsFor(op) & static_cast(kReplySlot)) == 0) { + MGLOG_F("MGPipe: Fatal{ProtocolCorruption, \"%s\"} the decoder tried to answer into " + "a reply slot for a call the catalogue gives no kReplySlot; s1 sizes " + "ReplyPool from MGPipeCallFlagsFor and reserved none", + WireOpName(op)); + std::abort(); + } + if (m_replies != nullptr) { + m_replies->PostReply(seq, status, bytes, size); + } + } + Bool MGPipeWireRecordApplyThunk(MGPWireOp op, const void* record, Uint64 size, Uint64 remaining) { (void)remaining; if (t_activeDecoder == nullptr) { @@ -875,13 +1082,21 @@ namespace MobileGL::MG_Remote::Wire { return t_activeDecoder->ApplyChecked(op, record, size); } - void PipeWireDecoder::NoteResolvedRun(const MGPBlobRef& blob) { + void PipeWireDecoder::NoteResolvedRun(MGPWireOp op, const MGPBlobRef& blob) { if (blob.Size == 0 || blob.Seg != kSegStage) { return; } - if (m_resolvedCount < sizeof(m_resolved) / sizeof(m_resolved[0])) { - m_resolved[m_resolvedCount++] = blob; + 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 + // that quietly stopped covering a run is the same failure as no poison at all - + // rule C's only mechanical control going dark without a line in the log. + MGLOG_F("MGPipe: Fatal{ProtocolCorruption, \"%s\"} more than %llu SEG_STAGE runs in " + "one record; the audit fill would stop covering them", + WireOpName(op), + static_cast(sizeof(m_resolved) / sizeof(m_resolved[0]))); + std::abort(); } + m_resolved[m_resolvedCount++] = blob; } const void* PipeWireDecoder::ResolveOrFatal(MGPWireOp op, const MGPBlobRef& blob) { @@ -893,7 +1108,7 @@ namespace MobileGL::MG_Remote::Wire { // way: there is no recovery from a segment that stopped covering its own runs. WireProtocolFatalAt("segment-resolve", blob.Offset, blob.Size); } - NoteResolvedRun(blob); + NoteResolvedRun(op, blob); return bytes; } @@ -905,7 +1120,12 @@ namespace MobileGL::MG_Remote::Wire { for (Uint32 i = 0; i < m_resolvedCount; ++i) { const MGPBlobRef& blob = m_resolved[i]; const SegmentView view = m_segments->Get(kSegStage); - if (view.Base == nullptr || blob.Offset + blob.Size > view.Size) { + // The SUBTRACTION form, the same one Resolve uses, so the addition cannot wrap. + // Every run here has already been through Resolve, so this is unreachable today - + // but it is the one place in the file where a wrap would be an arbitrary 0xDD + // memset, and "unreachable" is not a reason to write the weaker test. + if (view.Base == nullptr || blob.Offset > view.Size || + blob.Size > view.Size - blob.Offset) { continue; } // R-2.5. The record has been applied and its bytes are retired, so an applier that @@ -937,13 +1157,14 @@ namespace MobileGL::MG_Remote::Wire { const MGPWireOp op = static_cast(record.kind); m_resolvedCount = 0; + m_lastAcceptanceKnown = false; PipeWireDecoder* previous = t_activeDecoder; t_activeDecoder = this; // The generated gate first, ALWAYS: MGPipeApplyWireRecord owns the per-opcode // `size >= sizeof(MGPWireRec_X)` check, because that is the half that follows from the // opcode alone and therefore belongs to the generator. It then calls back into - // ApplyChecked through the hook, which owns the half that needs the payload. - MG_Pipe::gMGPipeWireRecordApply = &MGPipeWireRecordApplyThunk; + // ApplyChecked through the hook, which owns the half that needs the payload. The hook + // was installed once, at construction. const Bool applied = MG_Pipe::MGPipeApplyWireRecord(op, base, size, size); t_activeDecoder = previous; @@ -1008,12 +1229,20 @@ namespace MobileGL::MG_Remote::Wire { }; // The four Bool-returning appliers answer ACCEPTANCE, not "applied" (R-5: the client // may not re-derive it, because an if-constexpr discard, a stale handle and a refused - // record are all invisible from the call site). The answer rides the reply slot the - // record's seq already names, so no payload of theirs needs an MGPReplySlot member. - const auto postAcceptance = [&](Bool accepted) { - if (m_replies != nullptr) { - m_replies->PostReply(seq, accepted ? ReplySink::kStatusOk : ReplySink::kStatusDeclined, - nullptr, 0); + // record are all invisible from the call site). + // + // IT DOES NOT GO IN A REPLY SLOT. None of the four carries kReplySlot, and s1 sizes + // ReplyPool from that table - see PostReply. The answer is recorded here and read + // through LastAcceptance() / Accepted+DeclinedRecords() until the integrator rules on + // which half of the contract moves (table 0 says these four use DECLINED; the + // catalogue gives them no slot). + const auto noteAcceptance = [&](Bool accepted) { + m_lastAcceptanceKnown = true; + m_lastAcceptance = accepted; + if (accepted) { + ++m_accepted; + } else { + ++m_declined; } }; @@ -1037,7 +1266,7 @@ namespace MobileGL::MG_Remote::Wire { // ---- resources ------------------------------------------------------------------- case MGPWireOp::ResourceCreate: - postAcceptance(MGPipeApplyResourceCreate(*static_cast(payload))); + noteAcceptance(MGPipeApplyResourceCreate(*static_cast(payload))); return true; case MGPWireOp::ResourceRespecify: { @@ -1056,7 +1285,7 @@ namespace MobileGL::MG_Remote::Wire { level.Level = MGPipeRespecifiedLevelOf(desc); scope = &level; } - postAcceptance(MGPipeApplyResourceRespecify(desc, nullptr, scope)); + noteAcceptance(MGPipeApplyResourceRespecify(desc, nullptr, scope)); return true; } @@ -1074,9 +1303,7 @@ 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). - if (m_replies != nullptr) { - m_replies->PostReply(seq, ReplySink::kStatusDeclined, nullptr, 0); - } + PostReply(op, seq, ReplySink::kStatusDeclined, nullptr, 0); return true; case MGPWireOp::UnmapPersistent: @@ -1297,7 +1524,10 @@ namespace MobileGL::MG_Remote::Wire { MGHostSpan span{}; std::memcpy(&span, reinterpret_cast(spans) + i * sizeof(MGHostSpan), sizeof(MGHostSpan)); - CheckHostSpanIsHonest(span); + // ALL FOUR ARMS, the segment-range one included: this row and DrawVbo are + // the only two host-span carriers in the catalogue, and an out-of-segment + // span used to pass every check here. + CheckHostSpanIsHonest(span, *m_segments); } } return false; @@ -1366,7 +1596,7 @@ namespace MobileGL::MG_Remote::Wire { } case MGPWireOp::SetTextureParams: - postAcceptance(MGPipeApplySetTextureParams(*static_cast(payload))); + noteAcceptance(MGPipeApplySetTextureParams(*static_cast(payload))); return true; // ---- transfer --------------------------------------------------------------------- @@ -1378,16 +1608,31 @@ namespace MobileGL::MG_Remote::Wire { // to compute") and under split it must declare too - a length the reader computes // from the record it is checking is not a bounds check. const Bool namesABuffer = rec.Target == kMGPipeResourceTargetBuffer; - const Bool carriesContent = - namesABuffer ? MGPipeSubDataBufferSize(rec) != 0 - : (rec.UnionBox.W != 0 && rec.UnionBox.H != 0 && rec.UnionBox.D != 0); + // THE TEXTURE PREDICATE, TIGHTENED. It used to be "all three extents non-zero", + // which read a 4x4x0 box as carrying nothing and let rule A's arm 2 sit out - so a + // record could describe a real destination and declare no bytes. A box is either + // EMPTY (every extent zero, which is how a pull that needs nothing is spelled) or + // WHOLE; a partially-zero extent is neither, and no emitter produces one. + const Bool boxIsEmpty = + rec.UnionBox.W == 0 && rec.UnionBox.H == 0 && rec.UnionBox.D == 0; + const Bool boxIsWhole = + rec.UnionBox.W != 0 && rec.UnionBox.H != 0 && rec.UnionBox.D != 0; + if (!namesABuffer && !boxIsEmpty && !boxIsWhole) { + MGLOG_F("MGPipe: Fatal{ProtocolCorruption, \"ResourceSubData\"} the union box " + "%ux%ux%u has a zero extent on some axes and not others; a box is " + "either empty or whole", + rec.UnionBox.W, rec.UnionBox.H, rec.UnionBox.D); + std::abort(); + } + const Bool carriesContent = namesABuffer ? MGPipeSubDataBufferSize(rec) != 0 + : (boxIsWhole || rec.RegionCount != 0); const void* bytes = nullptr; if (carriesContent) { bytes = ResolveOrFatal(op, rec.Blob); } else { CheckBlobIsHonest(op, rec.Blob, *m_segments); } - postAcceptance(MGPipeApplyResourceSubData( + noteAcceptance(MGPipeApplyResourceSubData( rec, bytes, reinterpret_cast(tailAt(0)))); return true; } @@ -1425,9 +1670,7 @@ namespace MobileGL::MG_Remote::Wire { // 22) - the destination is the client's shadow and the size is the resource's, not // a fixed slot's - so the reply slot carries COMPLETION only. MGPipeApplyResourceReadback(*static_cast(payload)); - if (m_replies != nullptr) { - m_replies->PostReply(seq, ReplySink::kStatusOk, nullptr, 0); - } + PostReply(op, seq, ReplySink::kStatusOk, nullptr, 0); return true; case MGPWireOp::ResourceCopyRegion: @@ -1466,7 +1709,11 @@ namespace MobileGL::MG_Remote::Wire { sizeof(MGHostSpan)); } std::memcpy(&span, tailAt(1), sizeof(span)); - CheckHostSpanIsHonest(span); + // ALL FOUR ARMS. WireVerbSink's header promises OnDrawVbo "a DECODED, + // VALIDATED argument list"; without the segment-range arm a span whose run + // left SEG_STAGE reached the sink and that promise was false. P8 is what arms + // this path, which is exactly when nobody will be reading this code. + CheckHostSpanIsHonest(span, *m_segments); userIndices = &span; } return m_verbs != nullptr && diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.h b/MobileGL/MG_Remote/Wire/PipeWireCodec.h index 80124f0d..1f87f2df 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.h +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.h @@ -131,10 +131,21 @@ namespace MobileGL::MG_Remote::Wire { // declared. Fatal on an absent blob, then CheckBlobIsHonest on a present one. void RequireDeclaredBlob(MG_Pipe::MGPWireOp op, const MG_Pipe::MGPBlobRef& blob, const SegmentTable& segments); - // R-2.3 arm for MGHostSpan. P5's reduced path should produce ZERO host spans + // R-2.3 arms 1 and 3 for MGHostSpan. P5's reduced path should produce ZERO host spans // (kCapNeedsHostIndexBytes / kCapNeedsHostUboBytes are both 0 in P5, table 0), so this // firing at all is a finding, not just a corruption check. + // + // IT CANNOT DO ARM 4 - it has no segment table - so a span that names a real segment and a + // run PAST THE END OF IT passes this function. Use the overload below on any path that has + // a table; this one exists because c0 shipped the signature and other packages compile + // against it. void CheckHostSpanIsHonest(const MG_Pipe::MGHostSpan& span); + // All four arms. Arm 4 is the one the signature above cannot express: a span is only + // honest if its Offset+Size actually lies inside the segment it names, and the promise + // WireVerbSink's header makes - that OnDrawVbo is handed a VALIDATED argument list - is + // false without it. Latent in P5 (nothing emits a span) and armed at P8, which is exactly + // when nobody will be reading this file. + void CheckHostSpanIsHonest(const MG_Pipe::MGHostSpan& span, const SegmentTable& segments); // ---- one record's shape, computed ONCE and read by both sides ---------------------- // @@ -222,18 +233,37 @@ namespace MobileGL::MG_Remote::Wire { const WireTail* tails, Uint32 tailCount); // Releases every SEG_STAGE run named by a record the apply side has RETIRED - // (RingControl::retiredSeq, R-9). Called by the client at its verb barrier and - // whenever StageBytes runs short; the allocator reclaims behind retiredSeq and - // nothing else may (table 1's "retires" column, R-11). + // (RingControl::retiredSeq, R-9, which this class only ever READS). Called from + // Publish() and whenever StageBytes runs short, so nothing outside this package has to + // remember to; the allocator reclaims behind retiredSeq and nothing else may + // (table 1's "retires" column, R-11). // // THE MARK IS HELD ON THIS SIDE, NOT ON THE WIRE. A record does not carry where its // staged bytes end, so the encoder remembers {seq, stage cursor} per record and // reclaims to the newest mark whose seq the server has retired. That is exact, needs // no wire field, and does not depend on the verb barrier - so it keeps working when // R-1's barrier retires family by family. + // + // SEG_STAGE'S CURSOR TRIPLE IN RingControl IS NOT USED BY EITHER SIDE IN P5, and this + // is the reason. Ring.h makes stageAppliedTail / stageRetiredTail CONSUMER-owned, the + // server never Pops the stage ring (the decoder resolves by offset), and a producer + // that wrote those cursors would be the very shape R-9 forbids one segment over. So + // the allocator below is entirely encoder-local and the triple is left at its + // InitRingControl values. **s1 must not attach a RingConsumer to + // RingCursorSet::Stage**: its m_localTail would never move, and PublishApplied / + // PublishRetired would then walk both cursors BACKWARDS under this allocator. void ReclaimStagedBytes(); // Staged bytes not yet reclaimed. The number MOBILEGL_IPC_STAGE_MB has to cover. Uint64 StagedBytesInFlight() const; + // The {seq, cursor} marks the queue is STILL STORING, consumed ones included - not the + // number in flight. Bounded by twice the records in flight, and exposed so a case can + // assert that bound rather than trust the comment, because an unbounded queue here is + // a steady-state leak no SSIM comparison and no two-scenario lane would ever see. + // + // It reports the storage deliberately: the in-flight count stays at one or two while + // the container behind it grows for ever, so a control written against that number + // goes green through exactly the leak it exists to catch. + SizeT StageMarksHeld() const; // Release-stores the head cursor, then rings the consumer doorbell IF PARKED. The // order is pinned by RingTest.cpp:446 and must not be swapped: notify-then-publish @@ -254,6 +284,12 @@ namespace MobileGL::MG_Remote::Wire { Uint64 StageCursor = 0; }; + // The SEG_STAGE linear allocator (BRIEF §5 w1's own wording). Monotonic byte counters + // over the segment; the in-segment offset is `cursor % capacity` and a run that would + // straddle the end skips to the boundary, exactly as a ring does, but WITHOUT touching + // RingControl - see ReclaimStagedBytes above. + Uint8* StageAllocate(Uint64 size); + Transport::RingControl* m_control = nullptr; Transport::RingProducer* m_cmd = nullptr; Transport::RingProducer* m_stage = nullptr; @@ -262,7 +298,10 @@ namespace MobileGL::MG_Remote::Wire { Uint64 m_maxRecordBytes = 0; Vector m_stageMarks; SizeT m_stageMarkFront = 0; - Uint64 m_stageReclaimed = 0; + Uint8* m_stageBase = nullptr; + Uint64 m_stageCapacity = 0; + Uint64 m_stageHead = 0; // monotonic bytes allocated + Uint64 m_stageTail = 0; // monotonic bytes reclaimed }; // ---- decoder ----------------------------------------------------------------------- @@ -394,13 +433,52 @@ namespace MobileGL::MG_Remote::Wire { // How many staged bytes this decoder has poisoned. Zero with the audit off, and the // number a t1 lane asserts is non-zero with it on: an instrumentation that cannot be // observed to have run is decoration. + // + // WHAT IT ACTUALLY COVERS is "the blob runs the arm resolved", which today is AT MOST + // ONE per record: tails live in SEG_CMD and are never noted, and CreateShaderState's + // six per-stage runs are Fatal rather than resolved, so the one archive is the only + // multi-kilobyte run in the catalogue that reaches it. The array is eight deep so a + // later phase that declares more can fill it without a code change - and overflowing + // it is Fatal rather than a silent drop, because a poison that quietly stopped + // covering a run is the same failure as no poison at all. Uint64 PoisonedStageBytes() const; + // R-5's acceptance answer for the four Bool-returning appliers - ResourceCreate, + // ResourceRespecify, ResourceSubData, SetTextureParams. + // + // IT DOES NOT RIDE A REPLY SLOT, and that is a contract conflict this package could + // not settle on its own. CONTRACT-P5.md table 0's reply-slot-header row says DECLINED + // "is how the four Bool acceptance entry points say false (R-5)", but PipeCalls.def + // gives none of those four `kReplySlot` - and table 0 ALSO says kMGPipeCallFlags is + // what "every package" reads, so s1 will size ReplyPool from it. Writing + // SEG_REPLY[seq % slots] for a record the pool never reserved a slot for overwrites a + // waiter's answer, and because the slot header stamps the writer's seq for self-check, + // the waiter's check then fails FOR EVER and the barrier hangs rather than returning + // something wrong. So the decoder posts a reply only for rows whose flags say + // kReplySlot (PostReply itself Fatals otherwise) and exposes the acceptance here + // instead. The integrator rules on which half of the contract moves. + Bool LastAcceptanceKnown() const; + Bool LastAcceptance() const; + Uint64 AcceptedRecords() const; + Uint64 DeclinedRecords() const; + + // Points MG_Pipe::gMGPipeWireRecordApply at this layer's thunk. Called once from the + // constructor and modelled on SegmentTable::InstallProcessResolver, which is the same + // shape for the same reason; Uninstall belongs beside that one at teardown. The thunk + // is inert without a decoder on the calling thread, so installing it early changes + // nothing for a monolith caller of MGPipeApplyWireRecord. + static void InstallApplyHook(); + static void UninstallApplyHook(); + private: Bool ApplyChecked(MG_Pipe::MGPWireOp op, const void* record, Uint64 size); const void* ResolveOrFatal(MG_Pipe::MGPWireOp op, const MG_Pipe::MGPBlobRef& blob); - void NoteResolvedRun(const MG_Pipe::MGPBlobRef& blob); + void NoteResolvedRun(MG_Pipe::MGPWireOp op, const MG_Pipe::MGPBlobRef& blob); void PoisonResolvedRuns(); + // The ONLY way this class answers a record. Fatals if `op` carries no kReplySlot - + // see LastAcceptanceKnown() for why that is a Fatal and not a log line. + void PostReply(MG_Pipe::MGPWireOp op, Uint64 seq, Int32 status, const void* bytes, + Uint64 size); friend Bool MGPipeWireRecordApplyThunk(MG_Pipe::MGPWireOp, const void*, Uint64, Uint64); @@ -411,14 +489,19 @@ namespace MobileGL::MG_Remote::Wire { Uint64 m_applySeq = kInvalidSeq; Bool m_auditPoison = false; Uint64 m_poisonedBytes = 0; - // The SEG_STAGE runs the record being applied resolved, for the 0xDD fill. At most - // seven (CreateShaderState's blob members) plus one tail. + Bool m_lastAcceptanceKnown = false; + Bool m_lastAcceptance = false; + Uint64 m_accepted = 0; + Uint64 m_declined = 0; + // The SEG_STAGE runs the record being applied resolved, for the 0xDD fill. Eight deep + // so CreateShaderState's seven blob members plus a tail would fit if a later phase + // declares them; today at most one run is ever noted (see PoisonedStageBytes). MG_Pipe::MGPBlobRef m_resolved[8]; Uint32 m_resolvedCount = 0; }; // The hook MGPipeApplyWireRecord dispatches to once its generated per-opcode bounds gate - // has passed. Installed by DecodeAndApply on the thread that decodes. + // has passed. Inert unless a decoder is active on the calling thread. Bool MGPipeWireRecordApplyThunk(MG_Pipe::MGPWireOp op, const void* record, Uint64 size, Uint64 remaining); diff --git a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp index 00d036f5..e29613c1 100644 --- a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp +++ b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp @@ -371,6 +371,19 @@ TEST_F(PipeWireCodecTest, AVarTailRecordIsNotSkippedAsAWrapFiller) { // anywhere on this ring. This case is the trip wire for that. static_assert(static_cast(kVarTail) == static_cast(Transport::kRecPad), "the collision this case exists for is gone; keep the case anyway"); + // And the third one, which the first round missed and a reviewer found: it bites in the + // REVERSE direction, because the encoder stamps kRecVarTail on nine opcodes and anyone + // trusting PipeWire.inc's "MGPipeCallFlags of the call" reads those nine as kReplySlot. + static_assert(static_cast(kReplySlot) == static_cast(Transport::kRecVarTail), + "kReplySlot and kRecVarTail alias"); + // Five of the six call-flag bits alias a ring bit; only kOptional is free, and only + // because RingRecordFlags has not reached 1<<5. + static_assert((static_cast(kNeedsAck | kHasBlob | kVarTail | kHostSpan | kReplySlot | + kOptional) & + static_cast(Transport::kRecNeedsAck | Transport::kRecHasBlob | + Transport::kRecPad | Transport::kRecBorrowSlot | + Transport::kRecVarTail)) == 0x1Fu, + "the overlap between the two flag spaces moved"); Wire2 wire; MGPVertexBuffers header{}; @@ -530,10 +543,12 @@ TEST_F(PipeWireCodecTest, KNeedsAckRespecifyCarriesItsRedefinitionScope) { kInvalidSeq); ASSERT_TRUE(wire.PumpOne(&applied)); EXPECT_TRUE(applied); - // Two records, two acceptance answers, on their own seqs (R-3: the seq IS the id). - ASSERT_EQ(wire.Answers().All.size(), 2u); - EXPECT_EQ(wire.Answers().All[0].Seq, 1u); - EXPECT_EQ(wire.Answers().All[1].Seq, 2u); + // Two records, two ACCEPTANCE answers - and NOT in a reply slot. Neither ResourceCreate + // nor ResourceRespecify carries kReplySlot, and s1 sizes ReplyPool from that table, so a + // reply written here would overwrite some waiter's slot. + EXPECT_TRUE(wire.Answers().All.empty()); + EXPECT_EQ(wire.Decoder().AcceptedRecords() + wire.Decoder().DeclinedRecords(), 2u); + EXPECT_TRUE(wire.Decoder().LastAcceptanceKnown()); } TEST_F(PipeWireCodecTest, KOptionalUnmapPersistentRoundTrips) { @@ -726,7 +741,7 @@ TEST_F(PipeWireCodecTest, ResourceSubDataCarriesABlobAndARegionTailTogether) { kInvalidSeq); ASSERT_TRUE(wire.PumpOne(&applied)); EXPECT_TRUE(applied); - ASSERT_EQ(wire.Answers().All.size(), 2u); + EXPECT_TRUE(wire.Answers().All.empty()); // ACCEPTANCE IS NOT "APPLIED", and this case is where the difference shows. The record // crossed and reached MGPipeApplyResourceSubData, which is the codec's whole job; the // applier then DECLINED it, because no backend registered a P4a texture consumer in this @@ -734,8 +749,10 @@ TEST_F(PipeWireCodecTest, ResourceSubDataCarriesABlobAndARegionTailTogether) { // into lost texels on Magma). That is exactly the answer R-5 says must travel rather than // be re-derived on the client: a client that cleared its dirty flags on the strength of // having EMITTED would drop these texels for good. - EXPECT_EQ(wire.Answers().All[1].Seq, 2u); - EXPECT_EQ(wire.Answers().All[1].Status, ReplySink::kStatusDeclined); + EXPECT_TRUE(wire.Decoder().LastAcceptanceKnown()); + EXPECT_FALSE(wire.Decoder().LastAcceptance()); + // Both records answered - the texture create is declined by the same NoP4aConsumer belt. + EXPECT_EQ(wire.Decoder().AcceptedRecords() + wire.Decoder().DeclinedRecords(), 2u); // And the texels themselves crossed intact: the decline is the applier's, not the wire's. const void* staged = wire.Segments().Resolve(upload.Blob.Seg, upload.Blob.Offset, upload.Blob.Size); @@ -1051,11 +1068,12 @@ TEST_F(PipeWireCodecTest, TheDecoderWritesNoRingControlFieldOfItsOwn) { TEST_F(PipeWireCodecTest, MaxRecordBytesSeenStaysFarBelowHalfTheRing) { // R-10's proof obligation. P5 does no chunking and must instead show it never needed any. // - // THE CAP IS ASKED FOR AT RUNTIME AND NEVER DERIVED FROM MOBILEGL_IPC_RING_MB. s1 found - // that SEG_CMD's 8 MiB holds a 4096-byte control page plus a POWER-OF-TWO ring, so the - // ring is 4 MiB and MaxRecordBytes() is 2 MiB - half of what both CONTRACT-P5.md §5 and - // Config.h's comment say. Every comparison in the codec goes through - // RingProducer::MaxRecordBytes() for exactly this reason. + // THE CAP IS ASKED FOR AT RUNTIME AND NEVER DERIVED FROM MOBILEGL_IPC_RING_MB, and this + // phase is why: the number moved twice in one afternoon. s1 first found that the control + // page came out of the segment (making the cap 2 MiB at the default), then fixed it the + // other way round - MOBILEGL_IPC_RING_MB now names the RING and the segment adds a page on + // top - so the cap is 4 MiB again and the contract is true as written. Nothing in the + // codec changed either time, because every comparison goes through MaxRecordBytes(). Wire2 wire; MGPDrawInfo info{}; info.NumDraws = 64; @@ -1119,6 +1137,102 @@ TEST_F(PipeWireCodecTest, StagedBytesAreReclaimedOnlyBehindRetiredSeq) { EXPECT_EQ(wire.Encoder().StagedBytesInFlight(), 0u); } +// ---- M2 / M3: SEG_STAGE's cursors and the mark queue -------------------------------------- + +TEST_F(PipeWireCodecTest, TheEncoderNeverWritesSegStagesConsumerCursors) { + // Ring.h makes stageAppliedTail / stageRetiredTail CONSUMER-owned and says the staging + // allocator reclaims behind retiredSeq "and nothing else may". A producer writing them is + // the same shape w1's own §8.1 argues against for appliedSeq, one segment over: if s1 ever + // attaches a RingConsumer to RingCursorSet::Stage - the obvious thing to do for a segment + // with a cursor triple - its m_localTail never moves (nothing Pops the stage ring) and the + // two cursors get walked forward by the client and back by the server. + Wire2 wire; + const std::uint8_t payload[128] = {}; + (void)wire.Encoder().StageBytes(payload, sizeof(payload)); + MGPBindRenderState bind{}; + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::BindRenderState, &bind, sizeof(bind)), + kInvalidSeq); + bool applied = false; + ASSERT_TRUE(wire.PumpOne(&applied)); + wire.Encoder().ReclaimStagedBytes(); + EXPECT_EQ(wire.Encoder().StagedBytesInFlight(), 0u) << "the reclaim did not run at all"; + + EXPECT_EQ(wire.Control().stageHead.load(), 0u); + EXPECT_EQ(wire.Control().stageAppliedTail.load(), 0u); + EXPECT_EQ(wire.Control().stageRetiredTail.load(), 0u); +} + +TEST_F(PipeWireCodecTest, TheStageMarkQueueStaysBoundedWithOneRecordAlwaysInFlight) { + // The steady-state leak: the queue used to be cleared ONLY when it drained completely, so + // one unretired record at every reclaim meant front < size() for ever and 16 bytes per + // encoded record for the life of the context. That is exactly the regime W-3 says the + // design exists to survive once R-1's barrier retires family by family - and no SSIM + // comparison and no two-scenario lane would ever see it. + Wire2 wire; + MGPBindRenderState bind{}; + const std::uint8_t blob[32] = {}; + + // PRIMING IS THE WHOLE POINT. Encoding and applying one record per iteration drains the + // queue at every reclaim, which is the one regime the old "clear only when front reaches + // size()" code handled - a case written that way stays green against the bug. One record + // encoded ahead of the one being applied is what makes `front < size()` permanent. + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::BindRenderState, &bind, sizeof(bind)), + kInvalidSeq); + for (int i = 0; i < 4000; ++i) { + (void)wire.Encoder().StageBytes(blob, sizeof(blob)); + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::BindRenderState, &bind, sizeof(bind)), + kInvalidSeq); + bool applied = false; + ASSERT_TRUE(wire.PumpOne(&applied)) << "record " << i; + } + EXPECT_EQ(wire.Encoder().EmitSeq(), 4001u); + EXPECT_EQ(wire.Decoder().AppliedSeq(), 4000u) << "one record must still be in flight"; + // Bounded by twice the records actually in flight, not by the records ever encoded. + EXPECT_LE(wire.Encoder().StageMarksHeld(), 4u) + << "the mark queue is tracking history rather than flight"; +} + +// ---- M4: the reply slot is for kReplySlot rows only ---------------------------------------- + +TEST_F(PipeWireCodecTest, NoReplyIsWrittenForARowTheCatalogueGivesNoReplySlot) { + // s1 sizes ReplyPool from MGPipeCallFlagsFor, so a reply written for a record the pool + // reserved no slot for overwrites a waiter's answer - and because the slot header stamps + // the WRITER's seq for self-check, the waiter's check then fails for ever and the barrier + // HANGS rather than returning something wrong. + Wire2 wire; + ASSERT_EQ(MGPipeCallFlagsFor(MGPWireOp::ResourceCreate) & static_cast(kReplySlot), 0u); + ASSERT_EQ(MGPipeCallFlagsFor(MGPWireOp::SetTextureParams) & static_cast(kReplySlot), 0u); + + MGPResourceDesc create{}; + create.Resource = MakeHandle(131); + create.Target = static_cast(MGPipeResourceTarget::Buffer); + create.Width = 64; + create.Height = 1; + create.Depth = 1; + create.ArrayLayers = 1; + create.Levels = 1; + create.Samples = 1; + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::ResourceCreate, &create, sizeof(create)), + kInvalidSeq); + bool applied = false; + ASSERT_TRUE(wire.PumpOne(&applied)); + EXPECT_TRUE(applied); + + EXPECT_TRUE(wire.Answers().All.empty()) << "a reply slot was written for a kNone row"; + // The acceptance answer R-5 requires is still produced - it just does not ride SEG_REPLY. + EXPECT_TRUE(wire.Decoder().LastAcceptanceKnown()); + EXPECT_EQ(wire.Decoder().AcceptedRecords() + wire.Decoder().DeclinedRecords(), 1u); +} + +TEST_F(PipeWireCodecTest, EveryRowThatDoesWriteAReplyCarriesKReplySlot) { + // The other half: the three rows in P5 that answer into a slot all carry the flag, so + // PostReply's gate cannot be firing on any of them. + for (const MGPWireOp op : + {MGPWireOp::MapPersistent, MGPWireOp::ResourceReadback, MGPWireOp::ReadPixels}) { + EXPECT_NE(MGPipeCallFlagsFor(op) & static_cast(kReplySlot), 0u) << WireOpName(op); + } +} + TEST_F(PipeWireCodecTest, TheAuditFillOverwritesExactlyTheRunsTheRecordResolved) { // R-2.5, the only mechanical control on rule C ("no applier entry point retains a pointer // past its return"). An instrumentation that cannot be observed to have run is decoration, @@ -1475,6 +1589,120 @@ TEST_F(PipeWireCodecTest, AHostSpanCountThatIsNeitherZeroNorCountIsFatal) { EXPECT_NE(r.Log.find("SetShaderBuffers.HostSpanCount"), std::string::npos) << r.Log; } +// ---- M1: R-2 arm 4 over MGHostSpan, on both host-span rows ------------------------------ + +TEST_F(PipeWireCodecTest, AHostSpanRunPastItsSegmentIsFatalOnSetShaderBuffers) { + // Before the fix this child exited 0: CheckHostSpanIsHonest took no SegmentTable and no + // caller resolved, so arm 4 was implemented for blobrefs only. The span names a REAL + // segment and a run that leaves it. + const ChildResult r = RunInChild([] { + Wire2 wire; + MGPShaderBuffers header{}; + header.Count = 1; + header.HostSpanCount = 1; + MGPBufferRange range{}; + MGHostSpan span{}; + span.Ptr = nullptr; // rule B satisfied, so only arm 4 can catch this + span.Seg = kSegStage; + span.Offset = Wire2::kStageBytes - 8; + span.Size = 1024; + std::vector tail(sizeof(range) + sizeof(span)); + std::memcpy(tail.data(), &range, sizeof(range)); + std::memcpy(tail.data() + sizeof(range), &span, sizeof(span)); + ForgeAndDecode(wire, MGPWireOp::SetShaderBuffers, &header, sizeof(header), tail.data(), + tail.size()); + }); + ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log; + EXPECT_NE(r.Log.find("Fatal{ProtocolCorruption, \"host-span\"}"), std::string::npos) << r.Log; + EXPECT_NE(r.Log.find("does not lie inside that segment (R-2.3 arm 4)"), std::string::npos) + << r.Log; +} + +TEST_F(PipeWireCodecTest, AHostSpanRunPastItsSegmentIsFatalOnDrawVbo) { + // The same span on the other host-span row - the one whose sink WireVerbSink's header + // promises is handed "a DECODED, VALIDATED argument list". Before the fix it reached + // OnDrawVbo. + const ChildResult r = RunInChild([] { + Wire2 wire; + MGPDrawInfo info{}; + info.Mode = 4; + info.IndexSize = 2; + info.Flags = kDrawHasUserIndices; + info.NumDraws = 1; + MGPDrawRange ranges[1] = {{0, 4, 0}}; + MGHostSpan span{}; + span.Ptr = nullptr; + span.Seg = kSegStage; + span.Offset = 0xFFFFFFFFull; + span.Size = 0xFFFFu; + // The layout realigns the span to 8 after a 12-byte MGPDrawRange, so the forged tail + // has to carry the same four pad bytes the encoder would. + std::vector tail(16 + sizeof(span), 0); + std::memcpy(tail.data(), ranges, sizeof(ranges)); + std::memcpy(tail.data() + 16, &span, sizeof(span)); + ForgeAndDecode(wire, MGPWireOp::DrawVbo, &info, sizeof(info), tail.data(), tail.size()); + }); + ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log; + EXPECT_NE(r.Log.find("does not lie inside that segment (R-2.3 arm 4)"), std::string::npos) + << r.Log; +} + +// ---- M5 / m8 / q2 ------------------------------------------------------------------------ + +TEST_F(PipeWireCodecTest, BufferSubDataResidentMayNotDeclareRegions) { + // Its catalogue row has no kVarTail and its applier takes no regions, so a non-zero + // RegionCount is a fault rather than a tail. The layout used to share ResourceSubData's + // arm, which REQUIRED 80 bytes of tail the arm then dropped. + const ChildResult r = RunInChild([] { + Wire2 wire; + MGPSubData rec{}; + rec.Res = MakeHandle(7); + rec.RegionCount = 2; + MGPipeSetSubDataBufferRange(rec, 0, 64); + rec.RegionCount = 2; // after the helper, which zeroes it + ForgeAndDecode(wire, MGPWireOp::BufferSubDataResident, &rec, sizeof(rec), nullptr, 0); + }); + ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log; + EXPECT_NE(r.Log.find("BufferSubDataResident.RegionCount"), std::string::npos) << r.Log; +} + +TEST_F(PipeWireCodecTest, TheEncoderRefusesThePerStageSpirvRunTheDecoderCallsFatal) { + // The rule lived only in the decoder, so an emitter that declared a per-stage run got a + // valid seq here and a Fatal on a peer that could only report "corrupt stream". + const ChildResult r = RunInChild([] { + Wire2 wire; + MG_State::GLState::LinkArtifacts link; + MG_State::GLState::SpirvArtifacts spirv; + Vector archive; + MG_State::GLState::EncodeProgramArtifacts(link, spirv, archive); + MGPProgramDesc desc{}; + desc.Cso = MakeHandle(3); + desc.Reflection = wire.Encoder().StageBytes(archive.data(), archive.size()); + const std::uint32_t words[4] = {1, 2, 3, 4}; + desc.Spirv[0] = wire.Encoder().StageBytes(words, sizeof(words)); + (void)wire.Encoder().EncodeRecord(MGPWireOp::CreateShaderState, &desc, sizeof(desc)); + }); + ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log; + EXPECT_NE(r.Log.find("declares 16 bytes at the ENCODER"), std::string::npos) << r.Log; +} + +TEST_F(PipeWireCodecTest, APartiallyZeroUploadBoxIsFatalRatherThanReadAsEmpty) { + // The content predicate used to be "all three extents non-zero", so a 4x4x0 box was read + // as carrying nothing and rule A's arm 2 sat out for a record that named a real + // destination. A box is either empty or whole. + const ChildResult r = RunInChild([] { + Wire2 wire; + MGPSubData rec{}; + rec.Res = MakeHandle(9); + rec.Target = MGPipePackSubDataTarget(static_cast(MGPipeResourceTarget::Tex2D), 0u); + rec.UnionBox = MGPBox{0, 0, 0, 4, 4, 0}; + ForgeAndDecode(wire, MGPWireOp::ResourceSubData, &rec, sizeof(rec), nullptr, 0); + }); + ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log; + EXPECT_NE(r.Log.find("has a zero extent on some axes and not others"), std::string::npos) + << r.Log; +} + TEST_F(PipeWireCodecTest, ASecondProcessResolverIsFatalRatherThanASilentRace) { // Table 3: one gMGPipeSegmentResolver per process, installed by the SERVER role only. const ChildResult r = RunInChild([] {