[Feat] (Pipe): widen the dirty-surface scan to MG_State so the MGP_NOTE_MUTATION sites stop being outside the gate

This commit is contained in:
2026-09-08 04:47:51 -04:00
parent 14efd6eb24
commit b11bb9650a
3 changed files with 100 additions and 19 deletions
+51 -8
View File
@@ -14,9 +14,18 @@
// forgets to publish renders stale, and no purity gate can see it. // forgets to publish renders stale, and no purity gate can see it.
// //
// So the surface is enumerated MECHANICALLY. scripts/gen_pipe_dirty_surface.py scans // So the surface is enumerated MECHANICALLY. scripts/gen_pipe_dirty_surface.py scans
// MG_Impl/GLImpl for every pGLContext-> mutator call and, with --check, fails if a scanned // MG_Impl/GLImpl AND MG_State/GLState for every pGLContext-> mutator call and every
// mutator has no row here or a row here names a mutator the scan no longer finds. Both // MGP_NOTE_MUTATION site, and, with --check, fails if a scanned mutator has no row here or a
// directions, so a deleted mutator cannot leave a stale row behind either. // row here names a mutator the scan no longer finds. Both directions, so a deleted mutator
// cannot leave a stale row behind either.
//
// THE SECOND ROOT AND THE SECOND MECHANISM ARE P3a's, and they were not cosmetic: reading
// MG_State/GLState found BumpSamplingResolutionGeneration, a mutator with no row at all
// whose every caller lives in that directory, and reading MGP_NOTE_MUTATION brought
// NoteUnitTouched - which no `pGLContext->` pattern can see, because every texture and
// sampler bind routes through it rather than calling it through the context - inside the
// gate. A notice site carries a FIELD name, so the mutator a row names is the ENCLOSING
// function.
// //
// ANSWERS. A row lists EVERY publisher that fires on EVERY path through that mutator, // ANSWERS. A row lists EVERY publisher that fires on EVERY path through that mutator,
// and only those; several are joined with '|'. A publisher that fires on some paths but // and only those; several are joined with '|'. A publisher that fires on some paths but
@@ -134,14 +143,16 @@
// that should have named one. // that should have named one.
// //
// KNOWN BLIND SPOTS OF THE SCANNER, recorded here rather than left implicit // KNOWN BLIND SPOTS OF THE SCANNER, recorded here rather than left implicit
// (gen_pipe_dirty_surface.py's own notes plus its scan root): // (gen_pipe_dirty_surface.py's own notes plus its scan roots):
// 1. it matches braced function bodies textually, so a mutator inside a LAMBDA is // 1. it matches braced function bodies textually, so a mutator inside a LAMBDA is
// attributed to the enclosing function; // attributed to the enclosing function;
// 2. a mutation published through a HELPER the entry point calls reads as deferred here; // 2. a mutation published through a HELPER the entry point calls reads as deferred here.
// 3. the scan root is MG_Impl/GLImpl only, so the four MGP_NOTE_MUTATION sites in // 3. CLOSED AT P3a. The scan root was MG_Impl/GLImpl only, so the four MGP_NOTE_MUTATION
// MG_State/GLState/TextureState/TextureState.h are outside it entirely. // sites in MG_State/GLState/TextureState/TextureState.h were outside it entirely. The
// root is now both directories and the notice is a recognised publish mechanism; what
// that found is the two rows marked "P3a" below.
// The gate is therefore a COMPLETENESS gate over what the scanner does see. The semantic // The gate is therefore a COMPLETENESS gate over what the scanner does see. The semantic
// proof stays the MOBILEGL_PIPE_VERIFY lane, which is blind to none of the three. // proof stays the MOBILEGL_PIPE_VERIFY lane, which is blind to none of them.
// //
// clang-format off // clang-format off
@@ -235,6 +246,29 @@
X(SetCurrentVertexAttributeUint, NEW_VERTEX_ATTRIB_DEFAULTS) \ X(SetCurrentVertexAttributeUint, NEW_VERTEX_ATTRIB_DEFAULTS) \
/* ---- object class ---- */ \ /* ---- object class ---- */ \
X(BumpTextureBindGeneration, NEW_SAMPLER_VIEWS) \ X(BumpTextureBindGeneration, NEW_SAMPLER_VIEWS) \
/* P3a, FOUND BY THE WIDENED SCAN ROOT and not by anything before it: every caller of */ \
/* this one is inside MG_State/GLState (SamplerObject::BumpVersion for any sampler */ \
/* parameter, TextureObjectBase's shape bump), which the scan did not read, so it had */ \
/* no row at all while its sibling above did. NEW_SAMPLERS is the bit whose shutter */ \
/* mixes the texture-params aggregate with exactly this generation, and the bump is */ \
/* unconditional on every path that reaches it - the early-outs are in the setters */ \
/* above it, which is the redundant-write guard this file's rule explicitly allows. */ \
X(BumpSamplingResolutionGeneration, NEW_SAMPLERS) \
/* P3a, and the reason the scan now reads MGP_NOTE_MUTATION as a publish mechanism of */ \
/* its own: this function is not a pGLContext-> mutator at all - every texture and */ \
/* sampler bind entry point routes THROUGH it - and it moves two pushed PipeInputs */ \
/* fields with two different answers. */ \
/* GetTextureBindGeneration moves only on the bindingChanged arm, so NEW_SAMPLER_ */ \
/* VIEWS fires on SOME of the paths that mutate and not all: a redundant re-bind */ \
/* of the object a unit already holds advances the high-water mark alone. That is */ \
/* precisely what kPulledPartialShutter is for. */ \
/* GetMaxTouchedTextureUnit has no shutter at all; it is in its verb class's may-read */ \
/* mask and the residual fill copies it at every verb. */ \
/* Both are ALSO published inline by MGP_NOTE_MUTATION when the write happens inside a */ \
/* verb already in flight - a backend binding its own synthesised fallback texture - */ \
/* which is the window no shutter and no pull can cover, and the reason those sites */ \
/* had to come inside this gate rather than stay a recorded blind spot. */ \
X(NoteUnitTouched, kPulledPartialShutter|NEW_SAMPLER_VIEWS) \
/* NOT NEW_SO_TARGETS, and this one was false on EVERY path: GLContext::SetNamed */ \ /* NOT NEW_SO_TARGETS, and this one was false on EVERY path: GLContext::SetNamed */ \
/* TransformFeedbackBinding either binds a BufferState binding point (index == the */ \ /* TransformFeedbackBinding either binds a BufferState binding point (index == the */ \
/* bound XFB object) or writes a saved-bindings entry, and NEW_SO_TARGETS mixes the */ \ /* bound XFB object) or writes a saved-bindings entry, and NEW_SO_TARGETS mixes the */ \
@@ -249,6 +283,15 @@
/* to carry one. Espryt 0b's delete_* / resource_destroy publishes the kinds */ \ /* to carry one. Espryt 0b's delete_* / resource_destroy publishes the kinds */ \
/* that have a handle on the wire; programs, program pipelines and shaders have */ \ /* that have a handle on the wire; programs, program pipelines and shaders have */ \
/* none in P2, so nothing publishes theirs - kUnpublishedDestroy, a known hole. */ \ /* none in P2, so nothing publishes theirs - kUnpublishedDestroy, a known hole. */ \
/* TWO OF THESE ROWS STOPPED BEING ASPIRATIONAL AT P3a, and the call that makes */ \
/* each true is named rather than implied: */ \
/* MarkBufferObjectForDeletion -> resource_destroy (PipeCalls.def), emitted */ \
/* from ~BufferObject the moment the last reference drops - which is the */ \
/* glDelete* that only marks the name, followed by whatever unbind actually */ \
/* releases it - and followed IN THAT ORDER by the client freeing the slot. */ \
/* MarkVertexArrayForDeletion -> delete_vertex_elements, published through the */ \
/* death notice ~VertexArrayObject already raises for the VertexElementsCso */ \
/* kind; the CSO handle is minted per frontend VAO off its lifetime id. */ \
X(MarkBufferObjectForDeletion, kExplicitDestroy) \ X(MarkBufferObjectForDeletion, kExplicitDestroy) \
X(MarkFramebufferObjectForDeletion, kExplicitDestroy) \ X(MarkFramebufferObjectForDeletion, kExplicitDestroy) \
X(MarkProgramForDeletion, kUnpublishedDestroy) \ X(MarkProgramForDeletion, kUnpublishedDestroy) \
+8 -2
View File
@@ -60,8 +60,14 @@
// What WOULD retire a row: the poison build already answers "was this field read at this // What WOULD retire a row: the poison build already answers "was this field read at this
// verb" exactly (MOBILEGL_PIPE_POISON_OMIT withholds one field's stamp for one verb and a // verb" exactly (MOBILEGL_PIPE_POISON_OMIT withholds one field's stamp for one verb and a
// read of it aborts naming the pair). Turning that into a retirement gate means running the // read of it aborts naming the pair). Turning that into a retirement gate means running the
// omission across the full CTS caselist on both devices, not the desktop corpus, and that is // omission across the full CTS caselist on both devices, not the desktop corpus.
// recorded as P3a work rather than done here on evidence that cannot support it. //
// P3a STATUS: still not done, and deliberately not done here. P3a is one of the five
// architecture boundaries that owe a full gl44to46 caselist run on both devices, so the
// omission sweep rides that run rather than duplicating it - the verdict lands with the
// caselist result at the phase exit, off the critical path, and every row above stays in
// place until it does. A row retired on desktop evidence would be retired on evidence that
// cannot support it, which is the reason this item exists rather than a scheduling excuse.
// --------------------------------------------------------------------------------------- // ---------------------------------------------------------------------------------------
// //
// gen_pipe.py's block regexes end at a blank line: keep the empty line after each macro. // gen_pipe.py's block regexes end at a blank line: keep the empty line after each macro.
+38 -6
View File
@@ -18,6 +18,14 @@ script reports every place in MG_Impl/GLImpl where a GL entry point BOTH mutates
state through pGLContext AND reaches the backend in the same function - those are the state through pGLContext AND reaches the backend in the same function - those are the
publish points, the ones that must map onto an aggregate generation. publish points, the ones that must map onto an aggregate generation.
P3a widens the scan root to MG_State/GLState as well, and with it reads the OTHER publish
mechanism: MGP_NOTE_MUTATION, which a state object spells when it moves a pushed PipeInputs
field from inside a backend's own verb. Those sites carry a FIELD name rather than a mutator
name and none of them is a `pGLContext->` call, so the scan attributes each to its ENCLOSING
function - which is the name a DirtySurface.def row is written against. Until this, the four
in TextureState.h were outside the gate entirely, which is the one of the three blind spots
P2 recorded that a scan can actually close.
P0 is the skeleton: it reports. P1 adds the mapping file and CI regenerates it with P0 is the skeleton: it reports. P1 adds the mapping file and CI regenerates it with
`git diff --exit-code` and zero unmapped mutators, the same shape as gen_pipe.py's G6. `git diff --exit-code` and zero unmapped mutators, the same shape as gen_pipe.py's G6.
@@ -51,13 +59,27 @@ import re
import sys import sys
REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
SCAN_ROOT = os.path.join(REPO_ROOT, "MobileGL", "MG_Impl", "GLImpl") # P3a WIDENS THE SCAN ROOT, and this is the one blind spot of the three the P2 review
# recorded that a scan can actually close (DirtySurface.def's header keeps the other two).
# `pGLContext->Set*` is not the only way a frontend mutation becomes observable: a state
# object that moves a PUSHED PipeInputs field from inside a backend's own verb publishes it
# with MGP_NOTE_MUTATION instead, and every one of those sites lives in MG_State/GLState -
# which the scan did not read at all, so the four in TextureState.h were outside the gate
# entirely. Reading both roots and both mechanisms is what makes "every mutation has an
# answer" a claim over the whole surface rather than over one directory of it.
SCAN_ROOTS = (os.path.join(REPO_ROOT, "MobileGL", "MG_Impl", "GLImpl"),
os.path.join(REPO_ROOT, "MobileGL", "MG_State", "GLState"))
# The mutating half of GLContext's surface. Prefix-matched, per the plan's list. # The mutating half of GLContext's surface. Prefix-matched, per the plan's list.
MUTATOR_PREFIXES = ("Add", "Set", "Mark", "Bump", "Allocate", "Truncate", "Record", "Notify", MUTATOR_PREFIXES = ("Add", "Set", "Mark", "Bump", "Allocate", "Truncate", "Record", "Notify",
"Begin", "End") "Begin", "End")
MUTATOR_RE = re.compile(r"pGLContext->\s*((?:%s)\w*)\s*\(" % "|".join(MUTATOR_PREFIXES)) MUTATOR_RE = re.compile(r"pGLContext->\s*((?:%s)\w*)\s*\(" % "|".join(MUTATOR_PREFIXES))
# The SECOND publish mechanism (MG_Pipe/PipeMutation.h). It carries the FIELD, not a mutator
# name, and it sits inside the state object rather than at a GL entry point - so the mutator
# it belongs to is the ENCLOSING function, which is what needs a row here. Reading it is what
# the widened root buys: MUTATOR_RE alone finds nothing at all under MG_State/GLState.
NOTE_MUTATION_RE = re.compile(r"MGP_NOTE_MUTATION\(\s*(\w+)\s*\)")
BACKEND_RE = re.compile(r"gBackendFunctionsTable\.GL\.(\w+)|pActiveBackendObject->\s*(\w+)") BACKEND_RE = re.compile(r"gBackendFunctionsTable\.GL\.(\w+)|pActiveBackendObject->\s*(\w+)")
FUNCTION_RE = re.compile(r"(?:^|\n)[ \t]*(?:[A-Za-z_][\w:<>,&*\s]*?)\b(\w+)\s*\(([^;{}]*)\)\s*" FUNCTION_RE = re.compile(r"(?:^|\n)[ \t]*(?:[A-Za-z_][\w:<>,&*\s]*?)\b(\w+)\s*\(([^;{}]*)\)\s*"
r"(?:const\s*)?(?:noexcept\s*)?\{") r"(?:const\s*)?(?:noexcept\s*)?\{")
@@ -150,6 +172,12 @@ def scan_file(path):
all_mutators = [(m.group(1), line_of(masked, m.start())) for m in MUTATOR_RE.finditer(masked)] all_mutators = [(m.group(1), line_of(masked, m.start())) for m in MUTATOR_RE.finditer(masked)]
for name, start, end in function_bodies(masked): for name, start, end in function_bodies(masked):
body = masked[start:end] body = masked[start:end]
# The push-on-mutation notice: the enclosing function is the mutator, because that is
# what a row in DirtySurface.def names and what a reader of this report has to look
# up. It is DEFERRED-shaped by construction (a state object does not reach the
# backend), so it joins all_mutators and never the immediate publish points below.
for note in NOTE_MUTATION_RE.finditer(body):
all_mutators.append((name, line_of(masked, start + note.start())))
mutators = [(m.group(1), line_of(masked, start + m.start())) for m in MUTATOR_RE.finditer(body)] mutators = [(m.group(1), line_of(masked, start + m.start())) for m in MUTATOR_RE.finditer(body)]
if not mutators: if not mutators:
continue continue
@@ -1495,9 +1523,10 @@ def analyse_snippet(text):
def scan_all(): def scan_all():
"""(findings-per-file, {mutator: call count}) over the whole scan root.""" """(findings-per-file, {mutator: call count}) over every scan root."""
sources = [] sources = []
for root, _, files in os.walk(SCAN_ROOT): for scan_root in SCAN_ROOTS:
for root, _, files in os.walk(scan_root):
for name in sorted(files): for name in sorted(files):
if name.endswith((".cpp", ".h")): if name.endswith((".cpp", ".h")):
sources.append(os.path.join(root, name)) sources.append(os.path.join(root, name))
@@ -1789,8 +1818,9 @@ def main():
help="run the canned negative controls; each must trip") help="run the canned negative controls; each must trip")
args = parser.parse_args() args = parser.parse_args()
if not os.path.isdir(SCAN_ROOT): for scan_root in SCAN_ROOTS:
sys.exit("missing %s" % SCAN_ROOT) if not os.path.isdir(scan_root):
sys.exit("missing %s" % scan_root)
if not os.path.isfile(DEF_PATH): if not os.path.isfile(DEF_PATH):
sys.exit("missing %s" % DEF_PATH) sys.exit("missing %s" % DEF_PATH)
if not os.path.isfile(RENDER_STATE_PATH): if not os.path.isfile(RENDER_STATE_PATH):
@@ -1891,7 +1921,9 @@ def main():
for mutator, line in finding["mutators"]: for mutator, line in finding["mutators"]:
print(" %-44s :%d %s" % (mutator, line, mapping.get(mutator, "UNMAPPED"))) print(" %-44s :%d %s" % (mutator, line, mapping.get(mutator, "UNMAPPED")))
print("\ndirty-surface: %d files scanned under MG_Impl/GLImpl" % len(sources)) print("\ndirty-surface: %d files scanned under %s"
% (len(sources), " + ".join(os.path.relpath(root, REPO_ROOT).replace(os.sep, "/")
for root in SCAN_ROOTS)))
print("dirty-surface: %d mutator calls in total, %d distinct mutators" % (deferred_mutators, print("dirty-surface: %d mutator calls in total, %d distinct mutators" % (deferred_mutators,
len(distinct_all))) len(distinct_all)))
print("dirty-surface: %d of them sit in %d IMMEDIATE PUBLISH POINTS - functions that also " print("dirty-surface: %d of them sit in %d IMMEDIATE PUBLISH POINTS - functions that also "