From 4ba14fa4ed2658605507c9c7f21bedb7bc7afedd Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 05:42:12 -0400 Subject: [PATCH 01/10] [Test] (MG_Test, Wire): ID-43 - RingTest.TheTwoFlagSpacesAreDisjointByTranslation states the tree: a header stamped with the CALL flags outside Reserve is POPPED with its lying kRecPad/kRecBorrowSlot bits visible, and the control that a genuine filler still vanishes names Pop kind check as the defence --- MobileGL/MG_Test/Wire/RingTest.cpp | 68 ++++++++++++++++++++++++++---- 1 file changed, 60 insertions(+), 8 deletions(-) diff --git a/MobileGL/MG_Test/Wire/RingTest.cpp b/MobileGL/MG_Test/Wire/RingTest.cpp index 56d8e0c7..8ffa604e 100644 --- a/MobileGL/MG_Test/Wire/RingTest.cpp +++ b/MobileGL/MG_Test/Wire/RingTest.cpp @@ -702,9 +702,11 @@ TEST(RingTest, TheTwoFlagSpacesAreDisjointByTranslation) { static_assert(Call(kHasBlob) == Rec(kRecHasBlob), "bit 1 stopped agreeing"); // The three that collide. Each of these is a live defect if it is ever passed through, - // and the middle one is the worst: RingConsumer::Pop treats kRecPad as a wrap filler and - // SKIPS the record, so a stamped kVarTail deletes every variable-tail call from the - // stream with no error raised anywhere. + // and the middle one was the worst: kRecPad is the flag RingConsumer::Pop reads as "wrap + // filler", so on the flag alone a stamped kVarTail would delete every variable-tail call + // from the stream with no error raised anywhere. Pop does not read it on the flag alone - + // it requires kind == kRingPadRecordKind beside it - and the runtime half below is the + // control on exactly that. static_assert(Call(kVarTail) == Rec(kRecPad), "the kVarTail/kRecPad collision moved"); static_assert(Call(kHostSpan) == Rec(kRecBorrowSlot), "the kHostSpan/kRecBorrowSlot collision moved"); static_assert(Call(kReplySlot) == Rec(kRecVarTail), "the kReplySlot/kRecVarTail collision moved"); @@ -757,8 +759,17 @@ TEST(RingTest, TheTwoFlagSpacesAreDisjointByTranslation) { // The one that Reserve cannot defend: a producer that writes the header ITSELF rather than // letting Reserve write it - which is precisely what a codec with its own header struct // does, since MGPWireRecHeader and RingRecordHeader are the same eight bytes. Then the - // mask is not in the path, Pop sees kRecPad, and the record is skipped as a wrap filler - // with no error raised anywhere. + // mask is not in the path and Pop sees kRecPad on a record that is not a filler. + // + // What saves it is the KIND. RingConsumer::Pop skips a record only when it carries kRecPad + // AND kind == kRingPadRecordKind (Ring.cpp: `(header.flags & kRecPad) != 0 && header.kind == + // kRingPadRecordKind` - "BOTH, not just the flag"). Kind 0 is the wrap filler's and nothing + // else's, because the call catalogue starts at 1. So the stamped record is DELIVERED, lies + // and all, and the decoder can reject it by name - which it can only do because it got it. + // THE ASSERT AND THE EXPECTS BELOW ARE THE CONTROL ON THAT KIND CHECK: delete the + // `&& header.kind == kRingPadRecordKind` half of Pop's condition and this record vanishes + // into the wrap-filler skip again, exactly as it did before the pair was required, and this + // case goes red on the ASSERT's own message. That perturbation was run. void* second = ring.Producer().Reserve(static_cast(MGPWireOp::DrawVbo), kRecNone, 16); ASSERT_NE(second, nullptr); @@ -771,10 +782,51 @@ TEST(RingTest, TheTwoFlagSpacesAreDisjointByTranslation) { sizeof(stamped)); ring.Producer().Publish(); + ASSERT_TRUE(ring.Consumer().Pop(view)) + << "the stamped record vanished into Pop's wrap-filler skip. Pop's kind check - a filler " + "must carry kind == kRingPadRecordKind as well as kRecPad - is the only thing standing " + "between a header stamped with the CALL flags and every var-tail call being deleted " + "from the stream with nothing logged on either side"; + EXPECT_EQ(view.kind, static_cast(MGPWireOp::DrawVbo)) + << "kind is what Pop tells a real record from a filler by, and a filler's is " + "kRingPadRecordKind"; + + // Delivered is not the same as correct. The stamped bits arrive verbatim, and they are + // exactly the collisions the table above names: bit 2 (kVarTail -> kRecPad) is why this + // record looked like a filler at all, and bit 3 (kHostSpan -> kRecBorrowSlot) still makes it + // claim a slot in the GPU timeline it never borrowed. draw_vbo is not a kReplySlot call, so + // bit 4 (kReplySlot -> kRecVarTail) is clear here; on a blocking call it would lie too. + EXPECT_EQ(view.flags, static_cast(drawVboCallFlags)) + << "the header did not arrive as it was stamped"; + EXPECT_NE(view.flags & static_cast(kRecPad), 0u) + << "the kVarTail/kRecPad collision arrives intact - the kind check narrows the SKIP, it " + "does not scrub the bit, and naming this record is the decoder's job"; + EXPECT_NE(view.flags & static_cast(kRecBorrowSlot), 0u) + << "the kHostSpan/kRecBorrowSlot collision is what makes a stamped header lie about " + "this record's lifetime"; + EXPECT_EQ(view.flags & static_cast(kRecVarTail), 0u) + << "draw_vbo started carrying kReplySlot; then the third collision lies here too"; + EXPECT_EQ(view.payloadSize, 16u); + EXPECT_TRUE(ring.Invariants()); + + // The other side of the same control, so that "the record is popped" cannot be satisfied by + // simply not skipping anything: kRecPad on a header whose kind IS kRingPadRecordKind is a + // genuine wrap filler and still vanishes. Pop's check was narrowed to the pair, not removed. + void* filler = ring.Producer().Reserve(kRingPadRecordKind, kRecNone, 16); + ASSERT_NE(filler, nullptr); + std::memset(filler, 0xEF, 16); + RingRecordHeader asFiller{}; + std::memcpy(&asFiller, static_cast(filler) - sizeof(RingRecordHeader), + sizeof(asFiller)); + asFiller.flags = static_cast(kRecPad); + std::memcpy(static_cast(filler) - sizeof(RingRecordHeader), &asFiller, + sizeof(asFiller)); + ring.Producer().Publish(); + EXPECT_FALSE(ring.Consumer().Pop(view)) - << "a header stamped with the CALL flags outside Reserve should vanish into Pop's " - "wrap-filler skip - if it did not, the collision has moved and this case is no " - "longer the control it was"; + << "a header carrying BOTH kRecPad and kind kRingPadRecordKind is a wrap filler and has " + "to be skipped; if it reaches a caller the skip is gone, not narrowed"; + EXPECT_TRUE(ring.Invariants()); // And the same record framed the way the encoder actually frames it - translated, with the // ring's own var-tail bit - round-trips intact. From f63c9483de3f9baeb0b49abf9c9a77235ed4eca9 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 05:56:08 -0400 Subject: [PATCH 02/10] [Fix] (scripts): compare G5's eleventh row against its pin ALWAYS and re-pin it on the reviewed P5 body, with a negative control that the pin itself can fail --- scripts/p3a_untouched_regions.sh | 214 +++++++++++++++++++++++++----- scripts/p4a_untouched_regions.sh | 215 ++++++++++++++++++++++++++----- 2 files changed, 364 insertions(+), 65 deletions(-) diff --git a/scripts/p3a_untouched_regions.sh b/scripts/p3a_untouched_regions.sh index 96e553e3..1f97bed7 100644 --- a/scripts/p3a_untouched_regions.sh +++ b/scripts/p3a_untouched_regions.sh @@ -37,7 +37,20 @@ # FlushPendingRangesNow at all, so a gate that hashed only that name protected text the shipping # build never sees, and a tier-threshold edit made in the ladder that DOES ship would pass it. So # both names are hashed. The pull build's ladder is compared against the base ref; the push build's -# is compared against a sha pinned at the commit where it was reviewed - see PINNED_FUNCTIONS. +# is compared, ALWAYS AND WHETHER OR NOT DEFINES IT, against a sha pinned at the commit +# where that body was reviewed - see PINNED_FUNCTIONS. +# +# "ALWAYS" IS INTEGRATOR DECISION ID-41 AND IT IS A CHANGE. Until P5 this text said "pinned" while +# the implementation consulted the pin only as a FALLBACK, when did not define the function +# at all. At P3a's own base ref it does not, so the two readings agreed and nobody noticed; from +# ff2994d9 onward it does, so the fallback stopped firing and the row silently went back to being +# ref-a-vs-ref-b. ID-41 rules that the pin is the baseline, because the pin is the REVIEWED text: +# P5 (b1) gave this body two defaulted parameters (hostBaseFrom/hostBaseTo), a +# MOBILEGL_PIPE_VERIFY-only StageSnapshotTooNarrow log and a tier-1 access computation moved into +# InvalidateFlushAccessFor, the pull arm (FlushPendingRangesNow) is byte-identical across that +# change and G1 reports .text +0, so it is the intended change to the eleventh row rather than +# drift - and the answer is to RE-PIN it, not to revert it and not to let the row stop being +# compared. The OTHER TEN stay ref-a-vs-ref-b: nothing about them moved. # # THE TENTH IS HERE BY INTEGRATOR DECISION ID-11, resolving a contradiction inside the brief. # D-F's "Decision: nine functions" table omits FlushPendingRangesNow, but BRIEF-P3A.md:420 calls it @@ -88,12 +101,22 @@ set -u -o pipefail SOURCE_PATH=MobileGL/MG_Backend/DirectGLES/Managers.cpp # The ten that exist at the P3a base ref, so their baseline is read out of . FUNCTIONS="IsPoolable EnrollIntoPool AcquireFromPool TrimBufferPool ClearBufferPool ProcessDeferredBufferReleases CreateRingStorage RingAvailable RingAllocate FlushPendingRangesNow" -# The ELEVENTH (ID-15), and it is a different kind of row: it was BORN in P3a, so there is no -# body at the base ref to compare it with and its baseline is PINNED below, captured at -# 3e298c9a - the commit at which the two-arm shape was reviewed and accepted. +# The ELEVENTH (ID-15), and it is a different kind of row: it was BORN in P3a, so there was no +# body at the base ref to compare it with, and its baseline is PINNED below and consulted +# UNCONDITIONALLY (ID-41 - see the long note at the top of this file). +# +# THE PIN, AND WHAT RE-PINS IT. Whoever moves this body deliberately replaces BOTH lines and +# writes the decision beside them; a pin with no commit and no decision next to it is a number +# nobody can audit. +# 3e298c9a 37fc94ff... ID-15, P3a: the two-arm shape, reviewed and accepted +# 3dadd4c1 172b0222... ID-41, P5 (b1): [Fix] (DirectGLES): make the extent hostBase is good +# for a parameter of the flush ladder, so tier 1's widening refusal is +# live code the moment a SEG_STAGE snapshot is narrower than the queued +# range. <- CURRENT PINNED_FUNCTIONS="FlushPendingRangesFrom" -PINNED_BASELINE_REF=3e298c9a -PINNED_SHA_FlushPendingRangesFrom=37fc94ffc5991923d222d585daa3af6511d2352d255623026ce35a3b6963c4a6 +PINNED_BASELINE_REF=3dadd4c1 +PINNED_BASELINE_DECISION=ID-41 +PINNED_SHA_FlushPendingRangesFrom=172b022273db01b16e772d15b269ffcd797fe38c767f7354d83ce113a66040d0 ALL_FUNCTIONS="$FUNCTIONS $PINNED_FUNCTIONS" EXPECTED_FUNCTION_COUNT=11 # The functions the self-test perturbs, one control each. ClearBufferPool is small, has no forward @@ -247,7 +270,7 @@ def extract(path, names): return rows, problems -def perturb(src, dst, names, target): +def perturb(src, dst, names, target, one_token=False): text = open(src, encoding='utf-8', newline='').read() masked = mask(text) hits = find_definition(text, masked, target) @@ -260,9 +283,17 @@ def perturb(src, dst, names, target): # perturbation somewhere that is not the body, and the control would be proving the wrong # thing. Offsets are identical between the two by construction (mask() preserves length). brace = masked.index('{', begin) - patched = (text[:brace + 1] + - '\n // p3a_untouched_regions.sh --self-test: a body that MOVED.\n' + - text[brace + 1:]) + if one_token: + # ONE TOKEN - a single empty statement - and nothing else. ID-41(d) asks the pinned row's + # control to perturb the LADDER rather than a comment beside it, and this is the smallest + # edit that is unambiguously code: a reader cannot answer "the gate only notices comments". + # The perturbed copy is never compiled, only hashed, so an empty statement is legal here in + # a way it would not be in the tree. + patched = text[:brace + 1] + ';' + text[brace + 1:] + else: + patched = (text[:brace + 1] + + '\n // p3a_untouched_regions.sh --self-test: a body that MOVED.\n' + + text[brace + 1:]) open(dst, 'w', encoding='utf-8', newline='').write(patched) return 0 @@ -282,8 +313,8 @@ def main(argv): for sha, name in rows: sys.stdout.write('%s %s\n' % (sha, name)) return 2 if problems else 0 - if mode == 'perturb': - return perturb(argv[2], argv[3], names, argv[4]) + if mode in ('perturb', 'perturb-token'): + return perturb(argv[2], argv[3], names, argv[4], mode == 'perturb-token') sys.stderr.write('[p3a-untouched] unknown mode %r\n' % mode) return 2 @@ -304,38 +335,74 @@ extract_ref() { return $? } -# The BASELINE side (). Same extraction, with one difference that the eleventh row makes -# necessary: a function born in P3a has no body at the P3a base ref, and CI passes exactly that -# ref as . Asking for it there is not "the gate could not run" - it is the expected -# answer - so a name in PINNED_FUNCTIONS that is missing at takes the sha pinned at the -# top of this script instead. Everything else still has to be found: the fallback is entered -# only after a strict extraction failed, and it then re-extracts the pre-P3a ten strictly, so a -# genuine rename of one of THOSE is still exit 2 rather than a silently short list. +# True when $1 is one of the rows whose baseline is the PIN rather than . +is_pinned_row() { + local name candidate + for candidate in $PINNED_FUNCTIONS; do + [ "$candidate" = "$1" ] && return 0 + done + return 1 +} + +# Overwrite the pinned rows of a ` ` list with the shas PINNED at the top of this +# script. ONE spelling of the substitution, called both by extract_baseline and by --self-test's +# pin controls, so the control drives the gate's own path instead of re-spelling it - a control +# that re-implements what it checks proves only that the copy agrees with itself. +apply_pinned_shas() { + local file=$1 name pinned + for name in $PINNED_FUNCTIONS; do + eval "pinned=\$PINNED_SHA_$name" + if [ -z "$pinned" ] || [ "$pinned" = "PLACEHOLDER_SHA" ]; then + say "$name has no pinned baseline sha; the eleventh row cannot be compared" + return 2 + fi + grep -v " $name\$" "$file" > "$file.unpinned" || true + mv -f "$file.unpinned" "$file" || return 2 + # Appended LAST, which is also its position in the fixed order (it is the eleventh of eleven), + # so the header's "stdout is always the sha list in the fixed order" stays true and a baseline + # captured by redirect still diffs cleanly against a two-ref run. + printf '%s %s\n' "$pinned" "$name" >> "$file" + done + return 0 +} + +# The BASELINE side (). The TEN are extracted strictly, so a rename of one of THOSE is +# exit 2 rather than a silently short list. FlushPendingRangesFrom then takes the PINNED sha +# WHETHER OR NOT defines it (ID-41), and a that defines it DIFFERENTLY is +# reported - loudly - because the two answers disagreeing is itself a finding rather than a +# reason to prefer the ref. extract_baseline() { - local ref=$1 out=$2 blob="$WORK_DIR/$2.cpp" name sha + local ref=$1 out=$2 blob="$WORK_DIR/$2.cpp" name pinned atRef if ! git show "$ref:$SOURCE_PATH" > "$blob" 2>"$WORK_DIR/show.err"; then say "cannot read $SOURCE_PATH at '$ref':" sed 's/^/[p3a-untouched] /' "$WORK_DIR/show.err" >&2 return 2 fi - if python3 "$PY" extract "$blob" "$ALL_FUNCTIONS" > "$WORK_DIR/$out.sha" 2>"$WORK_DIR/$out.err"; then - return 0 - fi - if ! python3 "$PY" extract "$blob" "$FUNCTIONS" > "$WORK_DIR/$out.sha"; then + if ! python3 "$PY" extract "$blob" "$FUNCTIONS" > "$WORK_DIR/$out.sha" 2>"$WORK_DIR/$out.err"; then say "the baseline ref '$ref' does not define the pre-P3a ten exactly once each:" sed 's/^/[p3a-untouched] /' "$WORK_DIR/$out.err" >&2 return 2 fi for name in $PINNED_FUNCTIONS; do - eval "sha=\$PINNED_SHA_$name" - if [ -z "$sha" ] || [ "$sha" = "PLACEHOLDER_SHA" ]; then - say "$name has no pinned baseline sha; the eleventh row cannot be compared" - return 2 + eval "pinned=\$PINNED_SHA_$name" + atRef=$(python3 "$PY" extract "$blob" "$name" 2>/dev/null | awk -v n="$name" '$2 == n { print $1 }') + if [ -z "$atRef" ]; then + say "$name is not defined at '$ref' (it was born in P3a): its baseline is the sha PINNED in" + say " this script, captured at $PINNED_BASELINE_REF ($PINNED_BASELINE_DECISION)" + elif [ "$atRef" != "$pinned" ]; then + say "NOTE: $name IS defined at '$ref' and hashes" + say " $atRef, which is not the pin" + say " ($pinned," + say " captured at $PINNED_BASELINE_REF, $PINNED_BASELINE_DECISION). THE PIN IS WHAT IS" + say " COMPARED - it is the reviewed body - and this note is not a verdict in either" + say " direction. If '$ref' PREDATES $PINNED_BASELINE_REF the two SHOULD disagree: the pinned" + say " body is the change $PINNED_BASELINE_DECISION admitted, which is why it was re-pinned" + say " rather than reverted. If it does not predate it, the ladder that ships has moved away" + say " from the reviewed text without this gate being re-pinned - re-pin deliberately or" + say " revert, but do not leave them disagreeing." fi - printf '%s %s\n' "$sha" "$name" >> "$WORK_DIR/$out.sha" - say "$name is not defined at '$ref' (it was born in P3a): its baseline is the sha PINNED in" - say " this script, captured at $PINNED_BASELINE_REF" done + apply_pinned_shas "$WORK_DIR/$out.sha" || return 2 return 0 } @@ -350,6 +417,16 @@ compare_lists() { say "FIRST FUNCTION THAT MOVED: $name" say " $labelA $shaA" say " $labelB ${shaB:-}" + if is_pinned_row "$name"; then + # ITS OWN MESSAGE, and that is R-16 rather than decoration: the pinned row and the ten + # ref-a rows fail differently and are fixed differently, so a reader who sees only the + # generic paragraph below goes looking for a diff against that does not exist. + say " $name IS A PINNED ROW ($PINNED_BASELINE_DECISION): its baseline is ALWAYS the sha" + say " PINNED in this script - the body reviewed at $PINNED_BASELINE_REF - and never the" + say " body at '$labelA'. So this is not a diff against the base ref: the ladder that" + say " SHIPS has moved away from the text that was reviewed. Either re-pin deliberately," + say " replacing the sha AND the commit AND the decision beside it, or revert the body." + fi say " G5 (ARCHITECTURE.md:316, :515) says the buffer pool, the deferred-release drain, the" say " three rings and BOTH arms of the three-tier flush drain - FlushPendingRangesNow in" say " the pull build, FlushPendingRangesFrom in the push build (BRIEF-P3A.md:420, :1708," @@ -436,6 +513,81 @@ if [ "${1:-}" = "--self-test" ]; then fi say "negative control: a perturbed $target body is reported, and named" done + + # --- THE PINNED ROW (ID-41) ----------------------------------------------------------------- + # TWO more controls, and they exist because none of the five above can see the pin at all: every + # one of them compares one extraction of the working tree against another, so they would all be + # green on a build of this script in which PINNED_SHA_* was never read by anything. The eleventh + # row's whole claim is "the baseline is the PIN, not ", and that claim needs its own two. + for target in $PINNED_FUNCTIONS; do + eval "pinned=\$PINNED_SHA_$target" + + # (1) PIN PRECEDENCE, positive. A baseline that carries some OTHER sha for the pinned row - + # which is the shape of every CI passes today, since ff2994d9 and 37da3c3a both DEFINE + # FlushPendingRangesFrom - must come out of apply_pinned_shas carrying the PIN. This is the + # control that would have caught the ID-41 defect itself: before it, the pin was consulted + # only when lacked the function, so the row silently reverted to ref-a-vs-ref-b the + # moment a base ref had one. + grep -v " $target\$" "$WORK_DIR/pristine.sha" > "$WORK_DIR/pinprec.sha" || true + printf '%s %s\n' \ + "0000000000000000000000000000000000000000000000000000000000000000" "$target" \ + >> "$WORK_DIR/pinprec.sha" + apply_pinned_shas "$WORK_DIR/pinprec.sha" || exit 2 + got=$(awk -v n="$target" '$2 == n { print $1 }' "$WORK_DIR/pinprec.sha") + if [ "$got" != "$pinned" ]; then + say "PIN CONTROL FAILED: a baseline that carried a DIFFERENT sha for $target came out as" + say " '${got:-}' and not as the pin ($pinned). The eleventh row would be compared" + say " against again, which is exactly the defect $PINNED_BASELINE_DECISION closed." + exit 2 + fi + say "pin control: a baseline that defines $target differently is overridden by the PIN" + + # (2) A ONE-TOKEN EDIT TO THE PINNED LADDER, negative, AGAINST THE PIN. The comparison must go + # red, must name the row, and must say that the row is PINNED - R-16's "a control asserts its + # OWN failure string": the pinned row and the ten ref-a rows are fixed differently, and a + # reader who gets only the generic paragraph goes looking for a diff against that does + # not exist. + python3 "$PY" perturb-token "$WORK_DIR/pristine.cpp" "$WORK_DIR/pinperturbed.cpp" \ + "$target" "$ALL_FUNCTIONS" || exit 2 + python3 "$PY" extract "$WORK_DIR/pinperturbed.cpp" "$ALL_FUNCTIONS" \ + > "$WORK_DIR/pinperturbed.sha" || exit 2 + cp -f "$WORK_DIR/pristine.sha" "$WORK_DIR/pinbase.sha" || exit 2 + apply_pinned_shas "$WORK_DIR/pinbase.sha" || exit 2 + if compare_lists "$WORK_DIR/pinbase.sha" "$WORK_DIR/pinperturbed.sha" \ + "PIN($PINNED_BASELINE_REF)" "one-token-perturbed" 2> "$WORK_DIR/pinperturbed.err"; then + say "NEGATIVE CONTROL DID NOT TRIP: one token was inserted into $target's body and the" + say "comparison AGAINST THE PIN still reported every function as identical. The pinned row is" + say "not being compared at all, so every green this gate has printed for it means nothing." + exit 2 + fi + if ! grep -q "FIRST FUNCTION THAT MOVED: $target" "$WORK_DIR/pinperturbed.err"; then + say "NEGATIVE CONTROL TRIPPED FOR THE WRONG REASON: the comparison against the pin went red" + say "but did not name $target as the first function that moved. It said:" + sed 's/^/[p3a-untouched] /' "$WORK_DIR/pinperturbed.err" >&2 + exit 2 + fi + if ! grep -q "$target IS A PINNED ROW" "$WORK_DIR/pinperturbed.err"; then + say "NEGATIVE CONTROL TRIPPED FOR THE WRONG REASON: it went red and named $target, but did" + say "not say that this row's baseline is the PIN. That is the half a reader acts on, and a" + say "control that does not assert its own message is not a control (R-16). It said:" + sed 's/^/[p3a-untouched] /' "$WORK_DIR/pinperturbed.err" >&2 + exit 2 + fi + say "negative control: a ONE-TOKEN edit to the pinned $target body goes red AGAINST THE PIN," + say " is named, and says the row is pinned" + + # NOT a failure, deliberately: --self-test is about whether the comparison works, and it runs + # on the WORKING tree, which may legitimately carry an uncommitted edit. The two-ref gate is + # what fails when the committed ladder has left the pin. But say so, because a self-test that + # was green on a tree whose ladder no longer matches its pin is confusing in exactly one + # direction. + treeSha=$(awk -v n="$target" '$2 == n { print $1 }' "$WORK_DIR/pristine.sha") + if [ "$treeSha" != "$pinned" ]; then + say "NOTE: this working tree's $target hashes $treeSha, not the pin ($pinned). The" + say " self-test's verdict is unaffected; the two-ref gate will be RED until you re-pin or" + say " revert." + fi + done say "self-test passed" exit 0 fi diff --git a/scripts/p4a_untouched_regions.sh b/scripts/p4a_untouched_regions.sh index 5a7ac836..fa23391a 100644 --- a/scripts/p4a_untouched_regions.sh +++ b/scripts/p4a_untouched_regions.sh @@ -110,12 +110,21 @@ # next reader of this file meets them: # D-N/1 the namespace region kind, above. # D-N/2 the PINNED baseline is CONSULTED UNCONDITIONALLY for FlushPendingRangesFrom, where the -# parent consults it only when does not define the function. D-N says that row is -# compared "against its pinned 3e298c9a sha", and at P4a's base ref the function DOES -# exist - so the parent's fallback would silently never fire and the pin would stop being -# the baseline the brief names. Both readings agree on this tree (measured: the body at -# 37da3c3a hashes to the pinned value); where they would ever disagree, this script says -# so on stderr and keeps the PIN, because the pin is the reviewed text. +# parent consulted it only when did not define the function. D-N says that row is +# compared "against its pinned sha", and at P4a's base ref the function DOES exist - so +# the parent's fallback would silently never fire and the pin would stop being the +# baseline the brief names. INTEGRATOR DECISION ID-41 has since made this the parent's +# reading too, so D-N/2 is no longer a deviation between the two scripts; it is kept here +# as the record of why this one got there first. +# +# The two answers now DISAGREE on every ref CI passes, and that is the expected state +# rather than a finding: P5 (b1) re-parameterised the shipping ladder +# (hostBaseFrom/hostBaseTo, a MOBILEGL_PIPE_VERIFY-only StageSnapshotTooNarrow log, the +# tier-1 access computation moved into InvalidateFlushAccessFor), the PULL arm is +# byte-identical across that change and G1 reports .text +0, and ID-41 ruled that the row +# be RE-PINNED on the reviewed P5 body rather than reverted or quietly left comparing +# against . This script says so on stderr, in both directions, and keeps the PIN - +# because the pin is the reviewed text. set -u -o pipefail # One row per region: @@. The ORDER is the fixed order the sha list is printed @@ -143,11 +152,20 @@ ShouldUseCaveatTextureFormat@function@MobileGL/MG_Backend/DirectGLES/Utils.cpp" EXPECTED_FUNCTION_COUNT=17 # The one region born in P3a, so there is no body at P4a's base ref that this phase reviewed: its -# baseline is the sha captured at 3e298c9a, the commit at which the two-arm shape was reviewed and -# accepted (ID-15). See DEVIATIONS D-N/2 for why it is consulted unconditionally. +# baseline is PINNED and consulted UNCONDITIONALLY (DEVIATIONS D-N/2, and ID-41 for the parent). +# +# THE PIN, AND WHAT RE-PINS IT. Whoever moves this body deliberately replaces BOTH lines and +# writes the decision beside them; a pin with no commit and no decision next to it is a number +# nobody can audit. The parent script carries the identical table and the two must not drift. +# 3e298c9a 37fc94ff... ID-15, P3a: the two-arm shape, reviewed and accepted +# 3dadd4c1 172b0222... ID-41, P5 (b1): [Fix] (DirectGLES): make the extent hostBase is good +# for a parameter of the flush ladder, so tier 1's widening refusal is +# live code the moment a SEG_STAGE snapshot is narrower than the queued +# range. <- CURRENT PINNED_FUNCTIONS="FlushPendingRangesFrom" -PINNED_BASELINE_REF=3e298c9a -PINNED_SHA_FlushPendingRangesFrom=37fc94ffc5991923d222d585daa3af6511d2352d255623026ce35a3b6963c4a6 +PINNED_BASELINE_REF=3dadd4c1 +PINNED_BASELINE_DECISION=ID-41 +PINNED_SHA_FlushPendingRangesFrom=172b022273db01b16e772d15b269ffcd797fe38c767f7354d83ce113a66040d0 # The regions the self-test perturbs, one negative control each. FOUR, exactly as D-N requires, and # each is a different shape so that a control which only ever perturbed the easy one cannot leave @@ -372,7 +390,7 @@ def extract(rows): def perturb(rows, target, src, dst, where='head'): - """Insert one line into a region's body, at its HEAD or at its TAIL. + """Insert one line into a region's body at its HEAD or its TAIL, or one TOKEN at its head. TWO POSITIONS, AND THE SECOND ONE IS REVIEW FINDING F-m2. Every control used to insert at the very first byte after the opening brace, so all four of them would still have tripped if @@ -393,7 +411,15 @@ def perturb(rows, target, src, dst, where='head'): % (target, len(hits))) return 2 begin, end = hits[0] - if where == 'tail': + if where == 'token': + # ONE TOKEN - a single empty statement at the head of the body - and nothing else. + # ID-41(d) asks the pinned row's control to perturb the LADDER rather than a comment + # beside it, and this is the smallest edit that is unambiguously code: a reader cannot + # answer "the gate only notices comments". The perturbed copy is never compiled, only + # hashed, so an empty statement is legal here in a way it would not be in the tree. + brace = masked.index('{', begin) + patched = text[:brace + 1] + ';' + text[brace + 1:] + elif where == 'tail': # end is one PAST the closing brace (find_function / find_namespace both return # `match_forward(...) + 1`), so end - 1 is the brace itself and this lands inside the # body, one character before it ends. @@ -495,31 +521,59 @@ extract_baseline() { sed 's/^/[p4a-untouched] /' "$WORK_DIR/$out.err" >&2 return 2 fi + for name in $PINNED_FUNCTIONS; do + eval "pinned=\$PINNED_SHA_$name" + grep "^$name$(printf '\t')" "$WORK_DIR/$out.spec.all" > "$WORK_DIR/$out.spec.pinned" || true + atRef=$(python3 "$PY" extract "$WORK_DIR/$out.spec.pinned" 2>/dev/null | awk '{ print $1 }') + if [ -n "$atRef" ] && [ "$atRef" != "$pinned" ]; then + say "NOTE: $name IS defined at '$ref' and hashes" + say " $atRef, which is not the pin" + say " ($pinned," + say " captured at $PINNED_BASELINE_REF, $PINNED_BASELINE_DECISION). THE PIN IS WHAT IS" + say " COMPARED - it is the reviewed body - and this note is not a verdict in either" + say " direction. If '$ref' PREDATES $PINNED_BASELINE_REF the two SHOULD disagree: the pinned" + say " body is the change $PINNED_BASELINE_DECISION admitted, which is why it was re-pinned" + say " rather than reverted. If it does not predate it, the ladder that ships has moved away" + say " from the reviewed text without this gate being re-pinned - re-pin deliberately or" + say " revert, but do not leave them disagreeing." + fi + done + # The pinned rows are appended and the list is put back into the FIXED ORDER, both inside + # apply_pinned_shas - ONE spelling of the substitution, which --self-test's pin controls drive + # as well, so a control cannot prove only that a copy of the logic agrees with itself. + apply_pinned_shas "$WORK_DIR/$out.sha" || return 2 + return 0 +} + +# True when $1 is one of the rows whose baseline is the PIN rather than . +is_pinned_row() { + local name candidate + for candidate in $PINNED_FUNCTIONS; do + [ "$candidate" = "$1" ] && return 0 + done + return 1 +} + +# Overwrite the pinned rows of a ` ` list with the shas PINNED at the top of this +# script, then restore the FIXED ORDER (review F-m1: the pinned rows would otherwise be emitted +# LAST while extract_ref emits everything in REGIONS order. The gate itself never noticed - +# compare_lists looks rows up by name - but the documented capture workflow did: the header +# promises "stdout is always the sha list ... in the fixed order above - so a baseline capture is a +# plain redirect", and a baseline captured that way then diffed against a two-ref stdout showed +# seven spurious differences purely from row order). +apply_pinned_shas() { + local file=$1 name pinned for name in $PINNED_FUNCTIONS; do eval "pinned=\$PINNED_SHA_$name" if [ -z "$pinned" ] || [ "$pinned" = "PLACEHOLDER_SHA" ]; then say "$name has no pinned baseline sha; that row cannot be compared" return 2 fi - grep "^$name$(printf '\t')" "$WORK_DIR/$out.spec.all" > "$WORK_DIR/$out.spec.pinned" || true - atRef=$(python3 "$PY" extract "$WORK_DIR/$out.spec.pinned" 2>/dev/null | awk '{ print $1 }') - if [ -n "$atRef" ] && [ "$atRef" != "$pinned" ]; then - say "NOTE: $name IS defined at '$ref' and hashes $atRef, which is NOT the sha pinned in this" - say " script ($pinned, captured at $PINNED_BASELINE_REF). The PIN is what is compared - it is" - say " the reviewed text (ID-15) - but the two disagreeing means the push ladder moved between" - say " $PINNED_BASELINE_REF and '$ref' without this gate being re-pinned. Re-pin deliberately or" - say " revert; do not leave them disagreeing." - fi - printf '%s %s\n' "$pinned" "$name" >> "$WORK_DIR/$out.sha" + grep -v " $name\$" "$file" > "$file.unpinned" || true + mv -f "$file.unpinned" "$file" || return 2 + printf '%s %s\n' "$pinned" "$name" >> "$file" done - # ...and put the list back into the FIXED ORDER (review F-m1). The pinned rows were stripped out - # of the spec above and appended here, so without this the baseline side emits them LAST while - # extract_ref emits everything in REGIONS order. The gate itself never noticed - compare_lists - # looks rows up by name - but the documented capture workflow did: the header promises "stdout is - # always the sha list ... in the fixed order above - so a baseline capture is a plain redirect", - # and a baseline captured that way then diffed against a two-ref stdout showed seven spurious - # differences purely from row order. - reorder_sha_list "$WORK_DIR/$out.sha" || return 2 + reorder_sha_list "$file" || return 2 return 0 } @@ -553,6 +607,18 @@ compare_lists() { say "FIRST REGION THAT MOVED: $name" say " $labelA ${shaA:-}" say " $labelB ${shaB:-}" + if is_pinned_row "$name"; then + # ITS OWN MESSAGE, and that is R-16 rather than decoration: the pinned row and the + # sixteen ref-a rows fail differently and are fixed differently, so a reader who sees + # only the generic paragraph below goes looking for a diff against that does not + # exist. + say " $name IS A PINNED ROW ($PINNED_BASELINE_DECISION): its baseline is ALWAYS the sha" + say " PINNED in this script - the body reviewed at $PINNED_BASELINE_REF - and never the" + say " body at '$labelA'. So this is not a diff against the base ref: the ladder that" + say " SHIPS has moved away from the text that was reviewed. Either re-pin deliberately," + say " replacing the sha AND the commit AND the decision beside it in BOTH this script" + say " and its parent scripts/p3a_untouched_regions.sh, or revert the body." + fi say " G5 (ARCHITECTURE.md:318, :321, :515) says the Espryt do-not-touch list is literal:" say " P3a's buffer pool, deferred-release drain, three rings and BOTH arms of the three-tier" say " flush drain, plus P4a's unpack-PBO staging repack and its two ring helpers, the" @@ -573,10 +639,12 @@ compare_lists() { # --- self-test ------------------------------------------------------------------------------ # A gate that always says "identical" and a gate that is working produce the same green, so the # comparison has to be shown failing. Both controls run: the POSITIVE ones (an untouched copy -# compares equal; an edit OUTSIDE the regions is invisible) rule out a comparison that reports -# every region as moved, and the eight NEGATIVE ones - D-N's four regions, each perturbed at the -# HEAD of its body and again at its TAIL - rule out both the comparison that never reports any and -# the extraction whose extent stops before the closing brace (F-m2). +# compares equal; an edit OUTSIDE the regions is invisible; a baseline that names another sha for +# the PINNED row is overridden by the pin) rule out a comparison that reports every region as +# moved, and the NINE NEGATIVE ones - D-N's four regions, each perturbed at the HEAD of its body +# and again at its TAIL, plus a ONE-TOKEN edit to the pinned ladder compared AGAINST THE PIN +# (ID-41) - rule out the comparison that never reports any, the extraction whose extent stops +# before the closing brace (F-m2), and a pin that nothing consults. if [ "${1:-}" = "--self-test" ]; then [ $# -eq 1 ] || { say "--self-test takes no other arguments"; exit 2; } mkdir -p "$WORK_DIR/pristine" || exit 2 @@ -678,6 +746,85 @@ if [ "${1:-}" = "--self-test" ]; then say "its tail), ran $controls" exit 2 fi + + # --- THE PINNED ROW (ID-41) ----------------------------------------------------------------- + # TWO more controls, and they exist because none of the ten above can see the pin at all: every + # one of them compares one extraction of the working tree against another, so they would all be + # green on a build of this script in which PINNED_SHA_* was never read by anything. The pinned + # row's whole claim is "the baseline is the PIN, not " (D-N/2, ID-41), and that claim + # needs its own two. + for target in $PINNED_FUNCTIONS; do + eval "pinned=\$PINNED_SHA_$target" + targetSource=$(printf '%s\n' "$REGIONS" | awk -F@ -v n="$target" '$1 == n { print $3 }') + [ -n "$targetSource" ] || { say "$target is not one of the regions"; exit 2; } + + # (1) PIN PRECEDENCE, positive. A baseline that carries some OTHER sha for the pinned row - + # which is the shape of every CI passes, since 37da3c3a DOES define + # FlushPendingRangesFrom and no longer hashes the pin - must come out of apply_pinned_shas + # carrying the PIN. + grep -v " $target\$" "$WORK_DIR/pristine.sha" > "$WORK_DIR/pinprec.sha" || true + printf '%s %s\n' \ + "0000000000000000000000000000000000000000000000000000000000000000" "$target" \ + >> "$WORK_DIR/pinprec.sha" + apply_pinned_shas "$WORK_DIR/pinprec.sha" || exit 2 + got=$(awk -v n="$target" '$2 == n { print $1 }' "$WORK_DIR/pinprec.sha") + if [ "$got" != "$pinned" ]; then + say "PIN CONTROL FAILED: a baseline that carried a DIFFERENT sha for $target came out as" + say " '${got:-}' and not as the pin ($pinned). That row would be compared against" + say " again, which is exactly what D-N/2 and $PINNED_BASELINE_DECISION forbid." + exit 2 + fi + say "pin control: a baseline that defines $target differently is overridden by the PIN" + + # (2) A ONE-TOKEN EDIT TO THE PINNED LADDER, negative, AGAINST THE PIN. The comparison must go + # red, must name the region, and must say the region is PINNED - R-16's "a control asserts its + # OWN failure string": the pinned row and the sixteen ref-a rows are fixed differently, and a + # reader who gets only the generic paragraph goes looking for a diff against that does + # not exist. + rm -rf "$WORK_DIR/pinperturbed" + cp -r "$WORK_DIR/pristine" "$WORK_DIR/pinperturbed" || exit 2 + write_spec "$WORK_DIR/pinperturbed" "$WORK_DIR/pinperturbed.spec" + python3 "$PY" perturb "$WORK_DIR/pinperturbed.spec" "$target" \ + "$WORK_DIR/pristine/$(blob_name "$targetSource")" \ + "$WORK_DIR/pinperturbed/$(blob_name "$targetSource")" token || exit 2 + python3 "$PY" extract "$WORK_DIR/pinperturbed.spec" > "$WORK_DIR/pinperturbed.sha" || exit 2 + cp -f "$WORK_DIR/pristine.sha" "$WORK_DIR/pinbase.sha" || exit 2 + apply_pinned_shas "$WORK_DIR/pinbase.sha" || exit 2 + if compare_lists "$WORK_DIR/pinbase.sha" "$WORK_DIR/pinperturbed.sha" \ + "PIN($PINNED_BASELINE_REF)" "one-token-perturbed" 2> "$WORK_DIR/pinperturbed.err"; then + say "NEGATIVE CONTROL DID NOT TRIP: one token was inserted into $target's body and the" + say "comparison AGAINST THE PIN still reported every region as identical. The pinned row is" + say "not being compared at all, so every green this gate has printed for it means nothing." + exit 2 + fi + if ! grep -q "FIRST REGION THAT MOVED: $target" "$WORK_DIR/pinperturbed.err"; then + say "NEGATIVE CONTROL TRIPPED FOR THE WRONG REASON: the comparison against the pin went red" + say "but did not name $target as the first region that moved. It said:" + sed 's/^/[p4a-untouched] /' "$WORK_DIR/pinperturbed.err" >&2 + exit 2 + fi + if ! grep -q "$target IS A PINNED ROW" "$WORK_DIR/pinperturbed.err"; then + say "NEGATIVE CONTROL TRIPPED FOR THE WRONG REASON: it went red and named $target, but did" + say "not say that this row's baseline is the PIN. That is the half a reader acts on, and a" + say "control that does not assert its own message is not a control (R-16). It said:" + sed 's/^/[p4a-untouched] /' "$WORK_DIR/pinperturbed.err" >&2 + exit 2 + fi + controls=$((controls + 1)) + say "negative control $controls: a ONE-TOKEN edit to the pinned $target body goes red AGAINST" + say " THE PIN, is named, and says the row is pinned" + + # NOT a failure, deliberately: --self-test is about whether the comparison works, and it runs + # on the WORKING tree, which may legitimately carry an uncommitted edit. The two-ref gate is + # what fails when the committed region has left the pin. + treeSha=$(awk -v n="$target" '$2 == n { print $1 }' "$WORK_DIR/pristine.sha") + if [ "$treeSha" != "$pinned" ]; then + say "NOTE: this working tree's $target hashes $treeSha, not the pin ($pinned). The" + say " self-test's verdict is unaffected; the two-ref gate will be RED until you re-pin or" + say " revert." + fi + done + say "self-test passed: $controls negative controls, all tripped and all named" exit 0 fi From 6cabc6b070fba12b77b7035d99daccf8e1a996c1 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 06:03:52 -0400 Subject: [PATCH 03/10] [Fix, Test] (MG_IntegrationTest, MG_Test): make the persistent-map membership expectation a function of the arm the map landed in, and pin the arm-independent half on IsLivePersistentMap where both answers can be produced --- .../PersistentCoherentMapScenario.cpp | 76 +++++++++++- MobileGL/MG_Test/Buffer/SplitBufferTest.cpp | 109 +++++++++++++++++- 2 files changed, 178 insertions(+), 7 deletions(-) diff --git a/MobileGL/MG_IntegrationTest/Scenarios/PersistentCoherentMapScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/PersistentCoherentMapScenario.cpp index c6e08c73..7ee9be12 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/PersistentCoherentMapScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/PersistentCoherentMapScenario.cpp @@ -43,6 +43,16 @@ // address really is valid in this process, which is precisely why E3(d) is worth gating at all - // and a direct count of declines would need a counter from packages b1/v1. // +// AND MEMBERSHIP FOLLOWS THE ARM (ID-42). The client's live-persistent-map set is the set of maps +// the client still has to PUSH, so an ADOPTED store is deliberately not in it - +// PersistentMapTracker::IsLivePersistentMap's second row is IsBackendPersistentMapped(), and an +// adopted store's bytes are already in coherent GPU memory. AssertMembership therefore expects +// `live == (arm == emulated)`, from the same peek AssertOrRecordArm reads. The arm-INDEPENDENT +// claim - "the predicate reads the chain, not the adopt tier" - belongs to a unit case that can +// drive both answers on demand (SplitBufferSet. +// TheAdoptedArmIsNotAMemberAndItIsTheChainRowThatSaysSo) and not to an integration entry, which +// only ever sees whichever arm its driver and transport happen to give it. +// // `mpr` (map-persistent-roundtrips) is counted per ACQUISITION ATTEMPT, mint or decline // (ARCHITECTURE.md:492), so it is the SAME NUMBER on both arms and in both transports: that is // what makes exit gate E3(c)'s "mpr equal to the monolith arm's" checkable by one process. Both @@ -243,6 +253,11 @@ void main() { oColor = vec4(vColor, 1.0); } ASSERT_TRUE(PeekBufferIsAdoptedPersistentMap(vbo, &adopted)) << "no frontend BufferObject behind GL buffer " << vbo << " " << when; const char* arm = adopted ? "adopted" : "emulated"; + // REMEMBERED, because AssertMembership's expectation is a function of it (ID-42) + // and the two have to be talking about ONE observation of ONE buffer. A second + // peek would be a second question, and a scenario that asked it twice could + // straddle a change and then compare two different moments. + m_observedArm = adopted ? ObservedArm::Adopted : ObservedArm::Emulated; RecordProperty("persistent_map_arm", arm); if (declared.empty()) return; ASSERT_EQ(declared, std::string(arm)) @@ -259,21 +274,63 @@ void main() { oColor = vec4(vColor, 1.0); } // b1-v1.md 4.1 item 2: the client's live-persistent-map set is meant to be exactly // SyncPersistentMappedRange's early-out chain. A drift between the two stops the push // silently; asking here makes it a named failure instead. + // + // MEMBERSHIP IS A PROPERTY OF THE ARM, NOT OF THE FLAGS - INTEGRATOR DECISION ID-42, + // and the first cut of this function got it wrong in a way no lane could see. It + // asserted `live` UNCONDITIONALLY, on the grounds that "a PERSISTENT|WRITE|COHERENT map + // that is not FLUSH_EXPLICIT is a member by construction". That sentence is true only + // on the EMULATED arm. PersistentMapTracker::IsLivePersistentMap's second row is + // `if (buffer.IsBackendPersistentMapped()) return false;` - deliberately, because the + // predicate must read the CHAIN and not the tier: an adopted store's bytes are already + // in coherent GPU memory and there is nothing for the push to ship, so it is not a + // member and must not be one. On the adopted arm `live` is false BY DESIGN. + // + // It survived the first wave because the two builds that ran it could not contradict + // it: the push build compiles no MG_Remote, so MGITEST_PERSISTENT_MAP_TRACKER is + // undefined and this function returned at the line above before asserting anything; + // the split build's monolith lane, where the tracker IS compiled, adopts + // (MGPipeApplyMapPersistent's R-6 decline is ANDed with `Transport != Monolith`, + // PipeApply.cpp:2005), and seven entries went red the first time the assertion ran at + // all. + // + // So the expectation is `live == (arm == emulated)`, taken from the SAME peek + // AssertOrRecordArm read. The arm-independent half - "the predicate reads the chain, + // not the tier" - is not an integration claim and is pinned where it can be driven + // directly, by MG_Test/Buffer/SplitBufferTest.cpp's + // SplitBufferSet.TheAdoptedArmIsNotAMemberAndItIsTheChainRowThatSaysSo. void AssertMembership(unsigned int vbo, const char* when) { if (!PersistentMapTrackerAvailable()) { RecordProperty("persistent_map_membership", "unavailable"); return; } + if (m_observedArm == ObservedArm::NotLookedAt) { + // AssertOrRecordArm could not look, so there is no arm to condition on and + // "could not look" is not "it was emulated" (PersistentMapPeek.h). A lane that + // DECLARES an arm has already been skipped by AssertOrRecordArm in this case. + RecordProperty("persistent_map_membership", "arm unknown"); + return; + } + const bool emulated = (m_observedArm == ObservedArm::Emulated); bool live = false; ASSERT_TRUE(PeekBufferIsLivePersistentMap(vbo, &live)) << "the tracker is compiled in but could not answer for GL buffer " << vbo << " " << when; - EXPECT_TRUE(live) - << "a PERSISTENT|WRITE|COHERENT map that is not FLUSH_EXPLICIT is a member of " - "the client's live-persistent-map set by construction, and it is not one " - << when + RecordProperty("persistent_map_membership", live ? "member" : "not a member"); + EXPECT_EQ(live, emulated) + << "the client's live-persistent-map set is the set of maps the client still has " + "to PUSH, so membership follows the arm: on the emulated arm this " + "PERSISTENT|WRITE|COHERENT non-FLUSH_EXPLICIT map must be a member, and on " + "the adopted arm it must not be - IsLivePersistentMap's " + "IsBackendPersistentMapped() row takes it out, because an adopted store's " + "bytes are already in coherent GPU memory and there is nothing to ship. This " + "map landed in the " + << (emulated ? "emulated" : "adopted") << " arm " << when << " and the predicate " + << (live ? "made it a member" : "did not make it a member") << ". The set is supposed to BE SyncPersistentMappedRange's early-out chain; if " - "they have drifted, the push stops shipping this buffer's blocks and nothing " - "else says so."; + "they have drifted, the push either stops shipping this buffer's blocks or " + "starts shipping an adopted store's, and nothing else says so. (ID-42. The " + "arm-independent statement - that the predicate reads the chain and not the " + "adopt tier - is pinned by SplitBufferSet." + "TheAdoptedArmIsNotAMemberAndItIsTheChainRowThatSaysSo, not here.)"; } void TearDown() override { @@ -320,6 +377,13 @@ void main() { oColor = vec4(vColor, 1.0); } return RegionIsMostly(image, 8, image.Width() - 9, 8, image.Height() - 9, color, 0.0, when); } + // The arm this scenario OBSERVED, as opposed to the one its lane declared. A lane may + // declare none (the monolith counting lane does not), and the peek may be unable to + // look at all, so the third state is not "assume emulated" - it is "there is no arm to + // condition on", and AssertMembership records rather than asserts under it. + enum class ObservedArm { NotLookedAt, Adopted, Emulated }; + ObservedArm m_observedArm = ObservedArm::NotLookedAt; + unsigned int m_program = 0; unsigned int m_vao = 0; unsigned int m_vbo = 0; diff --git a/MobileGL/MG_Test/Buffer/SplitBufferTest.cpp b/MobileGL/MG_Test/Buffer/SplitBufferTest.cpp index a1370ddb..d60f7aa1 100644 --- a/MobileGL/MG_Test/Buffer/SplitBufferTest.cpp +++ b/MobileGL/MG_Test/Buffer/SplitBufferTest.cpp @@ -41,6 +41,21 @@ namespace { using MG_Remote::Client::GpuWriteProducer; using MG_Remote::Client::PersistentMapTracker; + // A backend that MINTS a persistent mapping, for the one case that needs the adopted arm + // (TheAdoptedArmIsNotAMemberAndItIsTheChainRowThatSaysSo). Only AcquirePersistentMap is + // filled in: the frontend null-checks every op individually, and a table with one live + // member is the smallest thing that makes AcquireMemoryRange's legacy adoption arm fire. + // The storage is the CASE's, not this table's - AdoptPersistentMap keeps the pointer and + // never owns it - so the base travels through a file-scope variable the case sets and + // clears around the one acquisition it wants minted. + void* g_mintedPersistentBase = nullptr; + void* MintPersistentMap(BufferObject&) { return g_mintedPersistentBase; } + const MG_State::GLState::BufferBackendOps g_mintingBufferOps = [] { + MG_State::GLState::BufferBackendOps ops{}; + ops.AcquirePersistentMap = &MintPersistentMap; + return ops; + }(); + // Everything in this package is gated on `Transport != Monolith`, so every case has to // put the process into a split configuration and put it back. A fixture rather than a // lambda because the tracker is a process-wide singleton and a case that left an entry in @@ -64,6 +79,11 @@ namespace { MG_Remote::Client::ResetProducerMarkCountsForTest(); } void TearDown() override { + // Belt and braces for the one case that installs a minting backend: a table left + // behind would make the NEXT case's acquisition land in the adopted arm, and every + // membership assertion in this file would then be about a different code path. + MG_State::GLState::SetBufferBackendOps(nullptr); + g_mintedPersistentBase = nullptr; PersistentMapTracker::Instance().ClearForTest(); MG_State::pGLContext = Move(m_context); MG_Config::Transport = m_transport; @@ -258,6 +278,92 @@ TEST_F(SplitBufferSet, MembershipIsSyncPersistentMappedRangesOwnEarlyOutChain) { buffer->ReleaseMemory(false); } +// THE ADOPTED ARM IS NOT A MEMBER, AND IT IS THE CHAIN THAT SAYS SO - NOT THE TIER (ID-42). +// +// IsLivePersistentMap's second row is `if (buffer.IsBackendPersistentMapped()) return false;`, +// and its comment promises that the row answers for ITSELF: at tier T2 the arm is unreachable +// because MapPersistent declines, but a build that reaches T0/T1 later must get the same answer +// out of the same row. Nothing drove that promise. The case above only ever sees the declined +// arm, and PersistentCoherentMapScenario's membership assertion cannot pin it either - an +// integration entry sees whichever arm its driver and transport hand it, which is exactly how +// that assertion came to state the emulated arm's property as if it were universal and go red on +// seven split-monolith entries the first time it ran. +// +// So the statement is pinned HERE, where both answers can be produced on demand. The adoption is +// made by the PRODUCTION path - AcquireMemoryRange dispatching to the backend's +// AcquirePersistentMap and calling PipeResource::AdoptPersistentMap - and not by the case +// reaching into the buffer, because a hand-set flag would still be "true" with the production +// adoption deleted (R-16). The transport is Monolith for exactly that call, because R-6's decline +// is ANDed with `Transport != Monolith` and there is no other way to reach a mint in this build; +// the PREDICATE is then asked with the tier back at T2/InProcess, which is the whole point: the +// tier says "emulated, always" and the chain still says "not a member". +TEST_F(SplitBufferSet, TheAdoptedArmIsNotAMemberAndItIsTheChainRowThatSaysSo) { + constexpr SizeT kSize = 4096; + // The storage the backend "mints". Declared first so it outlives the buffer: AdoptPersistentMap + // stores the pointer and releases the shadow, and it never owns what it was handed. + Vector minted(kSize, static_cast(0)); + g_mintedPersistentBase = minted.data(); + + auto adopted = MakeBuffer(27u, kSize); + auto declined = MakeBuffer(28u, kSize); + + // The declined twin first, with no backend ops at all: same flags, same size, same call. + declined->AcquireMemoryRange(Range1D{0, kSize}, + BufferMappingAccessBit::Write | BufferMappingAccessBit::Persistent); + ASSERT_FALSE(declined->IsBackendPersistentMapped()); + ASSERT_TRUE(PersistentMapTracker::IsLivePersistentMap(*declined)) + << "the emulated arm IS a member - if this fails the two halves are not comparable and the " + "assertion below proves nothing"; + + { + MG_State::GLState::SetBufferBackendOps(&g_mintingBufferOps); + MG_Config::Transport = MG_Config::TransportMode::Monolith; + adopted->AcquireMemoryRange(Range1D{0, kSize}, + BufferMappingAccessBit::Write | BufferMappingAccessBit::Persistent); + MG_Config::Transport = MG_Config::TransportMode::InProcess; + MG_State::GLState::SetBufferBackendOps(nullptr); + } + ASSERT_TRUE(adopted->IsBackendPersistentMapped()) + << "the backend declined the mint, so there is no adopted arm here to ask about"; + + // THE TIER SAYS EMULATED. The chain must still say "not a member". + ASSERT_TRUE(MG_Remote::Client::AdoptTierIsEmulate()); + ASSERT_NE(MG_Config::Transport, MG_Config::TransportMode::Monolith); + EXPECT_FALSE(PersistentMapTracker::IsLivePersistentMap(*adopted)) + << "an adopted store's bytes are already in host-visible coherent GPU memory and there is " + "nothing for the push to ship, so IsBackendPersistentMapped() takes it out of the set. " + "This answer must come from that ROW and not from the adopt tier: the tier is T2 and the " + "transport is InProcess right now, which is the configuration in which R-6 says every " + "acquisition declines - and the store in front of the predicate is adopted anyway, " + "because a later phase's T0/T1 will mint one. A predicate that read the tier would call " + "it a member and the push would read an adopted store's Bytes() as if it were the " + "shadow."; + + // The SET agrees with the predicate, asked through the production entry point rather than by + // reading a member: NoteMapStateChanged is what every one of the five maintenance events + // calls, and it must refuse to enrol an adopted store. + PersistentMapTracker::Instance().NoteMapStateChanged(*adopted); + EXPECT_EQ(PersistentMapTracker::Instance().MemberCount(), 1u) + << "only the declined twin: enrolling an adopted store would make the push read its " + "Bytes() - which is the GPU map, not the shadow - as if it were bytes to ship"; + + // ...and the consequence, which is the one that would actually corrupt something. + const Uint64 before = MG_Util::PipeStats::TotalBytes(MG_Util::PipeStats::ByteClass::PersistentMapPush); + MG_Remote::Client::PushPersistentMapsBeforeVerb(); + EXPECT_EQ(MG_Util::PipeStats::TotalBytes(MG_Util::PipeStats::ByteClass::PersistentMapPush) - before, + static_cast(kSize)) + << "the declined twin's 4096 bytes and nothing else: the adopted buffer must contribute no " + "pushed bytes at all"; + + declined->ReleaseMemory(false); + // The adopted one is NOT released through ReleaseMemory: ReleasePersistentMap is for a store + // being redefined, and a persistent map the application holds outlives every unmap by + // definition (PipeResource.h:121-127). It is dropped here with the mapping still adopted, + // which is also the shape ~BufferObject has to survive. + adopted.reset(); + g_mintedPersistentBase = nullptr; +} + // pmap is non-zero, and it is non-zero in BLOCKS. TEST_F(SplitBufferSet, ThePushCutsTheMappedSpanIntoBlocksAndMovesPmap) { constexpr SizeT kSize = 4u * 64u * 1024u; // exactly four 64 KiB blocks @@ -402,7 +508,7 @@ TEST_F(SplitBufferSet, OnlyAdoptTierTwoIsImplemented) { #else -// THE SAME FIFTEEN NAMES, SO THE ctest NAME SET DOES NOT MOVE BETWEEN LANES. G2 compares the +// THE SAME SIXTEEN NAMES, SO THE ctest NAME SET DOES NOT MOVE BETWEEN LANES. G2 compares the // pull and push name lists line for line and G14 allows build-split to ADD names but never to // remove one, so a case that exists only where it can run would break both gates for a reason // that has nothing to do with what it tests. It skips instead, and says why. @@ -417,6 +523,7 @@ TEST(SplitBufferSet, Row4AReadPixelsIntoAPackPboMarksThePbo) { MGL_SPLIT_ONLY_OR TEST(SplitBufferSet, Row5EndTransformFeedbackMarksTheCaptureTargets) { MGL_SPLIT_ONLY_OR_SKIP(); } TEST(SplitBufferSet, TheWholeSetIsInertOnTheMonolithPath) { MGL_SPLIT_ONLY_OR_SKIP(); } TEST(SplitBufferSet, MembershipIsSyncPersistentMappedRangesOwnEarlyOutChain) { MGL_SPLIT_ONLY_OR_SKIP(); } +TEST(SplitBufferSet, TheAdoptedArmIsNotAMemberAndItIsTheChainRowThatSaysSo) { MGL_SPLIT_ONLY_OR_SKIP(); } TEST(SplitBufferSet, ThePushCutsTheMappedSpanIntoBlocksAndMovesPmap) { MGL_SPLIT_ONLY_OR_SKIP(); } TEST(SplitBufferSet, TheLastBlockIsTheRemainderAndNotAWholeBlock) { MGL_SPLIT_ONLY_OR_SKIP(); } TEST(SplitBufferSet, BothEdgesOfAWriteMapPublishOneStateRecord) { MGL_SPLIT_ONLY_OR_SKIP(); } From 7110cdb61aba1e4cbe3d1c326c60c4e299f00012 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 06:33:07 -0400 Subject: [PATCH 04/10] [Fix] (scripts): make gen_pipe.py's expect_trip require the tripped guard's own message and a non-zero exit code, with an R-16 meta-control that an unrelated SystemExit or sys.exit(0) cannot pass as a trip - the any-SystemExit defect fb70704e fixed in the field-ownership generator, confirmed still present here by codex review finding 9 --- scripts/gen_pipe.py | 82 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 70 insertions(+), 12 deletions(-) diff --git a/scripts/gen_pipe.py b/scripts/gen_pipe.py index 6d08114e..00f593ff 100644 --- a/scripts/gen_pipe.py +++ b/scripts/gen_pipe.py @@ -1376,27 +1376,54 @@ def write(path, text, check, changed): handle.write(text) -def expect_trip(name, fn): - """Runs one negative control; a gate that lets it through is the failure.""" +def expect_trip(name, because, fn, quiet=False): + """Runs one negative control; a gate that lets it through, or trips for a reason that is + not ITS OWN, is the failure. + + The first version of this function caught any SystemExit - including an unrelated + diagnostic, and even a CLEAN sys.exit(0) - and asked nothing about which one, so the + codex cross-family review's finding 9 could replace a control's callback with either and + the "nine negative controls all trip" line stayed true while the guard it named never + fired. gen_pipe_field_ownership.py's M-1 was the same defect in the sibling generator; + this mirrors that fix. + + `because` is a substring the control's OWN message must contain, and an exit code of 0 + can never count as a trip.""" try: fn() except SystemExit as trip: - print("gen_pipe: self-test %s: tripped as expected (%s)" % (name, str(trip).splitlines()[0][:100])) - return 1 - print("gen_pipe: self-test %s: DID NOT TRIP" % name, file=sys.stderr) + code = trip.code + message = str(code) if code is not None else "" + if code != 0 and because in message: + if not quiet: + print("gen_pipe: self-test %s: tripped as expected (%s)" + % (name, message.splitlines()[0][:100])) + return 1 + if not quiet: + print("gen_pipe: self-test %s: tripped for SOMEONE ELSE'S reason:\n" + " expected to contain: %s\n" + " actually said (code=%r): %s" + % (name, because, code, message.splitlines()[0][:200] if message else ""), + file=sys.stderr) + return 0 + if not quiet: + print("gen_pipe: self-test %s: DID NOT TRIP" % name, file=sys.stderr) return 0 def self_test(accessors): """The negative controls (check_include_closure.py's shape): each gate must go red for - its reason, and zero trips is itself an error.""" + its OWN reason, and zero trips is itself an error.""" canned_struct = "struct Canned {\n Uint32 A;\n Uint32 B, C;\n Uint8 Pad0[3];\n void F() { return; }\n};\n" controls = [ ("struct member without F(...)", + "member(s) with no F(...) in PipeFields.def", lambda: check_field_lists_cover_struct_members({"Canned": ["A", "B"]}, ["Canned"], [canned_struct])), ("F(...) that is not a member", + "F(...) name(s) that are not members", lambda: check_field_lists_cover_struct_members({"Canned": ["A", "B", "C", "D"]}, ["Canned"], [canned_struct])), ("payload with no struct", + "struct not found", lambda: check_field_lists_cover_struct_members({"Nowhere": ["A"]}, ["Nowhere"], [canned_struct])), ] fill_text = read(os.path.join(PIPE_DIR, "FillPoints.def")) @@ -1404,16 +1431,24 @@ def self_test(accessors): field_row = re.compile(r"X\(\s*kDraw\s*,\s*GetBoundVertexArray\s*\)") if not verb_row.search(fill_text) or not field_row.search(fill_text): sys.exit("gen_pipe: self-test: FillPoints.def lost the rows the controls edit") - controls.append(("verb missing from FillPoints.def", lambda: parse_fill_points( + controls.append(("verb missing from FillPoints.def", + "GLFunctionsTable member(s) without a verb row", + lambda: parse_fill_points( accessors, text=verb_row.sub("", fill_text, count=1)))) - controls.append(("verb that is not a GLFunctionsTable member", lambda: parse_fill_points( + controls.append(("verb that is not a GLFunctionsTable member", + "verb(s) that are not GLFunctionsTable members", + lambda: parse_fill_points( accessors, text=verb_row.sub("X(DrawArrays, kDraw) X(NotAVerb, kDraw)", fill_text, count=1)))) - controls.append(("field row naming a non-accessor", lambda: parse_fill_points( + controls.append(("field row naming a non-accessor", + "is not an accessor in Coverage.def", + lambda: parse_fill_points( accessors, text=field_row.sub("X(kDraw, NotAnAccessor)", fill_text, count=1)))) # The EMITTED list's own gate: a row naming a call that is not in PipeCalls.def would # generate an enumerator nothing can dispatch on. calls_for_control = parse_calls() - controls.append(("emitted row naming a call that does not exist", lambda: gen_emitted_by( + controls.append(("emitted row naming a call that does not exist", + "which is not a call in PipeCalls.def", + 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 @@ -1421,13 +1456,36 @@ def self_test(accessors): # 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"]) + flag_typo_because = "which is not an MGPipeCallFlags enumerator" controls.append(("call flag that is not an MGPipeCallFlags enumerator", + flag_typo_because, lambda: check_call_flags_are_known([flag_typo]))) controls.append(("kNone combined with a real flag", + "combines kNone with", lambda: check_call_flags_are_known([flag_kNone]))) + + # R-16 meta-control (codex review finding 9; verified by execution in + # p5-results/wave1-codex-verify.md §9). The exact perturbation there replaced the + # flag-typo control's callback with `sys.exit("unrelated parser failure")` and, + # separately, with `sys.exit(0)`; the OLD expect_trip counted both as "tripped as + # expected" because it never looked at the message or the code. Both must be REJECTED + # here, under the real control's own `because`. This is a quiet, unlisted assertion (not + # one of the nine controls below) so it cannot itself inflate the trip count. + # "Red once by doing X" = reverting expect_trip to `except SystemExit as trip: return 1` + # (accept any SystemExit) - that alone turns each of these two checks from a silent pass + # into the sys.exit below. + if expect_trip("(R-16 meta-control) unrelated diagnostic standing in for the flag-typo guard", + flag_typo_because, lambda: sys.exit("unrelated parser failure"), quiet=True) != 0: + sys.exit("gen_pipe: self-test: expect_trip counted an UNRELATED SystemExit as the " + "flag-typo guard's own trip - finding 9 / R-16 is back") + if expect_trip("(R-16 meta-control) sys.exit(0) standing in for the flag-typo guard", + flag_typo_because, lambda: sys.exit(0), quiet=True) != 0: + sys.exit("gen_pipe: self-test: expect_trip counted sys.exit(0) as a trip - " + "finding 9 / R-16 is back") + trips = 0 - for name, fn in controls: - trips += expect_trip(name, fn) + for name, because, fn in controls: + trips += expect_trip(name, because, 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]) From 1a06b40a58ccbd9b3bca2aa875cb5710e2617a40 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 06:40:55 -0400 Subject: [PATCH 05/10] [Fix] (MG_Remote, Wire): let an empty SEG_STAGE take a blob that fits by rebasing its cursors to zero, so the wrap skip is only ever charged against bytes still in flight --- MobileGL/MG_Remote/Wire/PipeWireCodec.cpp | 32 ++++++++++++++++++ MobileGL/MG_Remote/Wire/PipeWireCodec.h | 11 ++++++ MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp | 37 +++++++++++++++++++++ 3 files changed, 80 insertions(+) diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp index 0b13148c..00b7264f 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp @@ -717,6 +717,12 @@ namespace MobileGL::MG_Remote::Wire { } for (int attempt = 0; attempt < 2; ++attempt) { + // THE WRAP SKIP MAY ONLY BE CHARGED AGAINST BYTES THAT ARE STILL IN FLIGHT. When + // there are none the allocator starts over at offset zero, so a blob the segment + // can hold whole is never refused (see RebaseEmptyStage). On attempt 1 this runs + // AFTER ReclaimStagedBytes, which is the case the finding describes: 8 MiB + // allocated, then retired, then a 28 MiB request that used to abort. + RebaseEmptyStage(); 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 @@ -936,6 +942,32 @@ namespace MobileGL::MG_Remote::Wire { return m_emitSeq; } + // THE EMPTY-STAGE REBASE. Head and tail are monotonic byte counts, so "every staged byte + // has retired" reads head == tail, NOT head == tail == 0, and `head % capacity` is left + // wherever the last run ended. Charging a wrap skip against that offset then costs the + // unused suffix a second time: with head == tail == 64 in a 256 KiB stage, the allocator's + // test became `2*capacity - 64 <= capacity`, which is false at EVERY occupancy, so a blob + // that fits the segment whole was refused with `Fatal{RingOverrun, "SEG_STAGE"}` - whose + // own message then reported `0 bytes still in flight`. ReclaimStagedBytes cannot help, + // because an already-empty tail has nothing left to move. + // + // THE MARK QUEUE COMES WITH IT. A mark holds the ABSOLUTE head cursor it was pushed at and + // ReclaimStagedBytes assigns that value straight to m_stageTail. Every mark not yet + // consumed has StageCursor <= head == tail - the head is monotonic and marks are pushed in + // order - so each of them names a region that is already reclaimed and zero is the + // truthful rebasing of it. Without that, one reclaim after a rebase would put the tail + // AHEAD of the head and StagedBytesInFlight() would underflow to about 2^64. + void PipeWireEncoder::RebaseEmptyStage() { + if (m_stageHead != m_stageTail || m_stageHead == 0) { + return; + } + m_stageHead = 0; + m_stageTail = 0; + for (SizeT i = 0; i < m_stageMarks.size(); ++i) { + m_stageMarks[i].StageCursor = 0; + } + } + void PipeWireEncoder::ReclaimStagedBytes() { if (m_control == nullptr) { return; diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.h b/MobileGL/MG_Remote/Wire/PipeWireCodec.h index 1f87f2df..ea2360bb 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.h +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.h @@ -290,6 +290,17 @@ namespace MobileGL::MG_Remote::Wire { // RingControl - see ReclaimStagedBytes above. Uint8* StageAllocate(Uint64 size); + // AN EMPTY STAGE STARTS OVER AT ZERO, so that the wrap skip is only ever charged + // against bytes that are really still in flight. Head and tail are monotonic, so once + // everything has retired they are EQUAL BUT NOT ZERO, and `head % capacity` is + // wherever the last run happened to end - a wrap skip charged against that offset + // costs the suffix a second time and refused a blob the whole segment could hold, with + // a message that reported zero bytes in flight while it did so. Rebasing also rewrites + // the marks still held: a mark stores an ABSOLUTE head cursor and a later reclaim + // assigns it to m_stageTail, so leaving a stale one behind would drive the tail past + // the head and underflow StagedBytesInFlight(). + void RebaseEmptyStage(); + Transport::RingControl* m_control = nullptr; Transport::RingProducer* m_cmd = nullptr; Transport::RingProducer* m_stage = nullptr; diff --git a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp index e29613c1..200bfdbd 100644 --- a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp +++ b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp @@ -1135,6 +1135,43 @@ TEST_F(PipeWireCodecTest, StagedBytesAreReclaimedOnlyBehindRetiredSeq) { ASSERT_TRUE(wire.PumpOne(&applied)); wire.Encoder().ReclaimStagedBytes(); EXPECT_EQ(wire.Encoder().StagedBytesInFlight(), 0u); + + // ---- AND AN EMPTY STAGE TAKES THE WHOLE SEGMENT --------------------------------- + // The verifier's extension of this case, kept (wave1-codex-verify.md §1). The 64-byte + // run has retired and in-flight bytes are ZERO, so every byte of SEG_STAGE is free - + // but head and tail are monotonic and both sit at 64, so `head % capacity` is 64 and the + // allocator used to charge a `capacity - 64` wrap skip against a capacity that had + // nothing in it. The test then read `2*capacity - 64 <= capacity`, false at every + // occupancy, and a blob the segment holds WHOLE aborted with + // `Fatal{RingOverrun, "SEG_STAGE"} ... with 0 bytes still in flight`. + // + // I made it red once, by doing X: X = deleting the `RebaseEmptyStage()` call at the top + // of StageAllocate's attempt loop (PipeWireCodec.cpp). The case then dies with SIGABRT + // inside PipeWireEncoder::StageAllocate on that message, exactly as the verifier + // recorded it. + // + // THE EXACT MAXIMUM. `need = Align8(size)` and the first bound is `need > capacity`, so + // an empty stage takes a blob of exactly the capacity the encoder adopted - here + // Wire2::kStageBytes, and in a real session the whole SEG_STAGE view, i.e. + // MOBILEGL_IPC_STAGE_MB (32 MiB by default; SessionRings.h keeps SEG_STAGE un-ringed and + // un-rounded, so there is no control page to subtract). + const Uint64 maxRecordBefore = wire.Encoder().MaxRecordBytesSeen(); + std::vector whole(Wire2::kStageBytes, 0x5A); + const MGPBlobRef full = wire.Encoder().StageBytes(whole.data(), whole.size()); + EXPECT_EQ(full.Offset, 0u) << "an empty stage must hand a whole-capacity blob offset zero"; + EXPECT_EQ(full.Size, Wire2::kStageBytes); + EXPECT_EQ(full.Seg, static_cast(kSegStage)); + EXPECT_EQ(wire.Encoder().StagedBytesInFlight(), Wire2::kStageBytes); + const void* back = wire.Segments().Resolve(full.Seg, full.Offset, full.Size); + ASSERT_NE(back, nullptr); + EXPECT_EQ(back, wire.StageBase()); + + // R-10's max-record counter DOES NOT SEE IT, and that is the point of R-10's carrier + // rule: EncodeRecord feeds m_maxRecordBytes from `layout.TotalBytes` - header + payload + + // tails, all of it SEG_CMD - while the blob leaves only {Seg, Offset, Size} in the + // record. A quarter-megabyte of staging moved the counter by zero bytes. SEG_STAGE has + // its own bound and its own named Fatal, and MaxRecordBytesSeen() is not it. + EXPECT_EQ(wire.Encoder().MaxRecordBytesSeen(), maxRecordBefore); } // ---- M2 / M3: SEG_STAGE's cursors and the mark queue -------------------------------------- From ba6dc4f7f11e0b8e29a6015d074976363c2241a5 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 06:41:00 -0400 Subject: [PATCH 06/10] [Fix] (MG_Remote, Wire): require every client-server content blob to declare SEG_STAGE and refuse any other carrier by name, which also puts the audit's run bookkeeping out of reach --- MobileGL/MG_Remote/Wire/PipeWireCodec.cpp | 46 ++++++++++++++- MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp | 63 +++++++++++++++++++++ 2 files changed, 108 insertions(+), 1 deletion(-) diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp index 00b7264f..d79af8ff 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp @@ -457,6 +457,36 @@ namespace MobileGL::MG_Remote::Wire { WireOpName(op), static_cast(blob.Size)); std::abort(); } + // R-2.3's SECOND HALF: "inside SOME segment" IS NOT THE RULE. Contract table 1 gives + // every client->server content blob - groups A, B and C, all nineteen rows - the ONE + // carrier SEG_STAGE, and R-10 sends blobs there whole. Until this arm existed the only + // test was that the run resolved, so `CreateSamplerState.Parameters={Seg=SEG_REPLY,...}` + // was accepted and APPLIED: a server-owned segment, whose reuse is the reply pool's + // business and has nothing to do with stage retirement, carrying bytes the applier + // then read. It also went unpoisoned - NoteResolvedRun skipped every non-stage carrier + // - so rule C's only mechanical control read zero on exactly the record that needed it. + // + // The segment is checked BEFORE the resolve, deliberately: a forged SEG_REPLY run that + // happens to lie inside a mapped reply pool must be refused for naming the wrong + // carrier, not left to pass or fail on whether that pool is mapped at all. + // + // NOT A NEW FATAL FAMILY. The review suggested `Fatal{BlobNotStaged}`; this is + // ProtocolCorruption like every other R-2 honesty arm, because the families are the + // vocabulary the operator and the CI greps share (ProtocolCorruption, AbiMismatch, + // UnmigratedVerb, UnmigratedPipeInput, UnsetCallMask, RingOverrun) and a one-off + // seventh name would be a token nothing else in the tree recognises. The SEGMENT is in + // the message, which is what has to be greppable. + if (blob.Seg != kSegStage) { + MGLOG_F("MGPipe: Fatal{ProtocolCorruption, \"%s.blob\"} seg=%u offset=%llu " + "size=%llu is not SEG_STAGE(%u); every client->server content blob is " + "staged whole in SEG_STAGE (contract table 1 groups A/B/C, R-10) and no " + "other segment may carry one", + WireOpName(op), static_cast(blob.Seg), + static_cast(blob.Offset), + static_cast(blob.Size), + static_cast(kSegStage)); + std::abort(); + } if (segments.Resolve(blob.Seg, blob.Offset, blob.Size) == nullptr) { MGLOG_F("MGPipe: Fatal{ProtocolCorruption, \"%s.blob\"} seg=%u offset=%llu size=%llu " "does not lie inside that segment (R-2.3)", @@ -1115,8 +1145,22 @@ namespace MobileGL::MG_Remote::Wire { } void PipeWireDecoder::NoteResolvedRun(MGPWireOp op, const MGPBlobRef& blob) { + // UNREACHABLE NOW, AND LOUD RATHER THAN SILENT. This used to `return`, and that made + // the audit's bookkeeping quietly optional: a record naming a non-SEG_STAGE carrier + // was applied AND recorded nothing, so PoisonedStageBytes() stayed zero and rule C's + // only mechanical control was dark on exactly the record it existed to catch. The one + // caller is ResolveOrFatal, which runs RequireDeclaredBlob first, and that now refuses + // both an undeclared blob and a non-SEG_STAGE one by name. If either ever arrives here + // the audit has stopped covering the carrier, which is the same failure as no audit at + // all - the reason the run-count overflow just below is a Fatal too. if (blob.Size == 0 || blob.Seg != kSegStage) { - return; + MGLOG_F("MGPipe: Fatal{ProtocolCorruption, \"%s\"} the audit was asked to record a " + "resolved run with seg=%u size=%llu; only declared SEG_STAGE(%u) runs " + "reach the poison fill (R-2.5)", + WireOpName(op), static_cast(blob.Seg), + static_cast(blob.Size), + static_cast(kSegStage)); + std::abort(); } 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 diff --git a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp index 200bfdbd..d6a7878a 100644 --- a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp +++ b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp @@ -1500,6 +1500,69 @@ TEST_F(PipeWireCodecTest, ARunThatLeavesItsSegmentIsFatal) { EXPECT_NE(r.Log.find("does not lie inside that segment"), std::string::npos) << r.Log; } +TEST_F(PipeWireCodecTest, AContentBlobCarriedOutsideSegStageIsFatalAtTheDecoder) { + // R-2.3's second half, and the verifier's finding-3 fixture kept as its own case + // (wave1-codex-verify.md §3). Contract table 1 row 17 puts CreateSamplerState's bytes in + // SEG_STAGE; here they sit in a mapped SEG_REPLY - the SERVER-owned reply pool, whose + // reuse has nothing to do with stage retirement - and the record names that segment. It + // used to be ACCEPTED and APPLIED, because the only test was that the run resolved + // somewhere: the verifier's probe printed `seg=3 accepted=1 poisoned=0`. + // + // The audit is armed, so the second half of the finding is nailed down too: with the + // poison ON, the record must DIE rather than be applied with PoisonedStageBytes() left at + // zero. NoteResolvedRun used to return silently for any non-stage carrier, which made + // rule C's only mechanical control dark on exactly the record it exists to catch; it is + // now a Fatal of its own and unreachable behind this arm. + // + // I made it red once, by doing X: X = deleting the `blob.Seg != kSegStage` arm in + // CheckBlobIsHonest (PipeWireCodec.cpp). The child then exits 0 instead of aborting and + // this case fails on DiedOfAbort - the verifier's `accepted=1` state. + const ChildResult r = RunInChild([] { + Wire2 wire; + std::vector replyBytes(4096, 0); + SamplerParameters params{}; + params.borderColorForm = BorderColorForm::Int; + std::memcpy(replyBytes.data(), ¶ms, sizeof(params)); + wire.Segments().Install(kSegReply, SegmentView{replyBytes.data(), replyBytes.size()}); + wire.Decoder().SetAuditPoison(true); + + MGPSamplerDesc desc{}; + desc.Cso = MakeHandle(88); + desc.Parameters.Seg = static_cast(kSegReply); + desc.Parameters.Offset = 0; + desc.Parameters.Size = sizeof(SamplerParameters); + ForgeAndDecode(wire, MGPWireOp::CreateSamplerState, &desc, sizeof(desc), nullptr, 0); + }); + ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log; + EXPECT_NE(r.Log.find("is not SEG_STAGE"), std::string::npos) << r.Log; + EXPECT_NE(r.Log.find("CreateSamplerState.blob"), std::string::npos) << r.Log; + EXPECT_NE(r.Log.find("seg=3"), std::string::npos) << r.Log; +} + +TEST_F(PipeWireCodecTest, TheEncoderRefusesTheNonStageCarrierTheDecoderCallsFatal) { + // THE ENCODER MUST NOT ACCEPT A RECORD THE DECODER FATALS ON - the same symmetry + // TheEncoderRefusesThePerStageSpirvRunTheDecoderCallsFatal states one arm over. Under + // `inproc` a SEG_REPLY pointer resolves, so an emitter that staged into the reply pool + // would get a valid seq here and a Fatal on a peer, which is the asymmetry EncodeRecord's + // own honesty loop exists to prevent. + // + // I made it red once, by doing X: X = deleting the `blob.Seg != kSegStage` arm in + // CheckBlobIsHonest. EncodeRecord then returns a real seq and the child exits 0. + const ChildResult r = RunInChild([] { + Wire2 wire; + std::vector replyBytes(4096, 0); + wire.Segments().Install(kSegReply, SegmentView{replyBytes.data(), replyBytes.size()}); + MGPSamplerDesc desc{}; + desc.Cso = MakeHandle(88); + desc.Parameters.Seg = static_cast(kSegReply); + desc.Parameters.Offset = 0; + desc.Parameters.Size = sizeof(SamplerParameters); + (void)wire.Encoder().EncodeRecord(MGPWireOp::CreateSamplerState, &desc, sizeof(desc)); + }); + ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log; + EXPECT_NE(r.Log.find("is not SEG_STAGE"), std::string::npos) << r.Log; +} + TEST_F(PipeWireCodecTest, AHalfDeclaredBlobIsFatalRatherThanReadAsAbsent) { // The shape a MONOLITH emitter produces - Seg None, Offset a host address, Size 0. Reading // it as "absent" would silently drop the bytes of every record an unconverted emitter sent. From bb6ba28241d4d8d657d16e97ea8c1f275da9cefe Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 06:41:01 -0400 Subject: [PATCH 07/10] [Fix] (MG_Remote, Wire): send the codec's MapPersistent arm through the same adoption-tier gate the applier uses, so MOBILEGL_IPC_ADOPT_TIER=0/1 die on their named diagnostic --- MobileGL/MG_Remote/Wire/PipeWireCodec.cpp | 16 ++++ MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp | 86 +++++++++++++++++++++ 2 files changed, 102 insertions(+) diff --git a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp index d79af8ff..f6c0b846 100644 --- a/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp +++ b/MobileGL/MG_Remote/Wire/PipeWireCodec.cpp @@ -31,6 +31,9 @@ #include #include #include +// R-6's tier gate, and the ONE spelling of it (b1's file, unchanged by this package): the +// MapPersistent arm below asks it the same question MGPipeApplyMapPersistent asks. +#include #include #include #include @@ -1379,6 +1382,19 @@ 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). + // + // THE TIER IS CONSULTED HERE, AND IT IS THE SAME CONJUNCTION THE MONOLITH APPLIER + // USES (PipeApply.cpp's `Transport != Monolith && AdoptTierIsEmulate()`). The arm + // used to decline UNCONDITIONALLY and AdoptTier had no reference anywhere on the + // codec path, so MOBILEGL_IPC_ADOPT_TIER=0 and =1 - which contract §5 promises + // "parse and are Fatal at use, naming P11" - decoded as an ordinary DECLINED and + // the operator got a run that looked like a working T0. AdoptTierIsEmulate returns + // true at T2 and ABORTS at T0/T1 on its own named diagnostic, so the return value + // is deliberately not a branch: P5 declines at every tier it survives (R-6), and + // the two forbidden ones never get this far. + if (MG_Config::Transport != MG_Config::TransportMode::Monolith) { + (void)MG_Remote::Client::AdoptTierIsEmulate(); + } PostReply(op, seq, ReplySink::kStatusDeclined, nullptr, 0); return true; diff --git a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp index d6a7878a..e61acbdb 100644 --- a/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp +++ b/MobileGL/MG_Test/Wire/PipeWireCodecTest.cpp @@ -34,6 +34,8 @@ #include "Includes.h" +// MG_Config::Transport and MG_Config::Ipc.AdoptTier: the two knobs R-6's tier gate reads. +#include #include #include #include @@ -516,6 +518,37 @@ TEST_F(PipeWireCodecTest, KReplySlotMapPersistentIsAConstantDecline) { EXPECT_TRUE(wire.Answers().All[0].Bytes.empty()); } +TEST_F(PipeWireCodecTest, TierTwoUnderSplitTransportStillDeclinesRatherThanRefusing) { + // The POSITIVE half of the two AdoptTier death cases below. Without it, those two could + // be satisfied by an arm that aborted on every tier, which is the opposite mistake to the + // one wave1-codex-verify.md §4 found. T2 is the only tier P5 implements and R-6 says the + // answer there is DECLINED - a real answer, not a failure - even when the transport is + // the split one that makes the tier question live at all. + const MG_Config::TransportMode savedTransport = MG_Config::Transport; + const Uint32 savedTier = MG_Config::Ipc.AdoptTier; + struct Restore { + MG_Config::TransportMode T; + Uint32 A; + ~Restore() { + MG_Config::Transport = T; + MG_Config::Ipc.AdoptTier = A; + } + } restore{savedTransport, savedTier}; + MG_Config::Transport = MG_Config::TransportMode::InProcess; + MG_Config::Ipc.AdoptTier = 2u; + + Wire2 wire; + const MGPHandleOnly handle = HandleOnly(5, MGPipeKind::Buffer); + ASSERT_NE(wire.Encoder().EncodeRecord(MGPWireOp::MapPersistent, &handle, sizeof(handle)), + kInvalidSeq); + bool applied = false; + ASSERT_TRUE(wire.PumpOne(&applied)); + EXPECT_TRUE(applied); + ASSERT_EQ(wire.Answers().All.size(), 1u); + EXPECT_EQ(wire.Answers().All[0].Status, ReplySink::kStatusDeclined); + EXPECT_TRUE(wire.Answers().All[0].Bytes.empty()); +} + TEST_F(PipeWireCodecTest, KNeedsAckRespecifyCarriesItsRedefinitionScope) { // Contract table 1 row 19b. Without the carrier every per-level glTexImage*D would take // the whole-resource arm on the far side and eat the other levels' pending uploads, so @@ -1818,6 +1851,59 @@ TEST_F(PipeWireCodecTest, ASecondProcessResolverIsFatalRatherThanASilentRace) { EXPECT_NE(r.Log.find("already installed"), std::string::npos) << r.Log; } +// ---- R-6 / contract §5: the two forbidden adoption tiers die ON THE WIRE PATH TOO -------- +// +// wave1-codex-verify.md §4: `AdoptTier` had ZERO references anywhere on the codec path, so +// MOBILEGL_IPC_ADOPT_TIER=0 and =1 - which contract §5 promises "parse and are Fatal at use, +// naming P11" - decoded as an ordinary DECLINED. The verifier set each forbidden tier inside +// KReplySlotMapPersistentIsAConstantDecline and watched its successful-decline assertions +// still pass, on BOTH tiers. +// +// THESE ARE FORKED, NOT EXPECT_DEATH, for the reason at the top of this file - and forking is +// what lets the case REQUIRE THE DIAGNOSTIC rather than any abort: r.Log is searched for the +// exact sentence AdoptTierIsEmulate prints. ID-46 finding 10 is an empty death regex; the +// EXPECT_NE lines below are the opposite of that, and a crash for any other reason fails the +// case on the log it prints. +// +// I made both red once, by doing X: X = restoring the unconditional decline in +// PipeWireCodec.cpp's MapPersistent arm (deleting the AdoptTierIsEmulate call). Both children +// then exit 0 having posted a clean DECLINED, and both cases fail on DiedOfAbort. + +TEST_F(PipeWireCodecTest, AdoptTierZeroIsFatalOnTheWirePathAndNamesP11) { + const ChildResult r = RunInChild([] { + // The child dies; nothing needs restoring. The transport half is the same conjunction + // MGPipeApplyMapPersistent uses - a monolith TRANSPORT mints like push (ID-42) and is + // not the arm this record can arrive on. + MG_Config::Transport = MG_Config::TransportMode::InProcess; + MG_Config::Ipc.AdoptTier = 0u; + Wire2 wire; + const MGPHandleOnly handle = HandleOnly(5, MGPipeKind::Buffer); + (void)wire.Encoder().EncodeRecord(MGPWireOp::MapPersistent, &handle, sizeof(handle)); + bool applied = false; + (void)wire.PumpOne(&applied); + }); + ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log; + EXPECT_NE(r.Log.find("MOBILEGL_IPC_ADOPT_TIER=0 names adoption tier T0, which P11 implements"), + std::string::npos) + << r.Log; +} + +TEST_F(PipeWireCodecTest, AdoptTierOneIsFatalOnTheWirePathAndNamesP11) { + const ChildResult r = RunInChild([] { + MG_Config::Transport = MG_Config::TransportMode::InProcess; + MG_Config::Ipc.AdoptTier = 1u; + Wire2 wire; + const MGPHandleOnly handle = HandleOnly(5, MGPipeKind::Buffer); + (void)wire.Encoder().EncodeRecord(MGPWireOp::MapPersistent, &handle, sizeof(handle)); + bool applied = false; + (void)wire.PumpOne(&applied); + }); + ASSERT_TRUE(DiedOfAbort(r)) << DescribeStatus(r) << "\n" << r.Log; + EXPECT_NE(r.Log.find("MOBILEGL_IPC_ADOPT_TIER=1 names adoption tier T1, which P11 implements"), + std::string::npos) + << r.Log; +} + #else TEST_F(PipeWireCodecTest, TheFatalArmsNeedFork) { From 2e2b41ab49aeaa7fb894642132ef71ffae65fb00 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Wed, 16 Sep 2026 06:58:34 -0400 Subject: [PATCH 08/10] [Fix, Test] (workflows, scripts/ci): make the three CI negative controls assert their own failure reason instead of accepting any non-zero ctest exit, count the arming baseline by PASSED rather than by not-skipped, and put both control bodies in files a smoke test can drive against a stubbed ctest --- .github/workflows/test.yml | 130 ++++++++---------- scripts/ci/control_smoke_test.sh | 106 +++++++++++++++ scripts/ci/junit_tally.py | 49 +++++++ scripts/ci/redcheck_control_smoke_test.sh | 71 ++++++++++ scripts/ci/retrace_pull_library_control.sh | 117 ++++++++++++++++ scripts/ci/split_negative_controls.sh | 151 +++++++++++++++++++++ scripts/ci/testdata/stub_ctest.sh | 119 ++++++++++++++++ 7 files changed, 667 insertions(+), 76 deletions(-) create mode 100755 scripts/ci/control_smoke_test.sh create mode 100755 scripts/ci/junit_tally.py create mode 100755 scripts/ci/redcheck_control_smoke_test.sh create mode 100644 scripts/ci/retrace_pull_library_control.sh create mode 100755 scripts/ci/split_negative_controls.sh create mode 100755 scripts/ci/testdata/stub_ctest.sh diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c1d81e2e..896e8ad9 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1074,57 +1074,32 @@ jobs: # Split entries SKIP and ctest reports green whatever the knob says, so an unconditional # control would be red for the whole of P5 for a reason that is not a defect. # - # So the expected state is DERIVED rather than assumed, from the same fact the lanes derive - # it from: MG_IntegrationTest/CMakeLists.txt puts MGITEST_REMOTE_CLIENT_PRESENT=1 into the - # Split entries' ENVIRONMENT exactly when MG_Remote carries no c0 signature stub, and that - # string is in the generated ctest include files this artifact ships. When it is there the - # controls MUST fire; when it is not, the step says so loudly and does not pretend. + # So the expected state is DERIVED FROM BEHAVIOUR rather than assumed. The first version read + # MGITEST_REMOTE_CLIENT_PRESENT out of the generated *_tests.cmake, which was a restatement of + # the CMake source probe review finding M-1 falsified; the arming condition is a runtime fact + # inside each test process (MG_Config::Transport, ClientSession::Active() and + # ImplementedVerbCount(), read by Harness/SplitRuntimePeek), so the only honest way to ask it + # from a shell is to look at what the entries DID. When entries passed, the controls MUST + # fire; when every one of them skipped, the step says so loudly and does not pretend. + # + # THE BODY OF THIS STEP IS scripts/ci/split_negative_controls.sh, and the move is the point + # rather than tidiness. A `run:` block executes nowhere but on a runner, so these lines were + # unreviewable and untestable: when the wave-1 cross-family review said they were broken, + # CONFIRMING it needed a hand-made copy of them (wave1-codex-verify.md 8), and a copy is not + # the thing. scripts/ci/control_smoke_test.sh now drives the very file this step runs. + # + # What that smoke test pins, and what ID-46 finding 8 found missing: each control asserts its + # OWN failure reason. A non-zero ctest exit used to be enough, so a timeout, a setup abort or + # any unrelated assertion printed "turned N selected entries red, as it must" and this step + # went green. The arming run's `|| true` had the matching defect - it counted a case that ran + # and FAILED as evidence the lane was live, so the controls could be measured against a + # baseline that was already red. - name: Negative controls - the verb barrier and the persistent-map push must be load-bearing working-directory: build-split env: MOBILEGL_ITEST_REQUIRE_GPU: "1" - run: | - # THE ARMED STATE IS DERIVED FROM BEHAVIOUR, not from a marker string in the generated - # ctest files. The first version read MGITEST_REMOTE_CLIENT_PRESENT out of - # *_tests.cmake, which was a restatement of the CMake source probe review finding M-1 - # falsified; the arming condition is now a runtime fact inside each test process, so the - # only honest way to ask it from a shell is to look at what the entries DID. - ctest -L integration-split -j 4 --no-tests=error --output-junit "${RUNNER_TEMP}/isplit.xml" || true - armed=$(python3 - "${RUNNER_TEMP}/isplit.xml" <<'PY' - import sys, xml.etree.ElementTree as ET - ran = 0 - for case in ET.parse(sys.argv[1]).getroot().iter('testcase'): - if case.find('skipped') is None and case.get('status') not in ('notrun', 'disabled'): - ran += 1 - print(ran) - PY - ) - echo "split entries that actually ran: ${armed}" - if [ "${armed}" -lt 1 ]; then - echo "::warning::every DirectGLES.Split. entry SKIPPED, so neither negative control can fire. The arming condition is a runtime fact - MG_Config::Transport, ClientSession::Active() and ImplementedVerbCount(), read by Harness/SplitRuntimePeek - and it becomes true on the commit that lands the last of c1/s1/v1. This step becomes a gate then, with no edit; it is not a green that asserted anything today." - exit 0 - fi - run_control() { - name="$1"; filter="$2"; shift 2 - matched=$(ctest -N -L integration-split -R "${filter}" | grep -cE '^ *Test *#[0-9]+:') - if [ "${matched}" -lt 1 ]; then - echo "::error::${name} selected ${matched} tests; its filter no longer matches anything" - exit 1 - fi - if env "$@" ctest --output-on-failure -L integration-split -R "${filter}" --no-tests=error; then - echo "::error::${name} left ${matched} split entries GREEN, so the knob it turns is not load-bearing and the gate it controls proves nothing." - exit 1 - fi - echo "${name} turned ${matched} selected entries red, as it must" - } - # E1: R-1's lockstep verb barrier. Without it the client keeps pulling fields from a live - # GLContext while the server runs ahead, so the server reads future values. - run_control "negative control E1 (MOBILEGL_IPC_VERB_BARRIER=0)" \ - 'DirectGLES\.Split\.(Triangle|ClearThenReadPixels)' MOBILEGL_IPC_VERB_BARRIER=0 - # E3(a): the persistent-map push. 0 is admitted by ConfigLoader on purpose and is - # documented there as this control. - run_control "negative control E3(a) (MOBILEGL_IPC_PERSISTENT_BLOCK_KB=0)" \ - 'DirectGLES\.Split\.PersistentCoherentMapScenario' MOBILEGL_IPC_PERSISTENT_BLOCK_KB=0 + CONTROL_TMPDIR: ${{ runner.temp }} + run: bash "${GITHUB_WORKSPACE}/scripts/ci/split_negative_controls.sh" - name: Upload split lane logs if: always() @@ -1959,37 +1934,25 @@ jobs: # The rerun replays into the same case directory, so the good run's images are put aside and # restored whichever way the control goes; "Upload actual image" below runs `if: always()` # and would otherwise ship the deliberately-wrong run's output under the good run's name. + # THE BODY OF THIS STEP IS scripts/ci/retrace_pull_library_control.sh, for the reason the + # split lane's control gives: a `run:` block cannot be executed off a runner, so these lines + # could not be tested until they ran in CI. scripts/ci/control_smoke_test.sh drives that file. + # + # Two holes ID-46 finding 8(b) found in this block, both CONFIRMED against the REAL ctest in a + # REAL build tree, both closed in the script: it had NO selection guard at all - unlike the + # split lane's run_control - so a case/backend regex matching nothing exited 8 through + # `--no-tests=error` and was read as "the pull library turned it red"; and only "non-zero + # ctest" was checked after the nm identity check, so a loader failure, a missing fixture or a + # timeout passed it. The red must now carry run_trace_case.cmake's own sentence. - name: Negative control - the PULL library must red this split retrace working-directory: build-retrace/tools/trace_replay - run: | - set +e - GOOD_OUTPUT="${RUNNER_TEMP}/split-verified-output" - rm -rf "${GOOD_OUTPUT}" - if [ -d "${{ matrix.case }}" ]; then cp -a "${{ matrix.case }}" "${GOOD_OUTPUT}"; fi - # The pull library, unpacked from build-linux's artifact, over the frozen path every - # case has baked in. It defines no MG_Remote symbol, so ConfigLoader has no transport - # parser and MOBILEGL_TRANSPORT=inproc is accepted and ignored - the exact shape of "the - # split lane ran monolith". - cp "${GITHUB_WORKSPACE}/pull-runtime/build-linux/libMobileGL.so" \ - "${GITHUB_WORKSPACE}/build-linux/libMobileGL.so" - if nm --defined-only "${GITHUB_WORKSPACE}/build-linux/libMobileGL.so" | grep -q -i MG_Remote; then - echo "::error::the control's own library defines MG_Remote symbols, so it is not a pull build and this control would prove nothing" - exit 1 - fi - export MOBILEGL_TRANSPORT=inproc - ctest -V --no-tests=error --timeout 10800 \ - -R "^MobileGLTraceReplay\.${{ matrix.case }}\.${{ matrix.backend }}$" - control_rc=$? - set -e - if [ -d "${GOOD_OUTPUT}" ]; then - rm -rf "${{ matrix.case }}"; mv "${GOOD_OUTPUT}" "${{ matrix.case }}" - echo "restored the verified run's output over the control's" - fi - if [ "${control_rc}" -eq 0 ]; then - echo "::error::a PULL library passed the split retrace. OpenRA scores ssim 1.000000 under a monolith library too (measured), so the picture is not and cannot be this lane's gate - run_trace_case.cmake's transport-resolution assertion is, and it has stopped working. Every green in this job is then a monolith run under a name that says split." - exit 1 - fi - echo "the pull library turned the split retrace red, as it must (ctest exit ${control_rc})" + env: + CONTROL_TMPDIR: ${{ runner.temp }} + PULL_LIBRARY: ${{ github.workspace }}/pull-runtime/build-linux/libMobileGL.so + FROZEN_LIBRARY: ${{ github.workspace }}/build-linux/libMobileGL.so + run: >- + bash "${GITHUB_WORKSPACE}/scripts/ci/retrace_pull_library_control.sh" + '${{ matrix.case }}' '${{ matrix.backend }}' # The refusal census, recorded rather than gated. run_trace_case.cmake already REDS the case # on any Fatal{, so reaching here means the count is zero - but the number and the distinct @@ -2314,6 +2277,21 @@ jobs: python3 scripts/gen_pipe_field_ownership.py --check python3 scripts/gen_pipe_field_ownership.py --self-test + # R-16 APPLIED TO THE NEGATIVE CONTROLS THEMSELVES. The split lane's E1/E3(a) controls and the + # retrace lane's pull-library control are gates, and until ID-46 finding 8 neither could be + # made red by anyone: their bodies were `run:` blocks, which execute only on a runner. Both + # bodies now live in scripts/ci/, and this step runs them against a stubbed ctest that + # reproduces the finding - a NON-EMPTY selection failing with UNRELATED_CONTROL_FAILURE, and a + # case/backend regex matching no tests - and requires each control to report FAILED. The same + # stub, failing with the diagnostics the scenarios really emit, must make them report PASSED. + # + # NO BRANCH GUARD: this asks "do the negative controls still reject a red that is not theirs", + # which is a question every branch can answer and none of which depends on the TEMPORARY + # feat/disaggregated trigger at the top of this file. It costs a couple of seconds and needs + # no build. + - name: The split and retrace negative controls still reject a red that is not theirs (R-16) + run: bash scripts/ci/control_smoke_test.sh + # A GATE as of P3a (G5). "pool 与延迟释放原样搬" (ROADMAP.md:19) is meant literally: the # buffer pool, the deferred-release drain and the three persistently mapped rings move # VERBATIM, and ARCHITECTURE.md:515 says why - their retire happens only inside Present, so diff --git a/scripts/ci/control_smoke_test.sh b/scripts/ci/control_smoke_test.sh new file mode 100755 index 00000000..d26fc2fc --- /dev/null +++ b/scripts/ci/control_smoke_test.sh @@ -0,0 +1,106 @@ +#!/bin/bash +# R-16 FOR THE CI NEGATIVE CONTROLS THEMSELVES: a control-run smoke test. +# +# BRIEF-P5 13 (R-16) says a negative control must assert its own failure reason and that every gate +# carries a line saying "I made it red once, by doing X". The two controls this file exercises ARE +# gates, and until ID-46 finding 8 nobody could make either of them red, because a workflow `run:` +# block only executes on a runner. The wave-1 verification agent had to hand-copy the blocks into +# throwaway harnesses to show they were broken (wave1-codex-verify.md 8). This file is that +# experiment, kept: it runs the REAL control scripts - the same files .github/workflows/test.yml +# invokes, not copies of them - against a stubbed ctest, and checks that each one passes exactly +# when it should. +# +# The case that matters is the first one. A stubbed ctest reports a NON-EMPTY selection and then +# fails with UNRELATED_CONTROL_FAILURE: a reason that has nothing to do with the knob the control +# turns. Before ID-48's fix both controls printed their success message and the step exited 0. They +# must now report FAILED. +# +# usage: control_smoke_test.sh +set -u + +HERE="$(cd "$(dirname "$0")" && pwd)" +WORK="$(mktemp -d)" +trap 'rm -rf "${WORK}"' EXIT + +STUB_DIR="${WORK}/stub" +mkdir -p "${STUB_DIR}" +cp "${HERE}/testdata/stub_ctest.sh" "${STUB_DIR}/ctest" +chmod +x "${STUB_DIR}/ctest" + +passes=0 +failures=0 + +# expect