diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9cbaa52c..5d38f32e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1094,6 +1094,14 @@ jobs: # private log carries the report. This step re-reads those logs from outside, so the claim # does not rest only on the assertion that produced them. # + # AND IT COUNTS, NEEDING TWO (V1 fix round 2). The server reads the pack field twice inside + # the ReadPixels window - PipeApplier::read_pixels saves the application's pack before it + # installs the neutral one, the backend's ReadPixels reads it after - and the hook perturbs + # its oracle once per read, because the ID-49 overwrite inside the window wipes the first + # perturbation before the second read's compare. A step satisfied by ONE line was satisfied + # by either perturbation alone and falsified neither; two is one per read, each falsified on + # its own. Measured at landing: 3 (DirectGLES) / 2 (DirectVulkan). + # # THE GREEN HALF is the lane run above: the same hook, the same window, with no corruption # armed, across 1070 entries at zero divergences. - name: Negative control A (split, read side) - the server's own read must report it @@ -1110,12 +1118,14 @@ jobs: ctest --output-on-failure -L '^integration-verify-split$' -R "${FILTER}" --no-tests=error for backend in DirectGLES DirectVulkan; do log="MobileGL/MG_IntegrationTest/pipe-verify-split-read-corrupt-${backend}.server.log" - if ! grep -q 'Fatal{PipeVerifyDiffer, "GetPixelStoreParameters@ReadPixels".*where=read}' "${log}"; then - echo "::error::${log} carries no compare-at-read report for the corrupted pack field: the hook is not comparing on the server's apply thread, or the ID-49 neutral-pack window swallowed the perturbation" + reads=$(grep -c 'Fatal{PipeVerifyDiffer, "GetPixelStoreParameters@ReadPixels".*where=read}' "${log}" 2>/dev/null || true) + reads=${reads:-0} + if [ "${reads}" -lt 2 ]; then + echo "::error::${log} carries ${reads} compare-at-read report(s) for the corrupted pack field, fewer than the two reads the server makes inside the ReadPixels window (the applier's saved-pack read and the backend's post-install read): the hook is not comparing on the server's apply thread, or one of its two perturbations no longer reaches its read because the ID-49 neutral-pack overwrite swallowed it" exit 1 fi done - echo "both backends reported GetPixelStoreParameters@ReadPixels where=read in the SERVER half, as they must" + echo "both backends reported GetPixelStoreParameters@ReadPixels where=read at least twice in the SERVER half, as they must" # NEGATIVE CONTROL B ON THE SPLIT ARM, WITH THE SPLIT ARM'S PAIR. The monolith pair # (GenerateMipmap:GetActiveTextureUnit) is inert here - the split backend's generate_mipmap diff --git a/MobileGL/MG_Impl/Pipe/PipeFill.cpp b/MobileGL/MG_Impl/Pipe/PipeFill.cpp index b01aabb6..6a588fca 100644 --- a/MobileGL/MG_Impl/Pipe/PipeFill.cpp +++ b/MobileGL/MG_Impl/Pipe/PipeFill.cpp @@ -738,6 +738,17 @@ namespace MobileGL::MG_Pipe { // exactly like a hook with nothing to report. The perturbation goes on the ORACLE, for // EntryCompare's reason: the arm under test is the stored value, so corrupting THAT would // be testing the corruption. + // + // WHICH READ THIS BLOCK TURNS RED (V1 fix round 2). The server reads the pack field TWICE + // inside the ReadPixels verb window, and both reads sit inside + // ServerReadsInsideTheNeutralPackWindow below (the verb stamp is up for the whole apply): + // PipeApplier::read_pixels saves the application's pack through the accessor BEFORE it + // installs the neutral one, and the backend's ReadPixels reads the field AFTER. This block + // is what turns the FIRST of them red: at that read `self` still holds the application's + // pack, so the first compare only reaches the window if this perturbation made it differ, + // and inside the window the application's pack against the neutral oracle differs on its + // own. It does nothing for the second read - `self` IS the neutral pack there - which is + // what the re-application inside the window is for. if (g_verify.Corrupt && *g_verify.Corrupt == field) { MGPipeApplyVerifyCorruption(g_readScratch, field); } @@ -758,10 +769,18 @@ namespace MobileGL::MG_Pipe { }); // AND THE CONTROL IS RE-APPLIED, because the overwrite above replaces the pack half // WHOLESALE - the corruption included, since CorruptStorage perturbs an array's FIRST - // ELEMENT and the pack half is element 0. Without this line the one window in which - // this arm is most load-bearing (the server's read_pixels, 8530 reads over 220 of the - // lane's entries) would be the one window MOBILEGL_PIPE_VERIFY_CORRUPT= - // GetPixelStoreParameters cannot turn red. + // ELEMENT and the pack half is element 0. This is the block for the POST-INSTALL read + // (the backend's ReadPixels, after PipeApplier::read_pixels installed the neutral pack + // into gPipeInputs): there `self` is the neutral pack, the overwrite just made the + // oracle equal to it, and only a perturbation applied AFTER the overwrite can make + // that compare differ - the block above cannot reach it. Without this line the one + // window in which this arm is most load-bearing (the server's read_pixels, 8530 reads + // over 220 of the lane's entries) would be the one window MOBILEGL_PIPE_VERIFY_CORRUPT= + // GetPixelStoreParameters cannot turn red - and a control that accepted ONE report + // would never notice, because the saved-pack read still reports through the block + // above. The VerifySplitReadCorrupted. entries and the CI step therefore require at + // least TWO `where=read` reports in the server half: one per block, each falsified + // alone (V1 fix round 2). if (g_verify.Corrupt && *g_verify.Corrupt == field) { MGPipeApplyVerifyCorruption(g_readScratch, field); } diff --git a/MobileGL/MG_IntegrationTest/Scenarios/PipeVerifyArmingScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/PipeVerifyArmingScenario.cpp index 066f4799..95b8c0e0 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/PipeVerifyArmingScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/PipeVerifyArmingScenario.cpp @@ -111,6 +111,25 @@ void main() { o_color = vec4(0.25, 0.5, 0.75, 1.0); } return value != nullptr && *value != '\0'; } + // The number of LINES in `text` that carry `needle` and `where=read`: one per backend read + // the compare-at-read hook reported for that field and verb (ReportDivergence is a single + // MGLOG_F, so a report never spans lines and two reports never share one). + std::size_t CountReadReports(const std::string& text, const std::string& needle) { + std::size_t count = 0; + std::size_t pos = 0; + while (pos < text.size()) { + const std::size_t eol = text.find('\n', pos); + const std::string line = + text.substr(pos, eol == std::string::npos ? std::string::npos : eol - pos); + if (line.find(needle) != std::string::npos && line.find("where=read") != std::string::npos) { + ++count; + } + if (eol == std::string::npos) break; + pos = eol + 1; + } + return count; + } + class PipeVerifyArmingScenario : public ScenarioTest { protected: // The library log this process is writing, or an empty path when none was configured. @@ -350,6 +369,26 @@ void main() { o_color = vec4(0.25, 0.5, 0.75, 1.0); } "arm - `where=` says which comparator spoke, and only `read` is this case's " "subject. Line:\n" << line; + + // TWO REPORTS, NOT ONE (V1 fix round 2) - the count is what makes BOTH perturbation + // blocks in the hook load-bearing. The server reads this field twice inside the + // ReadPixels window, and both reads sit inside the hook's neutral-pack branch: + // PipeApplier::read_pixels saves the application's pack BEFORE it installs the neutral + // one, and the backend's ReadPixels reads the field AFTER. The hook's first perturbation + // is what turns the saved-pack read red (past the first compare, the application's + // pack against the neutral oracle differs on its own); only the perturbation re-applied + // after the window's overwrite can turn the post-install read red, because there the + // stored value IS the neutral pack. A control satisfied by ONE line is satisfied by + // either block alone and so falsified neither. Measured at landing the server half + // carries 3 (DirectGLES) / 2 (DirectVulkan); removing either block drops it to 1. + const std::size_t reads = CountReadReports(server, expected); + EXPECT_GE(reads, std::size_t{2}) + << "the server half carries " << reads << " `where=read` report(s) for " << expected + << ", ...}. The server reads that field twice inside the ReadPixels window (the " + "applier's saved-pack read and the backend's post-install read) and the hook " + "perturbs the oracle once per read, so fewer than two means one perturbation has " + "stopped reaching its read. Server half of the log appended by this case:\n" + << server; } } // namespace