[Fix] (MG_Pipe, gen_pipe): export the per-opcode call flags and give GetCaps and CreateSamplerState the kHasBlob their payloads have always owned

MobileGL/MG_Pipe/PipeCalls.def

@@ -23,6 +23,14 @@
 //             (MGPipeTypes.h) is what says so, which is why the same call still carries
 //             every glBufferData without acknowledging one.
 //
+//             kHasBlob MEANS "THE PAYLOAD OWNS AN MGPBlobRef MEMBER", nothing weaker (P5
+//             R-13.1). It is not "this call carries bytes": three calls carry bytes with no
+//             MGPBlobRef at all and they are named in CONTRACT-P5.md table 1, not flagged
+//             here, because a decoder that trusts kHasBlob has to find a member to read.
+//             The flags are EXPORTED, once, as kMGPipeCallFlags[] in generated/PipeWire.inc
+//             (P5 R-13.4): before that table existed every consumer hard-coded its own copy,
+//             which is how the two carriers below came to disagree with their payloads.
+//
 // RECORD NUMBERING NEVER CHURNS. Entries that are not implemented yet still occupy their
 // line (plan section 11, P0: "the complete call catalogue, placeholders included"). A new
 // call is APPENDED to its group; a retired call keeps its slot with a comment. The wire
@@ -77,8 +85,20 @@
 // clang-format off
 #define MGP_CALL_LIST(X)                                                                            \
     /* ---- screen: caps, resources, persistent map, fences (plan 4.4.1) ---- */                    \
-    X(GetCaps,                  MGPCaps,                    kScreen,    kReplySlot)                 \
+    /* kHasBlob because MGPCaps owns TWO MGPBlobRef members - FormatCapabilities and      */  \
+    /* RendererInfo (MGPipeTypes.h). It carried none until P5 R-13.1; the flag is a        */  \
+    /* property of the payload, so its absence was simply wrong. Adding a flag does not    */  \
+    /* move an opcode: the opcode is this line's 1-based position.                          */  \
+    X(GetCaps,                  MGPCaps,                    kScreen,    kReplySlot|kHasBlob)     \
     X(ResourceCreate,           MGPResourceDesc,            kScreen,    kNone)                      \
+    /* NO kHasBlob, BY RULING (P5 R-13.3; CONTRACT-P5.md table 1 row 19). MGPResourceDesc   */  \
+    /* owns no MGPBlobRef and gains none: under split `initialBytes` is always nullptr and  */  \
+    /* the initial content of a glBufferData(size, data) / glTexImage*(..., data) arrives   */  \
+    /* as resource_subdata records IMMEDIATELY AFTER this one. Reuses a path that is        */  \
+    /* already chunked (MGPipeForEachSubDataRecordRange) and already acceptance-gated; the  */  \
+    /* cost is one extra record. NOTE the SECOND uncarried companion on this call, the      */  \
+    /* MGPRespecifiedLevel* scope (PipeApply.h:792) - it is NOT bytes and NOT covered by    */  \
+    /* this ruling; table 1 row 19 gives it the two pad fields of MGPResourceDesc.          */  \
     X(ResourceRespecify,        MGPResourceDesc,            kScreen,    kNeedsAck)                  \
     X(ResourceDestroy,          MGPHandleOnly,              kScreen,    kNone)                      \
     X(MapPersistent,            MGPHandleOnly,              kScreen,    kReplySlot|kOptional)       \
@@ -101,7 +121,11 @@
     X(CreateVertexElements,     MGPVertexElements,          kCtxCso,    kHasBlob)                   \
     X(BindVertexElements,       MGPHandleOnly,              kCtxCso,    kNone)                      \
     X(DeleteVertexElements,     MGPHandleOnly,              kCtxCso,    kNone)                      \
-    X(CreateSamplerState,       MGPSamplerDesc,             kCtxCso,    kNone)                      \
+    /* kHasBlob because MGPSamplerDesc owns an MGPBlobRef member, `Parameters` - the        */  \
+    /* SamplerParameters POD byte for byte, borderColorForm included (P5 R-13.1). The       */  \
+    /* companion pointer beside the record today is a TYPED frontend pointer               */  \
+    /* (const SamplerParameters*), which is exactly what split may not carry.              */  \
+    X(CreateSamplerState,       MGPSamplerDesc,             kCtxCso,    kHasBlob)                   \
     X(DeleteSamplerState,       MGPHandleOnly,              kCtxCso,    kNone)                      \
     X(CreateSamplerView,        MGPSamplerView,             kCtxCso,    kNone)                      \
     X(DeleteSamplerView,        MGPHandleOnly,              kCtxCso,    kNone)                      \
@@ -134,6 +158,13 @@
     X(ResourceSubData,          MGPSubData,                 kCtxObject, kHasBlob|kVarTail)          \
     X(BufferSubDataResident,    MGPSubData,                 kCtxObject, kHasBlob|kOptional)         \
     X(ResourceSubDataComplete,  MGPSubDataComplete,         kCtxObject, kNone)                      \
+    /* NO kHasBlob AND NO MGPBlobRef, BY RULING (P5 R-13.2; CONTRACT-P5.md table 1 row 21). */  \
+    /* It takes a content pointer today, but under split it CARRIES NO BYTES AT ALL: it is  */  \
+    /* a {range, AccessFlags} control record, and the bytes of [Offset, Offset+Size) arrive  */  \
+    /* AHEAD of it as resource_subdata records covering exactly that range. The ladder this  */  \
+    /* drives rewrites its range "from the authoritative shadow" (Managers.cpp:1047-1076),   */  \
+    /* and under split the authoritative shadow is server-owned (R-11), so subdata is the    */  \
+    /* only way bytes reach it - a blobref here would be a SECOND way to say the same thing. */  \
     X(ResourceFlushRange,       MGPFlushRange,              kCtxObject, kNone)                      \
     X(ResourceReadback,         MGPReadback,                kCtxObject, kReplySlot)                 \
     X(ResourceCopyRegion,       MGPCopyRegion,              kCtxObject, kNone)                      \

MobileGL/MG_Pipe/generated/PipeWire.inc

@@ -118,6 +118,127 @@ enum class MGPWireOp : Uint16 {
     kOpCount = 72,
 };
 
+// THE FLAGS, EXPORTED ONCE, INDEXED BY OPCODE (P5 R-13.4). MGPWireRecHeader::Flags is
+// documented as "MGPipeCallFlags of the call", and until this table existed nothing
+// generated said what those were: every consumer that needed to know whether a record owns
+// an MGPBlobRef, a variable tail or a reply slot had to hard-code its own copy of
+// PipeCalls.def's fourth column, and six of them were about to. A hard-coded copy is how
+// GetCaps and CreateSamplerState came to carry an MGPBlobRef member with no kHasBlob on
+// their line at all - nothing compared the two, because nothing had both in one place.
+//
+// Index 0 is MGPWireOp::kInvalid and is kNone: the catalogue is 1-based, and an encoder
+// that reads flags for an opcode it never got from the catalogue must see the empty set
+// rather than another call's flags.
+//
+// kHasBlob here means EXACTLY "the payload owns an MGPBlobRef member". Three calls carry
+// bytes without one - resource_respecify, resource_flush_range and map_persistent, whose
+// companion pointers have no carrier - and they are deliberately NOT flagged; MG_Remote's
+// CONTRACT-P5.md table 1 is where those live, because a decoder that trusts kHasBlob has
+// to find a member to read.
+inline constexpr Uint32 kMGPipeCallFlags[static_cast<SizeT>(MGPWireOp::kOpCount)] = {
+    /*  0 kInvalid                */ static_cast<Uint32>(kNone),
+    /*  1 GetCaps                 */ static_cast<Uint32>(kReplySlot | kHasBlob),
+    /*  2 ResourceCreate          */ static_cast<Uint32>(kNone),
+    /*  3 ResourceRespecify       */ static_cast<Uint32>(kNeedsAck),
+    /*  4 ResourceDestroy         */ static_cast<Uint32>(kNone),
+    /*  5 MapPersistent           */ static_cast<Uint32>(kReplySlot | kOptional),
+    /*  6 UnmapPersistent         */ static_cast<Uint32>(kOptional),
+    /*  7 FenceCreate             */ static_cast<Uint32>(kNone),
+    /*  8 FenceStatus             */ static_cast<Uint32>(kReplySlot),
+    /*  9 FenceWait               */ static_cast<Uint32>(kReplySlot),
+    /* 10 FenceDestroy            */ static_cast<Uint32>(kNone),
+    /* 11 QueryCreate             */ static_cast<Uint32>(kNone),
+    /* 12 QueryBegin              */ static_cast<Uint32>(kNone),
+    /* 13 QueryEnd                */ static_cast<Uint32>(kNone),
+    /* 14 QueryAvailable          */ static_cast<Uint32>(kReplySlot),
+    /* 15 QueryResult             */ static_cast<Uint32>(kReplySlot),
+    /* 16 QueryDestroy            */ static_cast<Uint32>(kNone),
+    /* 17 CreateRenderState       */ static_cast<Uint32>(kHasBlob),
+    /* 18 BindRenderState         */ static_cast<Uint32>(kNone),
+    /* 19 DeleteRenderState       */ static_cast<Uint32>(kNone),
+    /* 20 CreateVertexElements    */ static_cast<Uint32>(kHasBlob),
+    /* 21 BindVertexElements      */ static_cast<Uint32>(kNone),
+    /* 22 DeleteVertexElements    */ static_cast<Uint32>(kNone),
+    /* 23 CreateSamplerState      */ static_cast<Uint32>(kHasBlob),
+    /* 24 DeleteSamplerState      */ static_cast<Uint32>(kNone),
+    /* 25 CreateSamplerView       */ static_cast<Uint32>(kNone),
+    /* 26 DeleteSamplerView       */ static_cast<Uint32>(kNone),
+    /* 27 CreateShaderState       */ static_cast<Uint32>(kHasBlob),
+    /* 28 BindShaderState         */ static_cast<Uint32>(kNone),
+    /* 29 DeleteShaderState       */ static_cast<Uint32>(kNone),
+    /* 30 SetDynamicState         */ static_cast<Uint32>(kHasBlob),
+    /* 31 SetFramebufferState     */ static_cast<Uint32>(kNone),
+    /* 32 SetVertexBuffers        */ static_cast<Uint32>(kVarTail),
+    /* 33 SetIndexBuffer          */ static_cast<Uint32>(kNone),
+    /* 34 SetIndirectBuffers      */ static_cast<Uint32>(kNone),
+    /* 35 SetSamplerViews         */ static_cast<Uint32>(kVarTail),
+    /* 36 BindSamplerStates       */ static_cast<Uint32>(kVarTail),
+    /* 37 SetShaderImages         */ static_cast<Uint32>(kVarTail),
+    /* 38 SetShaderBuffers        */ static_cast<Uint32>(kVarTail | kHostSpan),
+    /* 39 SetStreamOutputTargets  */ static_cast<Uint32>(kVarTail),
+    /* 40 SetGlobalConstants      */ static_cast<Uint32>(kHasBlob),
+    /* 41 SetVertexAttribDefaults */ static_cast<Uint32>(kVarTail),
+    /* 42 SetPixelPackState       */ static_cast<Uint32>(kNone),
+    /* 43 SetPatchState           */ static_cast<Uint32>(kNone),
+    /* 44 SetDrawProgram          */ static_cast<Uint32>(kNone),
+    /* 45 SetDispatchProgram      */ static_cast<Uint32>(kNone),
+    /* 46 SetResidualValueState   */ static_cast<Uint32>(kHasBlob),
+    /* 47 SetTextureParams        */ static_cast<Uint32>(kNone),
+    /* 48 ResourceSubData         */ static_cast<Uint32>(kHasBlob | kVarTail),
+    /* 49 BufferSubDataResident   */ static_cast<Uint32>(kHasBlob | kOptional),
+    /* 50 ResourceSubDataComplete */ static_cast<Uint32>(kNone),
+    /* 51 ResourceFlushRange      */ static_cast<Uint32>(kNone),
+    /* 52 ResourceReadback        */ static_cast<Uint32>(kReplySlot),
+    /* 53 ResourceCopyRegion      */ static_cast<Uint32>(kNone),
+    /* 54 GenerateMipmap          */ static_cast<Uint32>(kNone),
+    /* 55 GetTextureImage         */ static_cast<Uint32>(kReplySlot),
+    /* 56 Blit                    */ static_cast<Uint32>(kNone),
+    /* 57 Clear                   */ static_cast<Uint32>(kNone),
+    /* 58 ReadPixels              */ static_cast<Uint32>(kReplySlot),
+    /* 59 DrawVbo                 */ static_cast<Uint32>(kHostSpan | kVarTail),
+    /* 60 LaunchGrid              */ static_cast<Uint32>(kNone),
+    /* 61 MemoryBarrier           */ static_cast<Uint32>(kNone),
+    /* 62 BeginStreamOutput       */ static_cast<Uint32>(kNone),
+    /* 63 EndStreamOutput         */ static_cast<Uint32>(kNone),
+    /* 64 PauseStreamOutput       */ static_cast<Uint32>(kNone),
+    /* 65 ResumeStreamOutput      */ static_cast<Uint32>(kNone),
+    /* 66 Flush                   */ static_cast<Uint32>(kNone),
+    /* 67 Present                 */ static_cast<Uint32>(kNone),
+    /* 68 SetSwapInterval         */ static_cast<Uint32>(kOptional),
+    /* 69 QueryTimestamp          */ static_cast<Uint32>(kReplySlot),
+    /* 70 QueryCounter            */ static_cast<Uint32>(kNone),
+    /* 71 FenceWaitServer         */ static_cast<Uint32>(kNone),
+};
+static_assert(sizeof(kMGPipeCallFlags) / sizeof(kMGPipeCallFlags[0]) ==
+                  static_cast<SizeT>(MGPWireOp::kOpCount),
+              "the flags table and the opcode space disagree");
+
+// The only supported read of the table. Out-of-range is kNone rather than undefined
+// behaviour, because the one caller that can pass a bad opcode is a decoder holding bytes
+// off a stream, and it must reach its own Fatal{ProtocolCorruption} rather than read past
+// the array on the way there.
+inline constexpr Uint32 MGPipeCallFlagsFor(MGPWireOp op) {
+    const SizeT index = static_cast<SizeT>(op);
+    return index < static_cast<SizeT>(MGPWireOp::kOpCount) ? kMGPipeCallFlags[index]
+                                                           : static_cast<Uint32>(kNone);
+}
+
+// Spot checks the generator states about its own output, so that a catalogue edit that
+// silently drops a flag is a build break here and not a wrong decode six packages away.
+static_assert(MGPipeCallFlagsFor(MGPWireOp::kInvalid) == static_cast<Uint32>(kNone),
+              "opcode 0 is not a call and carries no flags");
+static_assert((MGPipeCallFlagsFor(MGPWireOp::GetCaps) & static_cast<Uint32>(kHasBlob)) != 0,
+              "MGPCaps owns two MGPBlobRef members; R-13.1 gave the call its flag");
+static_assert((MGPipeCallFlagsFor(MGPWireOp::CreateSamplerState) & static_cast<Uint32>(kHasBlob)) != 0,
+              "MGPSamplerDesc owns an MGPBlobRef member; R-13.1 gave the call its flag");
+static_assert((MGPipeCallFlagsFor(MGPWireOp::ResourceFlushRange) & static_cast<Uint32>(kHasBlob)) == 0,
+              "R-13.2: resource_flush_range carries no bytes on the wire and owns no blobref");
+static_assert((MGPipeCallFlagsFor(MGPWireOp::ResourceRespecify) & static_cast<Uint32>(kHasBlob)) == 0,
+              "R-13.3: initial bytes follow as resource_subdata; MGPResourceDesc owns no blobref");
+static_assert((MGPipeCallFlagsFor(MGPWireOp::DrawVbo) &
+               static_cast<Uint32>(kHostSpan | kVarTail)) == static_cast<Uint32>(kHostSpan | kVarTail),
+              "draw_vbo is the conditional-tail plus host-span shape the codec is measured on");
+
 struct alignas(8) MGPWireRec_GetCaps {
     MGPWireRecHeader Header;
     MGPCaps Payload;

scripts/gen_pipe.py

@@ -487,6 +487,30 @@ def check_call_payloads_have_field_lists(calls, payloads):
                  "would be blind to them: %s" % ", ".join(missing))
 
 
+# The MGPipeCallFlags enumerators, MGPipe.h:39-54. Kept here rather than parsed out of the
+# header because this list is what the generated kMGPipeCallFlags[] table spells into C++:
+# a flag token in PipeCalls.def that is not one of these would generate an expression that
+# does not compile, and a build break several minutes later is a worse diagnosis than this
+# one line. kNone is listed but is NOT a flag - it is the empty set, and it may not be
+# combined with anything.
+KNOWN_CALL_FLAGS = ("kNeedsAck", "kHasBlob", "kVarTail", "kHostSpan", "kReplySlot", "kOptional")
+
+
+def check_call_flags_are_known(calls):
+    """Every flag token in PipeCalls.def must be an MGPipeCallFlags enumerator, and kNone may
+    not be combined with one. Runs in both modes, --check included: P5 R-13.4 exports these
+    flags as a table six packages read, so a typo here is a wrong decode rather than a
+    compile error in the one consumer that used to hard-code its own copy."""
+    for call in calls:
+        for flag in call.Flags:
+            if flag != "kNone" and flag not in KNOWN_CALL_FLAGS:
+                sys.exit("PipeCalls.def: %s carries flag %s, which is not an MGPipeCallFlags "
+                         "enumerator (%s)" % (call.Name, flag, ", ".join(KNOWN_CALL_FLAGS)))
+        if "kNone" in call.Flags and len(call.Flags) != 1:
+            sys.exit("PipeCalls.def: %s combines kNone with %s; kNone is the empty set"
+                     % (call.Name, "|".join(f for f in call.Flags if f != "kNone")))
+
+
 def parse_coverage():
     text = read(os.path.join(PIPE_DIR, "Coverage.def"))
     accessors = []
@@ -641,6 +665,59 @@ enum class MGPWireOp : Uint16 {
         out.append("    %s = %d," % (call.Name, call.Index))
     out.append("    kOpCount = %d," % (len(calls) + 1))
     out.append("};\n")
+    out.append("""// THE FLAGS, EXPORTED ONCE, INDEXED BY OPCODE (P5 R-13.4). MGPWireRecHeader::Flags is
+// documented as "MGPipeCallFlags of the call", and until this table existed nothing
+// generated said what those were: every consumer that needed to know whether a record owns
+// an MGPBlobRef, a variable tail or a reply slot had to hard-code its own copy of
+// PipeCalls.def's fourth column, and six of them were about to. A hard-coded copy is how
+// GetCaps and CreateSamplerState came to carry an MGPBlobRef member with no kHasBlob on
+// their line at all - nothing compared the two, because nothing had both in one place.
+//
+// Index 0 is MGPWireOp::kInvalid and is kNone: the catalogue is 1-based, and an encoder
+// that reads flags for an opcode it never got from the catalogue must see the empty set
+// rather than another call's flags.
+//
+// kHasBlob here means EXACTLY "the payload owns an MGPBlobRef member". Three calls carry
+// bytes without one - resource_respecify, resource_flush_range and map_persistent, whose
+// companion pointers have no carrier - and they are deliberately NOT flagged; MG_Remote's
+// CONTRACT-P5.md table 1 is where those live, because a decoder that trusts kHasBlob has
+// to find a member to read.""")
+    out.append("inline constexpr Uint32 kMGPipeCallFlags[static_cast<SizeT>(MGPWireOp::kOpCount)] = {")
+    out.append("    /*  0 %-24s*/ static_cast<Uint32>(kNone)," % "kInvalid")
+    for call in calls:
+        out.append("    /* %2d %-24s*/ static_cast<Uint32>(%s),"
+                   % (call.Index, call.Name, " | ".join(call.Flags)))
+    out.append("};")
+    out.append("static_assert(sizeof(kMGPipeCallFlags) / sizeof(kMGPipeCallFlags[0]) ==")
+    out.append("                  static_cast<SizeT>(MGPWireOp::kOpCount),")
+    out.append("              \"the flags table and the opcode space disagree\");")
+    out.append("""
+// The only supported read of the table. Out-of-range is kNone rather than undefined
+// behaviour, because the one caller that can pass a bad opcode is a decoder holding bytes
+// off a stream, and it must reach its own Fatal{ProtocolCorruption} rather than read past
+// the array on the way there.
+inline constexpr Uint32 MGPipeCallFlagsFor(MGPWireOp op) {
+    const SizeT index = static_cast<SizeT>(op);
+    return index < static_cast<SizeT>(MGPWireOp::kOpCount) ? kMGPipeCallFlags[index]
+                                                           : static_cast<Uint32>(kNone);
+}
+
+// Spot checks the generator states about its own output, so that a catalogue edit that
+// silently drops a flag is a build break here and not a wrong decode six packages away.
+static_assert(MGPipeCallFlagsFor(MGPWireOp::kInvalid) == static_cast<Uint32>(kNone),
+              "opcode 0 is not a call and carries no flags");
+static_assert((MGPipeCallFlagsFor(MGPWireOp::GetCaps) & static_cast<Uint32>(kHasBlob)) != 0,
+              "MGPCaps owns two MGPBlobRef members; R-13.1 gave the call its flag");
+static_assert((MGPipeCallFlagsFor(MGPWireOp::CreateSamplerState) & static_cast<Uint32>(kHasBlob)) != 0,
+              "MGPSamplerDesc owns an MGPBlobRef member; R-13.1 gave the call its flag");
+static_assert((MGPipeCallFlagsFor(MGPWireOp::ResourceFlushRange) & static_cast<Uint32>(kHasBlob)) == 0,
+              "R-13.2: resource_flush_range carries no bytes on the wire and owns no blobref");
+static_assert((MGPipeCallFlagsFor(MGPWireOp::ResourceRespecify) & static_cast<Uint32>(kHasBlob)) == 0,
+              "R-13.3: initial bytes follow as resource_subdata; MGPResourceDesc owns no blobref");
+static_assert((MGPipeCallFlagsFor(MGPWireOp::DrawVbo) &
+               static_cast<Uint32>(kHostSpan | kVarTail)) == static_cast<Uint32>(kHostSpan | kVarTail),
+              "draw_vbo is the conditional-tail plus host-span shape the codec is measured on");
+""")
     for call in calls:
         out.append("struct alignas(8) MGPWireRec_%s {" % call.Name)
         out.append("    MGPWireRecHeader Header;")
@@ -1232,12 +1309,24 @@ def self_test(accessors):
     calls_for_control = parse_calls()
     controls.append(("emitted row naming a call that does not exist", lambda: gen_emitted_by(
         [("GetViewport", "SetDynamicState")], calls_for_control, [("GetViewport", "NotACall")])))
+    # P5 R-13.4's gate. The flags are now a GENERATED TABLE six packages read instead of six
+    # hard-coded copies, so a token that is not an MGPipeCallFlags enumerator has to stop the
+    # generator rather than emit an expression that fails to compile minutes later - and
+    # kNone, the empty set, may not be OR'd with a real flag and quietly read as one.
+    flag_typo = Call(1, "Canned", "MGPHandleOnly", "kScreen", ["kHasBlobb"])
+    flag_kNone = Call(1, "Canned", "MGPHandleOnly", "kScreen", ["kNone", "kHasBlob"])
+    controls.append(("call flag that is not an MGPipeCallFlags enumerator",
+                     lambda: check_call_flags_are_known([flag_typo])))
+    controls.append(("kNone combined with a real flag",
+                     lambda: check_call_flags_are_known([flag_kNone])))
     trips = 0
     for name, fn in controls:
         trips += expect_trip(name, fn)
     # The positive control: the canned struct's exact list passes, and the parser sees the
     # padding member as padding and the function as not a member.
     check_field_lists_cover_struct_members({"Canned": ["A", "B", "C"]}, ["Canned"], [canned_struct])
+    # ... and the real catalogue's real flags pass the same gate.
+    check_call_flags_are_known(calls_for_control)
     if trips == 0:
         sys.exit("gen_pipe: self-test: no negative control tripped - the gates are not checking anything")
     if trips != len(controls):
@@ -1256,6 +1345,7 @@ def main():
 
     calls = parse_calls()
     payloads = parse_verify_payloads()
+    check_call_flags_are_known(calls)
     check_call_payloads_have_field_lists(calls, payloads)
     check_field_lists_cover_struct_members(parse_field_lists(), payloads)
     accessors, deltas, sticky, emitted = parse_coverage()