diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d84dfc5b..bee871ae 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -3113,11 +3113,19 @@ jobs: # The same shape for Espryt's re-keyed memos (P3b/P4b, PLAN-PH-P34B-P7.md 1.2). P4a/P5f # moved the resolved-binding, sampler-pass, image-sweep, twin-registry, unit-sampler and # twin-lookup memos onto {slot, gen} handles and applier serials; the legacy pointer-keyed - # arms stayed behind MOBILEGL_PIPE_LEGACY_MEMOS / !MOBILEGL_BUILD_DISAGGREGATED because - # they are the PULL build's only arms and G1 pins that build's .text. Nothing checked that - # a NEW frontend-pointer key could not appear outside those arms. --self-test runs the - # negative controls and then the real scan in the same invocation, as its sibling does. - - name: Espryt memo keys carry no frontend pointer outside the legacy arms + # arms stayed because they are the PULL build's only arms and G1 pins that build's .text. + # Nothing checked that a NEW frontend-pointer key could not appear outside them. + # + # MOBILEGL_PIPE_LEGACY_MEMOS IS A POLICY EXEMPTION, NOT AN ABSENCE. It defaults ON + # (CMakeLists.txt:43) and no job here clears it, so the split builds in this workflow DO + # compile those arms - TwinLookupMemo among them. The gate's rule is "the legacy arm may + # keep pointer keys", which is a decision; only !MOBILEGL_BUILD_DISAGGREGATED is code a + # split build genuinely never sees. + # + # --self-test runs the negative controls and then the real scan in one invocation, as its + # sibling does; the scan also asserts that every memo family's anchor still matches and + # that every allow-list entry still suppresses something. + - name: Espryt memo keys carry no frontend pointer outside the exempted legacy arm run: python3 scripts/ci/espryt_memo_purity.py --self-test # The same question for the symbol tool the P1 gate is written in terms of. diff --git a/scripts/ci/espryt_memo_purity.py b/scripts/ci/espryt_memo_purity.py old mode 100755 new mode 100644 index 1222add4..8f961e78 --- a/scripts/ci/espryt_memo_purity.py +++ b/scripts/ci/espryt_memo_purity.py @@ -4,13 +4,27 @@ P4a/P5f re-keyed the resolved-binding, sampler-pass, image-sweep and twin-registry memos onto {slot, gen} handles and applier serials. The legacy pointer-keyed arms were not deleted: they are the PULL build's only arms and G1 pins that build's -.text, so they stay behind MOBILEGL_PIPE_LEGACY_MEMOS or !MOBILEGL_BUILD_DISAGGREGATED -and retire with the pull path itself (Config.h:395-399). This gate does not pretend -the pointer types disappeared; it rejects a frontend-pointer KEY in the arm a split -build compiles, inside the named memo families, and nothing else. It does not -evaluate the preprocessor - it recognises the four conditional spellings the tree -actually uses - and it does not look at DirectVulkan, whose own key inventory is +.text, so they retire with the pull path itself (Config.h:395-399). + +WHAT THE TWO GUARDS MEAN HERE IS A POLICY EXEMPTION, NOT AN ABSENCE, and the first +version of this docstring got that wrong. MOBILEGL_PIPE_LEGACY_MEMOS defaults ON +(CMakeLists.txt:43) and nothing in test.yml clears it, so a CI split build DOES +compile the #if MOBILEGL_PIPE_LEGACY_MEMOS arms - TwinLookupMemo (DirectGLES.cpp:114) +sits inside one and is compiled into every lane this repo runs. The rule is therefore +"the legacy arm is ALLOWED to keep pointer keys", not "the split build never sees +them". !MOBILEGL_BUILD_DISAGGREGATED is the genuinely-absent one. + +This gate does not pretend the pointer types disappeared; it rejects a frontend-pointer +KEY outside those two exempted arms, inside the named memo families, and nothing else. +It does not evaluate the preprocessor - it recognises the conditional spellings the +tree actually uses - and it does not look at DirectVulkan, whose own key inventory is P7 wave 2's. + +Two structural self-checks keep the gate honest, because both failure modes are silent: +a renamed struct would drop a whole family and still pass, and an allow-list entry no +rule can consult would sit there documenting a carve-out that is not happening. So every +FAMILIES anchor must match at least one span in the real sources, and every ALLOW entry +must actually suppress something. """ from __future__ import annotations import argparse @@ -26,7 +40,15 @@ SOURCES = [ # registry's template parameter and is the ONLY spelling its map key ever has, so a # gate written in terms of the concrete names alone would read Managers.h and find # nothing. -FRONTEND = (r"(?:ITextureObject|TextureObject|SamplerObject|ProgramObject|FramebufferObject" +# +# THE LEADING (? rows` and +# `SamplerImpl::BackendSamplerObject* backend` - false positives from FRONTEND's missing +# word boundary, fixed above rather than exempted; +# * `BackendMap m_entries`, which names a typedef and no frontend type at all; +# * `const void* program` and `Uint64 programLifetimeId`, which no rule can consult - +# `void` is not in FRONTEND and neither line calls GetLifetimeId(). +# ResolvedTextureBindingMemo's type-erased monolith key is still there and still deliberate +# (it is nulled under `byHandle`, DirectGLES.cpp:7369-7373, and the pull build needs it for +# G1) - it simply never needed an exemption, and saying it did was misleading. The +# allow-liveness check below is what stops this list growing that way again. def code_only(source: str) -> str: @@ -129,13 +150,28 @@ def family_spans(code: str) -> list[tuple[str, int, int]]: return spans +ALLOW_HITS: set[int] = set() + + def allowed(path: str, line_text: str) -> str: - for allow_path, needle, reason in ALLOW: + for index, (allow_path, needle, reason) in enumerate(ALLOW): if path == allow_path and needle in line_text: + ALLOW_HITS.add(index) return reason return "" +def missing_families(path: str, text: str) -> list[str]: + """Anchors that matched NO span in this file's sources, when they should have. + + A renamed struct is the silent failure this exists for: family_spans() simply + returns fewer spans, every rule keeps passing, and the family stops being checked + without anything saying so.""" + code = code_only(text) + seen = {name for name, _, _ in family_spans(code)} + return [name for name, _ in FAMILIES if name not in seen] + + def violations(path: str, text: str) -> list[str]: code = code_only(text) lines = code.splitlines() @@ -203,11 +239,26 @@ def self_test() -> None: "#endif\n};\n"), "the #else of a DISAGGREGATED arm must stay green" assert not violations(gles, "struct ResolvedTextureBindingMemo {\n" " const void* program = nullptr;\n};\n"), \ - "the allow-listed type-erased monolith key must stay green" + "the type-erased monolith key must stay green without needing an exemption" assert not violations(gles, "void Unrelated(ProgramObject* program) { Use(program); }\n"), \ "a frontend pointer outside every memo family is not this gate's business" + # THE WORD-BOUNDARY CONTROLS. Both of these are memo VALUES holding BACKEND twins, and + # both used to be reported as frontend keys because FRONTEND matched as a suffix. + assert not violations(gles, "struct UnitSamplerLookupMemo {\n" + " SamplerImpl::BackendSamplerObject* backend = nullptr;\n};\n"), \ + "a BackendSamplerObject value must not match SamplerObject" + assert not violations(managers, "struct SamplerPassMemo {\n" + " Array rows{};\n};\n"), \ + "a BackendSamplerObject row must not match SamplerObject" + # ...and the qualified spelling still must, or the boundary went in too tight. + assert violations(gles, "struct ResolvedTextureBindingMemo {\n" + " MG_State::GLState::ProgramObject* key = nullptr;\n};\n"), \ + "a :: before the name is not a word character and must still match" + # THE FAMILY ANCHORS. Renaming a struct must be loud, not silent. + assert missing_families(gles, "struct SomethingElse {\n};\n"), \ + "a source with none of the anchors must report every family as missing" print(f"espryt memo purity negative controls: {len(controls)} named failures observed, " - f"{len(ALLOW)} allow-list entries carried") + f"{len(ALLOW)} allow-list entr(y/ies) carried") def main() -> int: @@ -216,13 +267,39 @@ def main() -> int: args = parser.parse_args() if args.self_test: self_test() - failures = [failure for name in SOURCES - for failure in violations(name, (ROOT / name).read_text(encoding="utf-8"))] + ALLOW_HITS.clear() + sources = {name: (ROOT / name).read_text(encoding="utf-8") for name in SOURCES} + failures = [failure for name, text in sources.items() for failure in violations(name, text)] + + # STRUCTURAL CHECK 1: every family anchor found its declaration somewhere. Each family + # lives in exactly one of the two sources, so the test is over their union. + found_anywhere: set[str] = set() + for text in sources.values(): + found_anywhere |= {name for name, _, _ in family_spans(code_only(text))} + for name, anchor in FAMILIES: + if name not in found_anywhere: + failures.append( + f"{SOURCES[0]}:0: the anchor for {name} matched nothing in either source. It was " + f"renamed, moved or reformatted, and this gate has silently stopped checking that " + f"family - every rule below still passes. Fix the anchor ({anchor!r}) or drop the " + f"family from FAMILIES with a reason.") + + # STRUCTURAL CHECK 2: every allow-list entry actually suppressed something. An entry no + # rule can consult documents a carve-out that is not happening, which is worse than no + # entry at all - it is a claim about the tree that nothing verifies. + for index, (allow_path, needle, reason) in enumerate(ALLOW): + if index not in ALLOW_HITS: + failures.append( + f"{allow_path}:0: the allow-list entry for {needle!r} suppressed nothing on this " + f"tree. Either the code it exempted is gone - delete the entry - or a rule stopped " + f"matching it, in which case the exemption is hiding a gap. Its reason was: {reason}") + if failures: print("\n".join(failures)) return 1 - print("espryt memo key purity gate: PASS (the legacy pointer-keyed arms behind " - "MOBILEGL_PIPE_LEGACY_MEMOS / !MOBILEGL_BUILD_DISAGGREGATED are retained on purpose)") + print(f"espryt memo key purity gate: PASS ({len(FAMILIES)} families anchored, " + f"{len(ALLOW)} allow-list entr(y/ies), all consulted; the legacy pointer-keyed arm " + f"behind MOBILEGL_PIPE_LEGACY_MEMOS is a policy exemption, not an absence)") return 0