From 1e3a74686f223b9994959b6c404b37b123872a3e Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Sun, 6 Sep 2026 02:38:38 -0400 Subject: [PATCH] [Test] (Pipe): give the arming assertion a lane and a log of its own, and stop the poison child calling a sequence it never ran a success - the arming case read the ambient lane's MOBILEGL_LOG_FILE_PATH, and that log is opened fopen(path, "w") by every process in the lane: with 406 entries per backend and CI running them -j 4, a whole-file read races a neighbour's bring-up, and the file that survives the lane holds only the LAST writer. Every other log-reading scenario in this suite (UnlocatedIoBlocks, the primgen reroute, the point-size demotion) is registered in a filtered lane with its own log for exactly that reason; PipeVerifyArmingScenario.Armed now follows them, in DirectGLES.VerifyArming. / DirectVulkan.VerifyArming., and skips anywhere MGITEST_PIPE_ARMING_LANE is unset - what that can prove is written down where it is asserted: arming is a property of (this library, this environment) and these two processes share both with their ~400 ambient siblings. A per-process census is not available through a shared log, and a comment that claimed one was the reason CI grepped a file that could not answer - the re-exec'd poison child ran RunSequence() and then _exit(0) unconditionally, so a fatal assertion inside it - the shader failing to compile, say - returned before the draw and the glGenerateMipmap and still reported success: WithoutOmissionCompletes, the one green entry negative control B turns red, passed on a child that ran none of the sequence. It now exits HasFailure() ? 1 : 0, and checks glGetError() after the mipmap so a rejected sequence is part of the answer rather than stderr nobody reads --- MobileGL/MG_IntegrationTest/CMakeLists.txt | 52 +++++++++++++++++-- .../Scenarios/PipeVerifyArmingScenario.cpp | 38 +++++++++++--- .../Scenarios/PoisonOmissionScenario.cpp | 26 ++++++++-- 3 files changed, 103 insertions(+), 13 deletions(-) diff --git a/MobileGL/MG_IntegrationTest/CMakeLists.txt b/MobileGL/MG_IntegrationTest/CMakeLists.txt index c4bc3d4a..c8100d56 100644 --- a/MobileGL/MG_IntegrationTest/CMakeLists.txt +++ b/MobileGL/MG_IntegrationTest/CMakeLists.txt @@ -657,9 +657,14 @@ gtest_discover_tests(MobileGLIntegrationTest # MOBILEGL_PIPE_POISON_OMIT. That is what lets CI's two always-on negative-control steps # export those knobs in the JOB environment and have them reach the test processes; a # property entry of the same name would silently win and the controls would prove nothing. -# * MOBILEGL_LOG_FILE_PATH is per lane. It is the only channel a test process has for reading -# the library's own report (MG_Config is not reachable from this module), and the log is -# opened with fopen(path, "w"), so each process truncates it and the cases can trust it. +# * MOBILEGL_LOG_FILE_PATH is per lane, and "per lane" is the exact limit of what it proves. It +# is the only channel a test process has for reading the library's own report (MG_Config is not +# reachable from this module), but the log is opened fopen(path, "w"), so every process in a +# lane TRUNCATES it: after an ambient lane of 400-odd entries the file holds the LAST process +# and nothing else. Reading it is therefore only sound in a filtered, one-entry lane - which is +# why the arming case has a lane and a log of its own below, and why neither this file nor CI +# may read the ambient logs as evidence about the entries that ran before the last one. The +# ambient path is kept for post-mortems (and to keep library chatter out of ctest's capture). if (MOBILEGL_PIPE_VERIFY) # 900s, not the ambient 120: the comparator re-reads every field of the fill mask at the verb # boundary and again at every accessor read, which the design budgets at 5-10x. @@ -674,6 +679,24 @@ if (MOBILEGL_PIPE_VERIFY) "MOBILEGL_LOG_FILE_PATH=${CMAKE_CURRENT_BINARY_DIR}/pipe-verify-DirectVulkan.log" ${MGL_ITEST_VULKAN_ENV}) + # The arming assertion's own lane, one case per backend, with a log path nothing else writes to. + # + # PipeVerifyArmingScenario.Armed reads the library's log, and the log is a per-LANE resource: it + # is opened fopen(path, "w"), so every process in a lane truncates it. In the ambient Verify. + # lane that is 400-odd processes on one path, run `-j 4` in CI, and a whole-file read there + # races a neighbour's bring-up. Every other log-reading scenario in this file (UnlocatedIoBlocks, + # the primgen reroute, the point-size demotion) is registered exactly like this for the same + # reason. MGITEST_PIPE_ARMING_LANE is a harness marker - the library never reads it - and it is + # what makes the case skip in the ambient lane instead of racing there. + mgl_itest_join_environment(MGL_ITEST_GLES_VERIFY_ARMING_ENVIRONMENT + "MOBILEGL_BACKEND_TYPE=DirectGLES" "MOBILEGL_PIPE_VERIFY=1" "MGITEST_PIPE_ARMING_LANE=1" + "MOBILEGL_LOG_FILE_PATH=${CMAKE_CURRENT_BINARY_DIR}/pipe-verify-arming-DirectGLES.log" + ${MGL_ITEST_COMMON_ENV}) + mgl_itest_join_environment(MGL_ITEST_VULKAN_VERIFY_ARMING_ENVIRONMENT + "MOBILEGL_BACKEND_TYPE=DirectVulkan" "MOBILEGL_PIPE_VERIFY=1" "MGITEST_PIPE_ARMING_LANE=1" + "MOBILEGL_LOG_FILE_PATH=${CMAKE_CURRENT_BINARY_DIR}/pipe-verify-arming-DirectVulkan.log" + ${MGL_ITEST_VULKAN_ENV}) + # Negative control A (G4). MOBILEGL_PIPE_VERIFY_FATAL=0 so the process SURVIVES its own # divergence and the case can read the report back out of the log; the CI step that exports # the same corruption against the ambient lane, where FATAL keeps its default of 1, asserts @@ -725,6 +748,29 @@ if (MOBILEGL_PIPE_VERIFY) ENVIRONMENT "${MGL_ITEST_VULKAN_VERIFY_ENVIRONMENT}" ) + # The arming assertion, one entry per backend. This is the entry that fails a lane whose library + # never armed: it runs the same library and the same MOBILEGL_PIPE_VERIFY=1 as the ambient + # entries above, but unlike them it cannot be green against a library with no comparator + # compiled in. Its log is its own, so `-j 4` cannot make it flake. + gtest_discover_tests(MobileGLIntegrationTest + TEST_PREFIX "DirectGLES.VerifyArming." + TEST_FILTER "PipeVerifyArmingScenario.Armed" + DISCOVERY_TIMEOUT 30 + PROPERTIES + LABELS "integration-gpu\;integration-verify" + TIMEOUT ${MGL_ITEST_VERIFY_TIMEOUT} + ENVIRONMENT "${MGL_ITEST_GLES_VERIFY_ARMING_ENVIRONMENT}" + ) + gtest_discover_tests(MobileGLIntegrationTest + TEST_PREFIX "DirectVulkan.VerifyArming." + TEST_FILTER "PipeVerifyArmingScenario.Armed" + DISCOVERY_TIMEOUT 30 + PROPERTIES + LABELS "integration-gpu\;integration-verify" + TIMEOUT ${MGL_ITEST_VERIFY_TIMEOUT} + ENVIRONMENT "${MGL_ITEST_VULKAN_VERIFY_ARMING_ENVIRONMENT}" + ) + # One case each: the knobs are process-wide, so a corrupted or poisoned process cannot also be # running the ambient assertions. These four entries are the ones that assert the RED - they # pass when the comparator and the poison report, and go red when either stops. diff --git a/MobileGL/MG_IntegrationTest/Scenarios/PipeVerifyArmingScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/PipeVerifyArmingScenario.cpp index 5ce95138..c7dca253 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/PipeVerifyArmingScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/PipeVerifyArmingScenario.cpp @@ -30,12 +30,25 @@ // (on Android it links the SHIPPING libMobileGL.so, built -fvisibility=hidden) and the arming // signal is a latched MGLOG_I. The ctest entry sets MOBILEGL_LOG_FILE_PATH; this only reads it. // -// Note on scope: the log file is opened with fopen(path, "w") at the first log write of a process -// (MG_Util/Debug/Log.cpp, InitFile), so the file holds THIS process's lines and nothing else - a -// whole-file search cannot be satisfied by a sibling ctest entry of the same lane. The arming line -// is latched at the FIRST fill of the process, which may be the harness bring-up rather than this -// test's draw, so the arming search is whole-file on purpose; the divergence search is restricted -// to the bytes this case appended, which is where a differ belongs. +// Note on scope, and why Armed runs in a lane of its own. The log file is opened with +// fopen(path, "w") at the first log write of a process (MG_Util/Debug/Log.cpp, InitFile), so each +// process TRUNCATES it. That is fine for one process and false for many: in the ambient Verify. +// lane, 400-odd sibling entries share the one MOBILEGL_LOG_FILE_PATH, and CI runs that lane with +// `ctest -j 4`, so a neighbour's bring-up can truncate the file between this case's draw and its +// read. Every existing scenario in this suite that reads the library log (UnlocatedIoBlockScenario, +// the primgen reroute, the point-size demotion) is registered in a FILTERED lane with a log path of +// its own for exactly that reason, and this case now follows them: it runs in the VerifyArming. +// entries, which set MGITEST_PIPE_ARMING_LANE=1 and their own log, and skips everywhere else. +// +// What that proves, stated honestly: the arming line is a property of (this library, this +// environment), not of an individual test body, and the VerifyArming. entry runs the same library +// with the same MOBILEGL_PIPE_VERIFY=1 as its ~400 ambient siblings. One process per backend is +// therefore the whole of the evidence available for "the lane armed" - the per-process claim the +// shared log CANNOT support, because it only ever holds the last writer. +// +// Within the process: the arming line is latched at the FIRST fill, which may be the harness +// bring-up rather than this test's draw, so the arming search is whole-file on purpose; the +// divergence search is restricted to the bytes this case appended, which is where a differ belongs. #include #include @@ -64,6 +77,12 @@ namespace MGITest { constexpr const char* kDifferPrefix = "Fatal{PipeVerifyDiffer"; constexpr const char* kUnmigratedPrefix = "Fatal{UnmigratedPipeInput"; + // Set by the VerifyArming. ctest entries and by nothing else. It is a HARNESS variable, not + // a library knob (hence the MGITEST_ prefix): the library never reads it. It exists because + // this case reads a log file, and a log file is a per-LANE resource - see the note at the + // top of the file. + constexpr const char* kArmingLaneMarker = "MGITEST_PIPE_ARMING_LANE"; + constexpr const char* kVS = R"(#version 330 core in vec2 aPos; void main() { gl_Position = vec4(aPos, 0.0, 1.0); } @@ -158,6 +177,13 @@ void main() { o_color = vec4(0.25, 0.5, 0.75, 1.0); } TEST_F(PipeVerifyArmingScenario, Armed) { if (!Ready()) return; + if (!StringKnobIsSet(kArmingLaneMarker)) { + GTEST_SKIP() << "this case reads the library's log file, so it runs in the VerifyArming. " + "lane, which owns a log path no other entry writes to. In the ambient " + "Verify. lane 400-odd entries share one path and each truncates it " + "(Log.cpp opens it \"w\"), so a whole-file read here would race a " + "neighbour under ctest -j 4. Set by the ctest entry, never by hand."; + } if (AmbientQuirkFromEnvironment("MOBILEGL_PIPE_VERIFY") != AmbientQuirk::On) { GTEST_SKIP() << "this case needs MOBILEGL_PIPE_VERIFY=1 for the whole process, which is " "what the Verify. ctest entries set; with the variable unset the " diff --git a/MobileGL/MG_IntegrationTest/Scenarios/PoisonOmissionScenario.cpp b/MobileGL/MG_IntegrationTest/Scenarios/PoisonOmissionScenario.cpp index 79f859d0..d6b1568e 100644 --- a/MobileGL/MG_IntegrationTest/Scenarios/PoisonOmissionScenario.cpp +++ b/MobileGL/MG_IntegrationTest/Scenarios/PoisonOmissionScenario.cpp @@ -182,6 +182,15 @@ void main() { o_color = vec4(0.25, 0.5, 0.75, 1.0); } glFinish(); std::fprintf(stderr, "[itest] poison worker: glGenerateMipmap returned\n"); + // The sequence is the WHOLE datum this child reports, so a GL error in it must be + // part of the answer rather than something only a human reading stderr would see. + // WithoutOmissionCompletes reads the child's exit status, and the status is built + // from HasFailure() below - so this EXPECT is what turns "the mipmap was rejected" + // into a red parent instead of a vacuous "it exited 0, the poison did not fire". + EXPECT_EQ(FirstGLError(), 0u) + << "the draw + glGenerateMipmap sequence the poison controls are about raised a " + "GL error, so neither control is measuring what it claims to measure"; + glBindVertexArray(0); glDeleteBuffers(1, &vbo); glDeleteVertexArrays(1, &vao); @@ -277,8 +286,14 @@ void main() { o_color = vec4(0.25, 0.5, 0.75, 1.0); } // _exit, and not a return into gtest's teardown: this process exists to reach the verb // above and its exit status is the datum the parent reads. A normal teardown of a live // context could add signals of its own to that answer. + // + // HasFailure(), not 0: RunSequence() is full of ASSERT_/EXPECT_ macros, and a fatal one + // (the shader failing to compile, say) RETURNS from RunSequence before the draw and the + // glGenerateMipmap ever happen. Exiting 0 there would have WithoutOmissionCompletes pass + // on a child that ran none of the sequence it is the control for - green because nothing + // happened. The child's assertion text is on its stderr, which ctest captures. std::fflush(nullptr); - _exit(0); + _exit(::testing::Test::HasFailure() ? 1 : 0); #endif } @@ -369,9 +384,12 @@ void main() { o_color = vec4(0.25, 0.5, 0.75, 1.0); } "add the row to MG_Pipe/FillPoints.def, never mark the field sticky." << note << " Child log:\n" << childLog; - EXPECT_EQ(WEXITSTATUS(status), 0) << "the child " << DescribeStatus(status) - << ". Child log:\n" - << childLog; + EXPECT_EQ(WEXITSTATUS(status), 0) + << "the child " << DescribeStatus(status) + << ". Status 1 is the child's OWN assertion failing inside the sequence (it exits " + "HasFailure() ? 1 : 0), so its gtest output on this job's stderr names the line; " + "anything else came from the harness. Child log:\n" + << childLog; EXPECT_EQ(childLog.find("Fatal{"), std::string::npos) << "an unpoisoned run logged a Fatal:\n" << childLog;