From 362daafeed3098de85ad42080261d8dd30596a41 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Tue, 29 Sep 2026 02:21:36 -0400 Subject: [PATCH] [Fix] (MG_Remote): the pack-buffer readbacks check their whole shape before the read and refuse it at one latch site per sink, each with a raw-peer control, and a verb counts as statically barriered only when every op that stamps it waits --- MobileGL/MG_Remote/Server/PipeApplier.cpp | 137 ++++++++++++---------- MobileGL/MG_Remote/Server/PipeApplier.h | 6 +- MobileGL/MG_Test/Wire/PeerLatchTest.cpp | 44 +++++++ scripts/ci/ph_latch_sites.py | 5 + scripts/gen_pipe_field_ownership.py | 66 +++++++---- 5 files changed, 174 insertions(+), 84 deletions(-) diff --git a/MobileGL/MG_Remote/Server/PipeApplier.cpp b/MobileGL/MG_Remote/Server/PipeApplier.cpp index 52caf639..0b78cd3f 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.cpp +++ b/MobileGL/MG_Remote/Server/PipeApplier.cpp @@ -553,28 +553,43 @@ namespace MobileGL::MG_Remote::Server { // bytes is the same silently wrong answer. So the refusal latches here, under the same family // words. + // Every number here is the peer's, and they are checked BEFORE the read, so a corrupt record + // costs no GPU work - and completely enough that LandReadbackInBuffer cannot meet a range it + // would have to refuse: every run it writes starts at or before the LAST row's offset and is + // no wider than the tight extent, and both are bounded here by what one MGPSubData can name + // (MGPipeSetSubDataBufferRange: an offset up to 2^31-1, a width up to 2^32-1). Whether the + // range fits the buffer is the applier's own gate, the one every buffer write meets. + static const char* ReadbackToBufferLayoutFault(const MG_Pipe::MGPReadbackToBuffer& record, + Uint64 bytesPerPixel) { + const MG_Pipe::MGPBox& box = record.Src.Box; + const Uint64 rowBytes = static_cast(box.W) * bytesPerPixel; + const Uint64 tightBytes = rowBytes * box.H * box.D; // the caller proved this fits + if (record.RowStride < rowBytes) return "the row stride is narrower than a tight row"; + if (box.D > 1 && record.ImageStride < record.RowStride) return "the image stride is narrower than a row"; + if (record.SwapGroup != 0 && record.SwapGroup != 2 && record.SwapGroup != 4 && record.SwapGroup != 8) + return "the swap group is not a byte group GL_PACK_SWAP_BYTES reverses (2, 4 or 8)"; + constexpr Uint64 kMaxOffset = 0x7FFFFFFFull; + const Uint64 lastImage = box.D - 1; + const Uint64 lastRow = box.H - 1; + if (record.DstBase > kMaxOffset) return "the first row starts past 2^31-1"; + if (lastImage != 0 && record.ImageStride > (kMaxOffset - record.DstBase) / lastImage) + return "the last image starts past 2^31-1"; + const Uint64 imageBase = record.DstBase + lastImage * record.ImageStride; + if (lastRow != 0 && record.RowStride > (kMaxOffset - imageBase) / lastRow) + return "the last row starts past 2^31-1"; + if (tightBytes > 0xFFFFFFFFull) return "the read is wider than one buffer write can name"; + return nullptr; + } + Bool ServerVerbSink::LandReadbackInBuffer(const MG_Pipe::MGPReadbackToBuffer& record, Uint8* tight, - Uint64 bytesPerPixel, const char* call) { + Uint64 bytesPerPixel) { const MG_Pipe::MGPBox& box = record.Src.Box; const Uint64 rowBytes = static_cast(box.W) * bytesPerPixel; const Uint64 rows = box.H; const Uint64 images = box.D; - const Uint64 tightBytes = rowBytes * rows * images; // the caller proved this fits - if (record.RowStride < rowBytes || (images > 1 && record.ImageStride < record.RowStride)) { - return SessionLatch(MGFatalFamily::ProtocolCorruption, "MGPipe: Fatal{ProtocolCorruption, \"%s.layout\"} row stride %llu " - "is narrower than a tight row (%llu) or image stride %llu narrower than a row; the " - "client's ReadbackLayout never produces either", - call, static_cast(record.RowStride), - static_cast(rowBytes), - static_cast(record.ImageStride)); - } + const Uint64 tightBytes = rowBytes * rows * images; if (record.SwapGroup != 0) { const Uint64 group = record.SwapGroup; - if (group != 2 && group != 4 && group != 8) { - return SessionLatch(MGFatalFamily::ProtocolCorruption, "MGPipe: Fatal{ProtocolCorruption, \"%s.SwapGroup\"} %llu is " - "not a byte group GL_PACK_SWAP_BYTES can reverse (2, 4 or 8)", - call, static_cast(group)); - } for (Uint64 at = 0; at + group <= tightBytes; at += group) { for (Uint64 i = 0; i < group / 2; ++i) std::swap(tight[at + i], tight[at + group - 1 - i]); } @@ -589,31 +604,19 @@ namespace MobileGL::MG_Remote::Server { const Uint64 runBytes = rowsTight ? (imagesTight ? tightBytes : rowBytes * rows) : rowBytes; const Uint64 runsPerImage = rowsTight ? 1 : rows; const Uint64 imageRuns = rowsTight && imagesTight ? 1 : images; - constexpr Uint64 kUint64Max = std::numeric_limits::max(); for (Uint64 z = 0; z < imageRuns; ++z) { for (Uint64 r = 0; r < runsPerImage; ++r) { - // DstBase + z * ImageStride + r * RowStride, checked: a peer can write any - // three numbers, and a wrapped offset would name bytes the range gate below - // then accepts. - if ((z != 0 && record.ImageStride > (kUint64Max - record.DstBase) / z) || - (r != 0 && record.RowStride > kUint64Max / r)) { - return Wire::WireProtocolLatch(call, "destination offset overflows"); - } - const Uint64 imageBase = record.DstBase + z * record.ImageStride; - if (r * record.RowStride > kUint64Max - imageBase) { - return Wire::WireProtocolLatch(call, "destination offset overflows"); - } - const Uint64 offset = imageBase + r * record.RowStride; + // No overflow and no refusal below: ReadbackToBufferLayoutFault bounded the last + // offset and the widest run before the read. + const Uint64 offset = record.DstBase + z * record.ImageStride + r * record.RowStride; MG_Pipe::MGPSubData write{}; write.Res = record.Dst; write.Target = MG_Pipe::kMGPipeResourceTargetBuffer; // HasLiveHostWrites stays 0 and that is the client's fact, not a default: the - // client takes this form only for a pack buffer it does not map (EmitTables.cpp, - // PackBufferReadbackIsAsync), so "no live host writer" is what a glBufferSubData - // of the same bytes would have said. - if (!MG_Pipe::MGPipeSetSubDataBufferRange(write, offset, runBytes)) { - return Wire::WireProtocolLatchAt(call, offset, 0x7FFFFFFFull); - } + // client takes this form only for a pack buffer it does not map + // (TextureReadbackEmit.inc, PackBufferReadbackTarget), so "no live host writer" is + // what a glBufferSubData of the same bytes would have said. + if (!MG_Pipe::MGPipeSetSubDataBufferRange(write, offset, runBytes)) return false; const Uint8* bytes = tight + (z * runsPerImage + r) * runBytes; ++m_bufferReadbackWrites; // The acceptance is the applier's (a dead handle is a counted no-op, a range @@ -630,35 +633,40 @@ namespace MobileGL::MG_Remote::Server { const Uint64 bpp = static_cast(MG_Util::GetInputBytesPerPixel( MG_Util::ConvertGLEnumToTextureInputFormat(static_cast(info.Format)), MG_Util::ConvertGLEnumToTexturePixelDataType(static_cast(info.Type)))); - // The reply form's shape checks, and its PH-3 bound: the scratch is sized from the - // record, so the SERVER's own maxReplyBytes caps it exactly as it caps an answer - the - // client bands this form by the same number (PlanReadbackBands), so no correct client - // is refused by it. + // The reply form's shape checks and its PH-3 bound: the scratch is sized from the record, + // so the SERVER's own maxReplyBytes caps it exactly as it caps an answer - the client bands + // this form by the same number (PlanReadbackBands), so no correct client is refused by it. + // ONE latch site for every shape fault (PeerLatchTest's row names it once). + constexpr Uint64 kUint64Max = std::numeric_limits::max(); + const char* fault = nullptr; + Uint64 tight = 0; if (bpp == 0 || info.Box.W == 0 || info.Box.H == 0 || info.Box.D != 1 || info.Box.W > static_cast(std::numeric_limits::max()) || info.Box.H > static_cast(std::numeric_limits::max())) { - return Wire::WireProtocolLatch("ReadPixelsToBuffer.extent", "invalid read box or format"); + fault = "the read box or its format is invalid"; + } else if (static_cast(info.Box.W) > kUint64Max / info.Box.H || + static_cast(info.Box.W) * info.Box.H > kUint64Max / bpp) { + fault = "the tight extent overflows"; + } else { + tight = static_cast(info.Box.W) * info.Box.H * bpp; + if (m_maxReplyBytes == 0 || tight > m_maxReplyBytes || + tight > static_cast(std::numeric_limits::max())) { + fault = "the tight extent is past this server's maxReplyBytes (PH-3)"; + } else if (info.DstOffset != 0 || info.DstSize != tight) { + fault = "Src.DstOffset / Src.DstSize are not 0 / the tight extent"; + } else { + fault = ReadbackToBufferLayoutFault(record, bpp); + } } - constexpr Uint64 kUint64Max = std::numeric_limits::max(); - const Uint64 width = info.Box.W; - const Uint64 height = info.Box.H; - if (width > kUint64Max / height || width * height > kUint64Max / bpp) { - return Wire::WireProtocolLatch("ReadPixelsToBuffer.extent", "tight extent overflows"); - } - const Uint64 tight = width * height * bpp; - if (m_maxReplyBytes == 0 || tight > m_maxReplyBytes || - tight > static_cast(std::numeric_limits::max())) { - return Wire::WireProtocolLatchAt("ReadPixelsToBuffer.extent", tight, m_maxReplyBytes); - } - if (info.DstOffset != 0 || info.DstSize != tight) { - return Wire::WireProtocolLatchAt("ReadPixelsToBuffer.Src.DstSize", info.DstSize, tight); + if (fault != nullptr) { + return SessionLatch(MGFatalFamily::ProtocolCorruption, "MGPipe: Fatal{ProtocolCorruption, \"ReadPixelsToBuffer.shape\"} %s", fault); } if (!ReadBoundFramebufferTight(info, tight)) { return SessionLatch(MGFatalFamily::ReadbackDeclined, "MGPipe: Fatal{ReadbackDeclined, \"ReadPixelsToBuffer\"} - the server " "has no GL.ReadPixels; the pack buffer would keep bytes the application is " "about to read as pixels, and there is no reply to carry the decline"); } - return LandReadbackInBuffer(record, m_readbackScratch.data(), bpp, "ReadPixelsToBuffer"); + return LandReadbackInBuffer(record, m_readbackScratch.data(), bpp); } Bool ServerVerbSink::OnGetTextureImageToBuffer(const MG_Pipe::MGPReadbackToBuffer& record) { @@ -666,15 +674,22 @@ namespace MobileGL::MG_Remote::Server { const Uint64 bpp = static_cast(MG_Util::GetInputBytesPerPixel( MG_Util::ConvertGLEnumToTextureInputFormat(info.Format), MG_Util::ConvertGLEnumToTexturePixelDataType(info.Type))); + const char* fault = nullptr; + Uint64 tight = 0; if (!bpp || !info.Box.W || !info.Box.H || !info.Box.D || info.Box.X || info.Box.Y || info.Box.Z) { - return Wire::WireProtocolLatch("GetTextureImageToBuffer.extent", "invalid tight image extent"); + fault = "the image extent is not a whole level's"; + } else { + tight = static_cast(info.Box.W) * info.Box.H * info.Box.D * bpp; + if (tight / bpp / info.Box.W / info.Box.H != info.Box.D) { + fault = "the tight extent overflows"; + } else if (info.DstOffset != 0 || info.DstSize != tight) { + fault = "Src.DstOffset / Src.DstSize are not 0 / the tight extent"; + } else { + fault = ReadbackToBufferLayoutFault(record, bpp); + } } - const Uint64 tight = static_cast(info.Box.W) * info.Box.H * info.Box.D * bpp; - if (tight / bpp / info.Box.W / info.Box.H != info.Box.D) { - return Wire::WireProtocolLatch("GetTextureImageToBuffer.extent", "tight extent overflows"); - } - if (info.DstOffset != 0 || info.DstSize != tight) { - return Wire::WireProtocolLatchAt("GetTextureImageToBuffer.Src.DstSize", info.DstSize, tight); + if (fault != nullptr) { + return SessionLatch(MGFatalFamily::ProtocolCorruption, "MGPipe: Fatal{ProtocolCorruption, \"GetTextureImageToBuffer.shape\"} %s", fault); } Vector bytes; if (!ReadTextureImageTight(info, bytes) || bytes.size() != tight) { @@ -684,7 +699,7 @@ namespace MobileGL::MG_Remote::Server { static_cast(bytes.size()), static_cast(tight)); } - return LandReadbackInBuffer(record, bytes.data(), bpp, "GetTextureImageToBuffer"); + return LandReadbackInBuffer(record, bytes.data(), bpp); } // P5b's server-side stub shape (CONTRACT-P5B.md): the same line the client's class-C table diff --git a/MobileGL/MG_Remote/Server/PipeApplier.h b/MobileGL/MG_Remote/Server/PipeApplier.h index 9fb6b81b..5047801a 100644 --- a/MobileGL/MG_Remote/Server/PipeApplier.h +++ b/MobileGL/MG_Remote/Server/PipeApplier.h @@ -309,10 +309,10 @@ namespace MobileGL::MG_Remote::Server { // get_texture_image's read, shared the same way: the whole level into `bytes`. Bool ReadTextureImageTight(const MG_Pipe::MGPReadbackInfo& image, Vector& bytes); // P9: `tight` holds Src's pixels, tightly packed; land them in record.Dst at the - // record's layout through MGPipeApplyResourceSubData, one write per contiguous run. - // `call` names the op in a refusal. + // record's layout through MGPipeApplyResourceSubData, one write per contiguous run. The + // layout was checked before the read (PipeApplier.cpp ReadbackToBufferLayoutFault). Bool LandReadbackInBuffer(const MG_Pipe::MGPReadbackToBuffer& record, Uint8* tight, - Uint64 bytesPerPixel, const char* call); + Uint64 bytesPerPixel); struct FenceEntry { Uint32 Gen = 0; diff --git a/MobileGL/MG_Test/Wire/PeerLatchTest.cpp b/MobileGL/MG_Test/Wire/PeerLatchTest.cpp index 5365ca62..bd26eb01 100644 --- a/MobileGL/MG_Test/Wire/PeerLatchTest.cpp +++ b/MobileGL/MG_Test/Wire/PeerLatchTest.cpp @@ -1238,6 +1238,50 @@ namespace { death.Kind = 999; return Forge(c, r, P::MGPWireOp::ObjectDeath, Bytes(death)); }}, + // P9 (CONTRACT-P9.md §1): the pack-buffer readbacks. Each sink has ONE shape site, fed by + // every check (box, extent, PH-3 bound, Src.DstSize, the layout), so one row per sink + // covers them; the texture sink's unreadable-level site is the third. + {"ReadPixelsToBufferShape", "PipeApplier.cpp", "ReadPixelsToBuffer.shape", Outcome::Latched, + "Fatal{ProtocolCorruption, \"ReadPixelsToBuffer.shape\"} the read box or its format is invalid", false, + [](Client::ClientSession& c, PeerReport& r) { + P::MGPReadbackToBuffer rec{}; + rec.Src.Box = P::MGPBox{0, 0, 0, 0, 4, 1}; + rec.Src.Format = GL_RGBA; + rec.Src.Type = GL_UNSIGNED_BYTE; + rec.Dst = {5u, 1u}; + rec.RowStride = 16; + return Forge(c, r, P::MGPWireOp::ReadPixelsToBuffer, Bytes(rec)); + }}, + {"GetTextureImageToBufferShape", "PipeApplier.cpp", "GetTextureImageToBuffer.shape", Outcome::Latched, + "Fatal{ProtocolCorruption, \"GetTextureImageToBuffer.shape\"} the image extent is not a whole level's", false, + [](Client::ClientSession& c, PeerReport& r) { + P::MGPReadbackToBuffer rec{}; + rec.Src.Res = {7u, 1u}; + rec.Src.Box = P::MGPBox{0, 0, 0, 4, 4, 0}; + rec.Src.Format = GL_RGBA; + rec.Src.Type = GL_UNSIGNED_BYTE; + rec.Src.Target = GL_TEXTURE_2D; + rec.Dst = {5u, 1u}; + rec.RowStride = 16; + return Forge(c, r, P::MGPWireOp::GetTextureImageToBuffer, Bytes(rec)); + }}, + // A well-shaped record naming a texture the server never created: the level cannot be + // read, and with no reply to answer ERROR in, the session latches under the reply form's + // family word. + {"GetTextureImageToBufferUnreadable", "PipeApplier.cpp", "GetTextureImageToBuffer", Outcome::Latched, + "Fatal{ReplyError, \"GetTextureImageToBuffer\"}", true, + [](Client::ClientSession& c, PeerReport& r) { + P::MGPReadbackToBuffer rec{}; + rec.Src.Res = {77u, 1u}; + rec.Src.Box = P::MGPBox{0, 0, 0, 4, 4, 1}; + rec.Src.Format = GL_RGBA; + rec.Src.Type = GL_UNSIGNED_BYTE; + rec.Src.Target = GL_TEXTURE_2D; + rec.Src.DstSize = 4u * 4u * 4u; + rec.Dst = {5u, 1u}; + rec.RowStride = 16; + return Forge(c, r, P::MGPWireOp::GetTextureImageToBuffer, Bytes(rec)); + }}, // ===== ServerLoop.cpp: DrainRing {"RingRecordHeaderCorrupt", "ServerLoop.cpp", "SEG_CMD record header", Outcome::Latched, "Fatal{ProtocolCorruption, \"SEG_CMD record header\"}", false, diff --git a/scripts/ci/ph_latch_sites.py b/scripts/ci/ph_latch_sites.py index 5d146c13..a57f52ad 100644 --- a/scripts/ci/ph_latch_sites.py +++ b/scripts/ci/ph_latch_sites.py @@ -73,6 +73,11 @@ UNREACHABLE = [ ("ServerLoop.cpp", '"surfaceDestroyed"', "raised by the server's own display (surfaceDestroyed -> ServerDisplay::Detach), never by the " "peer's bytes; ServerLoopTest's lost-window case is its negative control"), + # P9 (CONTRACT-P9.md §1): the pack-buffer read's decline. The reply form's twin answers + # DECLINED for the same case and has no row either. + ("PipeApplier.cpp", 'Fatal{ReadbackDeclined, "ReadPixelsToBuffer"}', + "both server backends install GL.ReadPixels; no record bytes can take the slot away, so the " + "arm exists only for a backend that never had one"), ] REMOTE = ROOT / "MobileGL" / "MG_Remote" diff --git a/scripts/gen_pipe_field_ownership.py b/scripts/gen_pipe_field_ownership.py index cbb5b452..d0982a31 100644 --- a/scripts/gen_pipe_field_ownership.py +++ b/scripts/gen_pipe_field_ownership.py @@ -328,11 +328,18 @@ def build_admitted(accessors, ownership, phase, verbs, verb_ops, waits, verb_cla Disjunct (1) survives as its own term because it is a stronger claim about the same pair: it says the client waits for the record STATICALLY, which is what lets a row whose phase IS this - one (GetFramebufferBindingSlot@ReadPixels, GetTextureUnitObject@CopyTexImage2D) still be - admitted. Without (1) those two would fail the lane; without (2) - GetTransformFeedbackProgram@DrawArrays would fail it, and CONTRACT-P5E §5.7 rules transform - feedback out of this phase entirely - so the lane would be red on a row no package here is - allowed to touch. + one (GetTextureUnitObject@CopyTexImage2D) still be admitted. Without (1) it would fail the + lane; without (2) GetTransformFeedbackProgram@DrawArrays would fail it, and CONTRACT-P5E §5.7 + rules transform feedback out of this phase entirely - so the lane would be red on a row no + package here is allowed to touch. + + (1) IS A STATEMENT ABOUT EVERY OP THAT STAMPS THE VERB (P9, CONTRACT-P9.md §1). The mask is + per verb and the server stamps the same verb for each op in MGP_VERB_OP_LIST that names it, so + one op that publishes without a wait makes the verb's pull torn for THAT op's records. + read_pixels_to_buffer stamps ReadPixels and waits for nothing, which is why + GetFramebufferBindingSlot@ReadPixels - admitted by (1) until P9 - no longer is. The first + version of this function kept the LAST op per verb, so the answer depended on the order of the + stamp list. A verb with no stamp row still admits NOTHING under (1): the server stamps only the ops MGP_VERB_OP_LIST names. Under (2) it admits its non-P5e debts, which is correct for the same @@ -340,16 +347,17 @@ def build_admitted(accessors, ownership, phase, verbs, verb_ops, waits, verb_cla Returns (per-verb (word0, word1), the sorted @ pairs).""" field_index = {name: i for i, name in enumerate(accessors)} - op_for_verb = {} + ops_for_verb = {} for op, verb in verb_ops: - op_for_verb[verb] = op + ops_for_verb.setdefault(verb, []).append(op) masks = [] pairs = [] for verb in verbs: words = [0, 0] - op = op_for_verb.get(verb) - barriered = op is not None and waits.get(op) not in (None, "kWaitNone") + ops = ops_for_verb.get(verb) + op = ops[0] if ops else None + barriered = bool(ops) and all(waits.get(o) not in (None, "kWaitNone") for o in ops) # A VERB WITH NO STAMP ROW ADMITS NOTHING UNDER EITHER DISJUNCT, and that is not an # omission - it is the precondition both disjuncts are stated under. This table is read # only from CountBarrierPull, which is reachable only after MGPipeServerStampVerbBoundary, @@ -1026,8 +1034,12 @@ def self_test(): derivation.append((name, bool(condition))) # ---- ID-125's six survivors, each by the disjunct that carries it -------------------- - expect("GetFramebufferBindingSlot@ReadPixels is admitted by disjunct 1 (read_pixels waits)", - "GetFramebufferBindingSlot@ReadPixels" in derived) + # P9 moved the first of them: read_pixels_to_buffer stamps ReadPixels and waits for nothing, + # so ReadPixels is no longer statically barriered and its P5e-owed pair leaves (the P9 + # controls below put it back by barriering that op, and take it away again from the other op). + expect("GetFramebufferBindingSlot@ReadPixels is REJECTED since P9 (read_pixels_to_buffer " + "stamps ReadPixels without a wait)", + "GetFramebufferBindingSlot@ReadPixels" not in derived) expect("GetTextureUnitObject@CopyTexImage2D is admitted by disjunct 1", "GetTextureUnitObject@CopyTexImage2D" in derived) # Disjunct 2's three. Each is a debt some LATER phase owes, on a verb this phase does not @@ -1053,17 +1065,27 @@ def self_test(): # "did the text change" guard would call its own control broken while it was working # perfectly - and once item 5 moves that op, the same guard would start passing for a # different reason. What has to hold is that the row was FOUND. - def unbarrier(op): + def set_wait(op, wait_class, text=None): edited, count = re.subn( r"(/\*\s*\d+\s+" + op + r"\s*\*/\s*static_cast\()kWait\w+(\))", - r"\1kWaitNone\2", wire_inc, count=1) + r"\g<1>" + wait_class + r"\2", wire_inc if text is None else text, count=1) if count != 1: - sys.exit("gen_pipe_field_ownership: self-test: could not unbarrier %s - the control's " - "own edit no longer matches generated/PipeWire.inc" % op) + sys.exit("gen_pipe_field_ownership: self-test: could not set %s's wait class - the " + "control's own edit no longer matches generated/PipeWire.inc" % op) return edited - unwaited = unbarrier("ReadPixels") - expect("unbarriering ReadPixels' op drops its P5e-owed pair " + def unbarrier(op, text=None): + return set_wait(op, "kWaitNone", text) + + # P9: EVERY OP THAT STAMPS THE VERB. Barrier read_pixels_to_buffer as well and ReadPixels is + # statically barriered again, so its P5e-owed pair comes back by disjunct 1 - which proves the + # pair left above because of THAT op and not for some other reason. + rewaited = set_wait("ReadPixelsToBuffer", "kWaitApplied") + expect("barriering read_pixels_to_buffer too brings GetFramebufferBindingSlot@ReadPixels back " + "(disjunct 1 needs every op that stamps the verb to wait)", + "GetFramebufferBindingSlot@ReadPixels" in admitted_pairs_for(wire_text=rewaited)) + unwaited = unbarrier("ReadPixels", rewaited) + expect("unbarriering ReadPixels' own op drops the pair again " "(GetFramebufferBindingSlot@ReadPixels) and keeps the later phases' pairs", "GetFramebufferBindingSlot@ReadPixels" not in admitted_pairs_for(wire_text=unwaited) and "GetTextureObject@ReadPixels" in admitted_pairs_for(wire_text=unwaited)) @@ -1102,12 +1124,16 @@ def self_test(): # AND THE RULE OVER THE WHOLE TABLE, not just its six named rows: nothing THIS PHASE owes is # ever admitted on a verb no barriered op stamps. If this goes false the allowlist has # started forgiving the debt P5e exists to retire. - op_for_verb = {verb: op for op, verb in parse_ownership(ownership_text)[3]} + # A verb is unbarriered when ANY op that stamps it publishes without a wait (P9) - the same + # rule build_admitted states, restated here so that the two cannot drift together. + ops_for_verb = {} + for op, verb in parse_ownership(ownership_text)[3]: + ops_for_verb.setdefault(verb, []).append(op) ownership_now, phase_now = run()[0], run()[1] unbarriered_p5e = sorted( pair for pair in derived - if (lambda f, v: (op_for_verb.get(v) is None - or waits_real.get(op_for_verb[v]) in (None, "kWaitNone")) + if (lambda f, v: (not ops_for_verb.get(v) + or any(waits_real.get(o) in (None, "kWaitNone") for o in ops_for_verb[v])) and phase_is_owed_by_this_phase(phase_now[f]))(*pair.split("@", 1))) expect("no pair this phase owes is admitted on an unbarriered verb: " + (", ".join(unbarriered_p5e) or "none"),