From 90f9a79058c7b8c8ddf7c468bb59073b32d26499 Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Mon, 21 Sep 2026 19:21:43 -0400 Subject: [PATCH] [MG_Remote] (P6/hs): the build stamp that was silently empty, and two asymmetries the handshake never checked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CONTRACT-P6 4.2 and 4.3. Three defects, all of them things the handshake believed it was checking and was not. 1. THE BUILD STAMP WAS "" AND NOTHING SAID SO. CMakeLists took the git hash through an execute_process with NO RESULT_VARIABLE, so a failed `git rev-parse` yielded the empty string. Verified live rather than inferred: this worktree's `.git` is a file holding a WINDOWS path, WSL's git answers `fatal: not a git repository`, result 128 - and every WSL build mixed "" into its ABI fingerprint. Two builds from different commits then agreed exactly, which is the one thing the stamp exists to prevent. The already-failing SessionHandshakeTest was reporting this the whole time. The fix may NOT touch GIT_COMMIT_HASH_SHORT: it is baked into glGetString(GL_VERSION), so redefining it changes an application-visible GL string and the pull build's .rodata - a behaviour leak and a G1 break at once. MGGitHash.h.in gains a SECOND pair instead, consumed only by the fingerprint, plus -DMOBILEGL_BUILD_STAMP for trees git cannot read (CI has ${GITHUB_SHA} in hand). A stampless build is now a configure-time WARNING naming the consequence, not silence. MOBILEGL_BUILD_STAMP_PRESENT IS MIXED AS ITS OWN FIELD. "this build cannot name its commit" and "this build's commit is the empty string" are different claims, and collapsing them is how every stampless build in the world agreed with every other. The test asserts the flag moves the fingerprint in EITHER build state, and the `BuildStamp = ""` perturbation is now guarded - in a stampless build production's value already IS "", so that assertion was passing while proving nothing. 2. THE CLIENT NEVER READ abiMajor/abiMinor. Hello has carried them since P5 and the SERVER checks them; the client compared only the fingerprint. The fingerprint mixes AbiVersion so a mismatch would fire anyway - but as "struct shapes", which sends the reader looking for a layout change that is not there. Under spawn the peer is a SEPARATE IMAGE the operator chose with MOBILEGL_IPC_SERVER_PATH, so this is exactly the failure that needs its own sentence. Both handshake sites call one helper; there are two sites, and only one of them was reachable before `sm`, which is how the gap survived. 3. Hello::pid WAS 0 AND Welcome::serverPid ECHOED IT. So both were always 0 and neither named anything, while §9.5's arm-proof gate wants exactly the child pid. Both now carry real ids, and the pair is the cheapest arm proof in the protocol: under spawn they MUST differ, and a same-process session cannot produce two different values. The client keeps what the server STATED - local launcher knowledge is a different fact from one that crossed the wire, and only the second can be asserted from the server's own mouth. R-16 both ways: revert serverPid to echoing hello->pid() and the new test reds on both halves, naming 4.3. G1 re-measured after the CMake and header changes: 0 symbol diff, .text unchanged at 0xa52203. Unit suite goes from two long-standing failures to one - the remaining PipeCatalogue.FrontendNeverTakesAnApplierAddress is an environmental path problem in this tree, unrelated. All six OpenRA arms still green. --- CMakeLists.txt | 46 +++++++++++++ MobileGL/MG_Remote/CapsCodec.cpp | 8 ++- MobileGL/MG_Remote/Client/ClientSession.cpp | 69 ++++++++++++++++++- MobileGL/MG_Remote/Client/ClientSession.h | 9 +++ MobileGL/MG_Remote/Server/ServerSession.cpp | 29 +++++++- MobileGL/MG_Remote/Transport/Ring.cpp | 6 ++ MobileGL/MG_Remote/Transport/SessionRings.h | 21 +++++- MobileGL/MG_Test/Wire/ServerSpawnTest.cpp | 31 +++++++++ .../MG_Test/Wire/SessionHandshakeTest.cpp | 30 +++++++- MobileGL/MG_Util/Miscellany/MGGitHash.h.in | 9 +++ 10 files changed, 249 insertions(+), 9 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index e35ac8d7..f1c30fe3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -185,15 +185,61 @@ if (MSVC) endif() # Generate MGGitHash.h +# +# RESULT_VARIABLE, WHICH THIS CALL DID NOT HAVE (CONTRACT-P6 4.2). Without it a failed +# `git rev-parse` yields "" and nothing says so. Verified live: building this worktree from WSL +# produces an EMPTY hash, because the worktree's `.git` is a file holding a WINDOWS path that +# WSL's git cannot resolve - so two builds from different commits got IDENTICAL ABI +# fingerprints and the stamp silently stopped doing its documented job. execute_process( COMMAND git rev-parse HEAD WORKING_DIRECTORY ${CMAKE_SOURCE_DIR} OUTPUT_VARIABLE GIT_COMMIT_HASH_FULL OUTPUT_STRIP_TRAILING_WHITESPACE + RESULT_VARIABLE MOBILEGL_GIT_STAMP_RESULT + ERROR_QUIET ) string(SUBSTRING "${GIT_COMMIT_HASH_FULL}" 0 7 GIT_COMMIT_HASH_SHORT) +# --- the BUILD STAMP, and why it is a second pair rather than a fix to the first ------------- +# +# GIT_COMMIT_HASH_SHORT MAY NOT MOVE. It is baked into glGetString(GL_VERSION) +# (MG_Impl/GLImpl/Getter/GL_Getter.cpp), so redefining it changes an APPLICATION-VISIBLE GL +# string and the pull build's .rodata - a behaviour leak and a G1 break at once. The handshake +# gets its own pair instead, consumed only by the ABI fingerprint, which no pull build compiles. +# +# PRESENT IS A FIRST-CLASS INPUT, not the emptiness of VALUE. "this build has no stamp" and +# "this build's stamp is the empty string" are different claims, and a fingerprint that could +# not tell them apart is how two unrelated builds agreed in the first place. +# +# -DMOBILEGL_BUILD_STAMP= (or the environment variable) overrides, because a tree git +# cannot read is not always a tree whose commit is unknown - CI has ${GITHUB_SHA} in hand. +set(MOBILEGL_BUILD_STAMP "" CACHE STRING "Explicit ABI-fingerprint build stamp when git cannot be read") +set(MOBILEGL_BUILD_STAMP_VALUE "") +if(NOT "${MOBILEGL_BUILD_STAMP}" STREQUAL "") + set(MOBILEGL_BUILD_STAMP_VALUE "${MOBILEGL_BUILD_STAMP}") +elseif(NOT "$ENV{MOBILEGL_BUILD_STAMP}" STREQUAL "") + set(MOBILEGL_BUILD_STAMP_VALUE "$ENV{MOBILEGL_BUILD_STAMP}") +elseif(MOBILEGL_GIT_STAMP_RESULT EQUAL 0 AND NOT "${GIT_COMMIT_HASH_FULL}" STREQUAL "") + set(MOBILEGL_BUILD_STAMP_VALUE "${GIT_COMMIT_HASH_FULL}") +endif() + +if("${MOBILEGL_BUILD_STAMP_VALUE}" STREQUAL "") + set(MOBILEGL_BUILD_STAMP_PRESENT 0) + # A WARNING AND NOT A FATAL: a build with no stamp is still a usable build, it just cannot + # prove that a peer was built from the same tree. Saying so once at configure time is the + # difference between a known limitation and the silent one 4.2 found. + message(WARNING + "MobileGL: no build stamp. `git rev-parse HEAD` failed (result " + "'${MOBILEGL_GIT_STAMP_RESULT}') and MOBILEGL_BUILD_STAMP was not set, so the ABI " + "fingerprint cannot distinguish two builds of the same sizes from different commits - " + "which is exactly what a stale MOBILEGL_IPC_SERVER_PATH looks like. Pass " + "-DMOBILEGL_BUILD_STAMP= to restore it.") +else() + set(MOBILEGL_BUILD_STAMP_PRESENT 1) +endif() + configure_file( ${CMAKE_SOURCE_DIR}/MobileGL/MG_Util/Miscellany/MGGitHash.h.in ${CMAKE_BINARY_DIR}/generated/MGGitHash.h diff --git a/MobileGL/MG_Remote/CapsCodec.cpp b/MobileGL/MG_Remote/CapsCodec.cpp index 12ef320e..ea2019a4 100644 --- a/MobileGL/MG_Remote/CapsCodec.cpp +++ b/MobileGL/MG_Remote/CapsCodec.cpp @@ -485,7 +485,13 @@ namespace MobileGL::MG_Remote { // which is the one break the rest of the mix is blind to. inputs.AbiVersion = static_cast(MOBILEGL_ABI_VERSION(MOBILEGL_PROTOCOL_ABI_MAJOR, MOBILEGL_PROTOCOL_ABI_MINOR)); - inputs.BuildStamp = GIT_COMMIT_HASH_SHORT; + // MOBILEGL_BUILD_STAMP_VALUE, NOT GIT_COMMIT_HASH_SHORT (CONTRACT-P6 4.2). The old + // macro is application-visible through glGetString(GL_VERSION) and may not move, and it + // came from an execute_process with no RESULT_VARIABLE - so a tree git could not read + // produced "" and every such build agreed with every other. The new pair separates the + // VALUE from whether the build could name a commit at all. + inputs.BuildStamp = MOBILEGL_BUILD_STAMP_VALUE; + inputs.BuildStampPresent = MOBILEGL_BUILD_STAMP_PRESENT; return inputs; } diff --git a/MobileGL/MG_Remote/Client/ClientSession.cpp b/MobileGL/MG_Remote/Client/ClientSession.cpp index a57561d0..b166fc15 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.cpp +++ b/MobileGL/MG_Remote/Client/ClientSession.cpp @@ -36,8 +36,29 @@ #include #include +// CONTRACT-P6 4.3: the handshake now carries real process ids. +#if defined(_WIN32) +#include +#else +#include +#endif + namespace MobileGL::MG_Remote::Client { + namespace { + // CONTRACT-P6 4.3. Hello::pid was hard-coded 0 and Welcome::serverPid ECHOED it, so both + // were always 0 and neither named anything. Under spawn the pair is the cheapest arm + // proof there is: two different values mean two processes, and a same-process session + // cannot produce them. + std::uint32_t SelfProcessId() { +#if defined(_WIN32) + return static_cast(::_getpid()); +#else + return static_cast(::getpid()); +#endif + } + } // namespace + #define MGP5_C0_STUB(what) \ do { \ MGLOG_F("MGPipe: Fatal{UnimplementedClientSession, \"%s\"} - P5 packages s1/c1 have not " \ @@ -89,6 +110,30 @@ namespace MobileGL::MG_Remote::Client { return ::MobileGL::Wire::GetCtrlEnvelope(bytes.data()); } + // CONTRACT-P6 4.3: THE DECLARED PROTOCOL VERSION, WHICH THIS SIDE NEVER READ. + // + // Hello has carried abiMajor/abiMinor since P5 and the SERVER checks them, but the + // client only ever compared the fingerprint - so the asymmetry was silent in the one + // direction that matters for spawn, where the peer is a SEPARATE IMAGE the operator + // chose with MOBILEGL_IPC_SERVER_PATH. + // + // The fingerprint mixes AbiVersion and would catch a mismatch anyway, but it catches it + // as "struct shapes" - which sends the reader looking for a layout change that is not + // there. Checking the version FIRST turns the same failure into the sentence that + // explains it. Both handshake sites call this; there are two, and only one of them used + // to be reachable, which is how the gap survived review. + [[noreturn]] void FatalProtocolVersionMismatch(unsigned theirMajor, unsigned theirMinor, + const char* theirStamp) { + MGLOG_F("MGPipe: Fatal{AbiMismatch, \"protocol version\"} ours=%u.%u theirs=%u.%u " + "ourBuild=%s theirBuild=%s - the peer speaks a different MGPipe protocol. " + "Under spawn this is most often a stale MOBILEGL_IPC_SERVER_PATH pointing at " + "a libMobileGLServer.so built from another tree.", + static_cast(MOBILEGL_PROTOCOL_ABI_MAJOR), + static_cast(MOBILEGL_PROTOCOL_ABI_MINOR), theirMajor, theirMinor, + GIT_COMMIT_HASH_SHORT, theirStamp == nullptr ? "?" : theirStamp); + std::abort(); + } + [[noreturn]] void FatalAbiMismatch(const char* what, Uint64 ours, Uint64 theirs, const char* theirStamp) { MGLOG_F("MGPipe: Fatal{AbiMismatch, \"%s\"} ours=%llu theirs=%llu ourBuild=%s " @@ -548,7 +593,7 @@ namespace MobileGL::MG_Remote::Client { auto stamp = builder.CreateString(GIT_COMMIT_HASH_SHORT); auto hello = ::MobileGL::Wire::CreateHello( builder, MOBILEGL_PROTOCOL_ABI_MAJOR, MOBILEGL_PROTOCOL_ABI_MINOR, stamp, - /*backendType=*/0u, /*pid=*/0u, /*configBlob=*/0, fingerprint); + /*backendType=*/0u, /*pid=*/SelfProcessId(), /*configBlob=*/0, fingerprint); auto root = ::MobileGL::Wire::CreateCtrlEnvelope( builder, ::MobileGL::Wire::CtrlMsg::Hello, hello.Union()); ::MobileGL::Wire::FinishCtrlEnvelopeBuffer(builder, root); @@ -593,6 +638,16 @@ namespace MobileGL::MG_Remote::Client { const char* theirStamp = welcome->buildFingerprint() == nullptr ? nullptr : welcome->buildFingerprint()->c_str(); + // 4.3: the pid the SERVER stated, taken before any check can abort, so a Fatal line + // can name it too. + m_peerServerPid = static_cast(welcome->serverPid()); + // The VERSION before the fingerprint: both would fire, and only one names the cause. + if (welcome->abiMajor() != MOBILEGL_PROTOCOL_ABI_MAJOR || + welcome->abiMinor() != MOBILEGL_PROTOCOL_ABI_MINOR) { + FatalProtocolVersionMismatch(static_cast(welcome->abiMajor()), + static_cast(welcome->abiMinor()), + theirStamp); + } if (welcome->abiFingerprint() != fingerprint) { FatalAbiMismatch("struct shapes", fingerprint, welcome->abiFingerprint(), theirStamp); @@ -713,7 +768,7 @@ namespace MobileGL::MG_Remote::Client { auto stamp = builder.CreateString(GIT_COMMIT_HASH_SHORT); auto hello = ::MobileGL::Wire::CreateHello( builder, MOBILEGL_PROTOCOL_ABI_MAJOR, MOBILEGL_PROTOCOL_ABI_MINOR, stamp, - 0u, 0u, 0, fingerprint); + /*backendType=*/0u, /*pid=*/SelfProcessId(), /*configBlob=*/0, fingerprint); auto root = ::MobileGL::Wire::CreateCtrlEnvelope( builder, ::MobileGL::Wire::CtrlMsg::Hello, hello.Union()); ::MobileGL::Wire::FinishCtrlEnvelopeBuffer(builder, root); @@ -754,6 +809,16 @@ namespace MobileGL::MG_Remote::Client { const char* theirStamp = welcome->buildFingerprint() == nullptr ? nullptr : welcome->buildFingerprint()->c_str(); + // 4.3: the pid the SERVER stated, taken before any check can abort, so a Fatal line + // can name it too. + m_peerServerPid = static_cast(welcome->serverPid()); + // The VERSION before the fingerprint: both would fire, and only one names the cause. + if (welcome->abiMajor() != MOBILEGL_PROTOCOL_ABI_MAJOR || + welcome->abiMinor() != MOBILEGL_PROTOCOL_ABI_MINOR) { + FatalProtocolVersionMismatch(static_cast(welcome->abiMajor()), + static_cast(welcome->abiMinor()), + theirStamp); + } if (welcome->abiFingerprint() != fingerprint) { FatalAbiMismatch("struct shapes", fingerprint, welcome->abiFingerprint(), theirStamp); diff --git a/MobileGL/MG_Remote/Client/ClientSession.h b/MobileGL/MG_Remote/Client/ClientSession.h index a9adea71..3ccfd5b5 100644 --- a/MobileGL/MG_Remote/Client/ClientSession.h +++ b/MobileGL/MG_Remote/Client/ClientSession.h @@ -90,6 +90,12 @@ namespace MobileGL::MG_Remote::Client { // then watch this object, not a side effect three layers up. Transport::Doorbell* SelfDoorbellForTest() { return m_producer.SelfDoorbell(); } + // CONTRACT-P6 4.3: THE PID THE SERVER STATED IN ITS Welcome, which is not the same fact + // as the pid this session's launcher recorded - that one is local knowledge, this one + // CROSSED THE WIRE. Under spawn they must agree and neither may be ours; under inproc + // the server states its own pid, which IS ours, and that is the honest answer there. + std::uint32_t PeerServerPid() const { return m_peerServerPid; } + ~ClientSession(); // Builds the four segments, performs Hello/Welcome, takes the first CapsSnapshot, and @@ -443,6 +449,9 @@ namespace MobileGL::MG_Remote::Client { // Written by whichever thread first notices the hangup - the GL thread in the barrier, // or the event pump - and read by glGetGraphicsResetStatus on the GL thread. std::atomic m_deviceLost{false}; + // Welcome::serverPid, kept because §9.5's arm proof wants it and because a value that is + // only ever logged cannot be asserted on. + std::uint32_t m_peerServerPid = 0; }; // One per process in P5, because P5 serves one context, and LEAKED AT EXIT like every other diff --git a/MobileGL/MG_Remote/Server/ServerSession.cpp b/MobileGL/MG_Remote/Server/ServerSession.cpp index be5a3560..9bd84694 100644 --- a/MobileGL/MG_Remote/Server/ServerSession.cpp +++ b/MobileGL/MG_Remote/Server/ServerSession.cpp @@ -24,8 +24,27 @@ #include #include +// CONTRACT-P6 4.3: the one value in the handshake that a same-process session cannot fake. +#if defined(_WIN32) +#include +#else +#include +#endif + namespace MobileGL::MG_Remote::Server { + namespace { + // File-local on purpose: two callers, both handshake facts. A process-id helper on a + // public header invites uses that are not. + std::uint32_t SelfProcessId() { +#if defined(_WIN32) + return static_cast(::_getpid()); +#else + return static_cast(::getpid()); +#endif + } + } // namespace + // THE THREE FLAG-SPACE COLLISIONS, AS TRIPWIRES RATHER THAN AS A COMMENT. // // MGPipeCallFlags (MG_Pipe/MGPipe.h:42-54) and RingRecordFlags (Ring.h) are separate spaces @@ -531,7 +550,15 @@ namespace MobileGL::MG_Remote::Server { auto stamp = builder.CreateString(GIT_COMMIT_HASH_SHORT); auto welcome = ::MobileGL::Wire::CreateWelcome( builder, MOBILEGL_PROTOCOL_ABI_MAJOR, MOBILEGL_PROTOCOL_ABI_MINOR, - static_cast(hello->pid()), cmd, stage, reply, event, stamp, ourFingerprint); + // CONTRACT-P6 4.3: THE SERVER'S OWN PID, not an echo of the client's. + // + // This field echoed hello->pid() - which is itself hard-coded 0 - so serverPid + // was ALWAYS 0 and named nothing. §9.5's arm-proof gate wants the child pid and + // this is its natural carrier: under spawn it is the one value in the handshake + // that a same-process session cannot produce, because there the two pids are + // equal by construction. + static_cast(SelfProcessId()), + cmd, stage, reply, event, stamp, ourFingerprint); auto root = ::MobileGL::Wire::CreateCtrlEnvelope( builder, ::MobileGL::Wire::CtrlMsg::Welcome, welcome.Union()); ::MobileGL::Wire::FinishCtrlEnvelopeBuffer(builder, root); diff --git a/MobileGL/MG_Remote/Transport/Ring.cpp b/MobileGL/MG_Remote/Transport/Ring.cpp index fbf7d39c..3fd2f6a6 100644 --- a/MobileGL/MG_Remote/Transport/Ring.cpp +++ b/MobileGL/MG_Remote/Transport/Ring.cpp @@ -678,6 +678,12 @@ namespace MobileGL::MG_Remote::Transport { mix(inputs.RendererInfoCodecVersion); mix(inputs.OpCount); mix(inputs.AbiVersion); + // CONTRACT-P6 4.2: THE BUILD SYSTEM'S OWN ANSWER, mixed before the bytes and separately + // from them. `BuildStampPresent == 0` means the build could not determine a commit at + // all; that is a different claim from "the commit is the empty string", and before this + // field existed the two collapsed - so every stampless build agreed with every other, + // which is the exact agreement the stamp is there to refuse. + mix(inputs.BuildStampPresent); // A presence marker before the bytes, so that "no stamp" (nullptr) and // "an empty stamp" ("") are different inputs rather than the same // absence of bytes. diff --git a/MobileGL/MG_Remote/Transport/SessionRings.h b/MobileGL/MG_Remote/Transport/SessionRings.h index 441f600f..00af0dcc 100644 --- a/MobileGL/MG_Remote/Transport/SessionRings.h +++ b/MobileGL/MG_Remote/Transport/SessionRings.h @@ -576,10 +576,25 @@ namespace MobileGL::MG_Remote::Transport { // MOBILEGL_ABI_VERSION(major, minor): a protocol change that left every // struct the same size, which nothing above can see. std::uint32_t AbiVersion = 0; - // GIT_COMMIT_HASH_SHORT: two builds of the same sizes can still disagree - // about a FIELD ORDER, which no sizeof can see. nullptr and "" are - // distinct inputs and neither equals a real stamp. + // MOBILEGL_BUILD_STAMP_VALUE: two builds of the same sizes can still + // disagree about a FIELD ORDER, which no sizeof can see. nullptr and "" + // are distinct inputs and neither equals a real stamp. const char* BuildStamp = nullptr; + // CONTRACT-P6 4.2. WHETHER THE BUILD COULD NAME ITS COMMIT AT ALL, which + // is not the same question as whether BuildStamp is empty - and telling + // them apart is the entire fix. + // + // The old mix took GIT_COMMIT_HASH_SHORT, and a `git rev-parse` that + // FAILED yielded "" with nothing reporting it (no RESULT_VARIABLE). + // Every stampless build in the world then mixed the same "" and agreed + // with every other, which is precisely the agreement the stamp exists to + // refuse. Measured in this worktree: WSL cannot resolve the Windows path + // in its `.git` file, so every WSL build was stampless and silent. + // + // Mixed as its own field so that 0-with-"" and 1-with-"" are different + // fingerprints: a build that says "I have no stamp" must not collide + // with one whose stamp genuinely is the empty string. + std::uint32_t BuildStampPresent = 0; }; // FNV-1a over every field above, in declaration order. Never 0: that value is diff --git a/MobileGL/MG_Test/Wire/ServerSpawnTest.cpp b/MobileGL/MG_Test/Wire/ServerSpawnTest.cpp index 03a152f5..7fcefb05 100644 --- a/MobileGL/MG_Test/Wire/ServerSpawnTest.cpp +++ b/MobileGL/MG_Test/Wire/ServerSpawnTest.cpp @@ -193,6 +193,37 @@ TEST(ServerSpawnTest, AnEglControlOpCrossesToTheOtherProcessAndAnswers) { ASSERT_EQ(Server::ReapServer(session.server, 5000, &exitCode), MOBILEGL_OK); } +TEST(ServerSpawnTest, TheHandshakeCarriesTwoDifferentProcessIdsAndNotTwoZeroes) { + // CONTRACT-P6 4.3. Hello::pid was hard-coded 0 and Welcome::serverPid ECHOED it back, so the + // handshake carried 0 and 0 and named nothing at all. Both now carry real ids, which makes + // the pair the cheapest arm proof in the protocol: under spawn they MUST differ, and a + // same-process session cannot produce two different values. + // + // ServerSession records the pid it welcomed; ClientSession knows the child it launched. The + // assertion is that those two agree AND that neither is this process. + Session session; + ASSERT_TRUE(Bring("pids", &session)); + ASSERT_EQ(Handshake(session), MOBILEGL_OK); + + const std::uint32_t self = static_cast(::getpid()); + const std::uint32_t server = static_cast(session.server.pid); + EXPECT_NE(server, 0u) << "the launcher did not record a child pid"; + EXPECT_NE(server, self) << "that is this process, so there is nothing to prove"; + + // THE VALUE THAT CROSSED THE WIRE, not the one the launcher happens to remember. Welcome + // carries the server's own getpid(), so this equality says the handshake reached the process + // we started - rather than some other listener that answered on the name - and it says it + // from the server's own mouth. + EXPECT_EQ(Client::ClientSessionInstance().PeerServerPid(), server) + << "Welcome::serverPid does not name the process we launched"; + EXPECT_NE(Client::ClientSessionInstance().PeerServerPid(), self) + << "the peer stated OUR pid; serverPid is echoing the client again (4.3)"; + + Client::ClientSessionInstance().Stop(); + int exitCode = -1; + ASSERT_EQ(Server::ReapServer(session.server, 5000, &exitCode), MOBILEGL_OK); +} + TEST(ServerSpawnTest, AKilledServerLatchesDeviceLostFromTheHangupAndNotFromADeadline) { // `dl`, and exit gate S2: kill -9 the server mid-session and the client must find out FROM A // DESCRIPTOR. This is the red-once for CONTRACT-P6 D5c, whose whole content is that the diff --git a/MobileGL/MG_Test/Wire/SessionHandshakeTest.cpp b/MobileGL/MG_Test/Wire/SessionHandshakeTest.cpp index f79417be..8411b564 100644 --- a/MobileGL/MG_Test/Wire/SessionHandshakeTest.cpp +++ b/MobileGL/MG_Test/Wire/SessionHandshakeTest.cpp @@ -136,10 +136,25 @@ TEST(SessionHandshakeTest, TheAbiFingerprintChangesWhenAnyOfItsInputsDoes) { EXPECT_EQ(inputs.OpCount, static_cast(MG_Pipe::MGPWireOp::kOpCount)); EXPECT_EQ(inputs.AbiVersion, static_cast(MOBILEGL_ABI_VERSION( MOBILEGL_PROTOCOL_ABI_MAJOR, MOBILEGL_PROTOCOL_ABI_MINOR))); + // CONTRACT-P6 4.2. THE ASSERTION SPLITS WITH THE MACRO PAIR, and it has to: this used to + // demand a non-empty stamp unconditionally and was RED in every build whose git could not be + // read - a real signal, but one that told the reader nothing about WHICH of the two states + // the build was in. Now the build system states it, and both states are asserted. ASSERT_NE(inputs.BuildStamp, nullptr); - EXPECT_NE(inputs.BuildStamp[0], '\0'); + EXPECT_EQ(inputs.BuildStampPresent, static_cast(MOBILEGL_BUILD_STAMP_PRESENT)) + << "the fingerprint disagrees with the build system about whether this build has a stamp"; #if MGL_HANDSHAKE_TEST_HAS_GIT_HASH - EXPECT_STREQ(inputs.BuildStamp, GIT_COMMIT_HASH_SHORT); + EXPECT_STREQ(inputs.BuildStamp, MOBILEGL_BUILD_STAMP_VALUE); +#endif +#if MOBILEGL_BUILD_STAMP_PRESENT + EXPECT_NE(inputs.BuildStamp[0], '\0') + << "the build claims a stamp and the fingerprint mixed an empty one"; +#else + // A stampless build is a KNOWN, DECLARED state - CMake warns at configure time - but it must + // not be a silent one here either. What still has to hold is that the ABSENCE is mixed, which + // the BuildStampPresent perturbation below proves. + EXPECT_STREQ(inputs.BuildStamp, "") + << "PRESENT is 0 but a value came through; the two halves of the pair disagree"; #endif // Every input moves the answer. Each lambda changes exactly one field of a copy of the @@ -176,8 +191,19 @@ TEST(SessionHandshakeTest, TheAbiFingerprintChangesWhenAnyOfItsInputsDoes) { perturbed([](Transport::AbiFingerprintInputs& i) { i.BuildStamp = "not-this-build"; })) << "the git stamp is not mixed"; // A missing stamp is not the same as an empty one, and neither is the same as a real build. + // THE PRESENCE FLAG IS ITSELF AN INPUT (4.2), and this is the assertion that makes a + // stampless build safe to reason about: a peer that says "I could not name my commit" must + // not produce the same fingerprint as one whose commit genuinely is "". It has to move the + // answer in EITHER build state, which is why it sits outside the #if below. + EXPECT_NE(production, perturbed([](Transport::AbiFingerprintInputs& i) { ++i.BuildStampPresent; })) + << "BuildStampPresent is not mixed; 'no stamp' and 'empty stamp' are one input again"; + EXPECT_NE(production, perturbed([](Transport::AbiFingerprintInputs& i) { i.BuildStamp = nullptr; })); +#if MOBILEGL_BUILD_STAMP_PRESENT + // Vacuous without a stamp: production's value already IS "", so this perturbation would be + // the identity and the assertion would pass while proving nothing. EXPECT_NE(production, perturbed([](Transport::AbiFingerprintInputs& i) { i.BuildStamp = ""; })); +#endif EXPECT_NE(perturbed([](Transport::AbiFingerprintInputs& i) { i.BuildStamp = nullptr; }), perturbed([](Transport::AbiFingerprintInputs& i) { i.BuildStamp = ""; })) << "\"no stamp\" and \"an empty stamp\" collapsed into one input"; diff --git a/MobileGL/MG_Util/Miscellany/MGGitHash.h.in b/MobileGL/MG_Util/Miscellany/MGGitHash.h.in index 57652f62..27cc247d 100644 --- a/MobileGL/MG_Util/Miscellany/MGGitHash.h.in +++ b/MobileGL/MG_Util/Miscellany/MGGitHash.h.in @@ -2,3 +2,12 @@ #define GIT_COMMIT_HASH_SHORT "@GIT_COMMIT_HASH_SHORT@" #define GIT_COMMIT_HASH_FULL "@GIT_COMMIT_HASH_FULL@" + +// CONTRACT-P6 4.2. A SECOND PAIR, because the two above are application-visible through +// glGetString(GL_VERSION) and may not move. These are consumed by the ABI fingerprint alone. +// +// PRESENT is not "VALUE is non-empty": it is the build system's own answer to whether it could +// determine a commit at all. A build that cannot say so reports 0 and the handshake treats that +// as a distinct input rather than agreeing with every other stampless build in the world. +#define MOBILEGL_BUILD_STAMP_VALUE "@MOBILEGL_BUILD_STAMP_VALUE@" +#define MOBILEGL_BUILD_STAMP_PRESENT @MOBILEGL_BUILD_STAMP_PRESENT@