mirror of
https://github.com/MobileGL-Dev/MobileGL
synced 2026-11-11 12:03:24 +09:00
[MG_Remote] (P6/hs): the build stamp that was silently empty, and two asymmetries the handshake never checked
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.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5
@@ -36,8 +36,29 @@
|
||||
#include <utility>
|
||||
#include <vector>
|
||||
|
||||
// CONTRACT-P6 4.3: the handshake now carries real process ids.
|
||||
#if defined(_WIN32)
|
||||
#include <process.h>
|
||||
#else
|
||||
#include <unistd.h>
|
||||
#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<std::uint32_t>(::_getpid());
|
||||
#else
|
||||
return static_cast<std::uint32_t>(::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<unsigned>(MOBILEGL_PROTOCOL_ABI_MAJOR),
|
||||
static_cast<unsigned>(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<std::uint32_t>(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<unsigned>(welcome->abiMajor()),
|
||||
static_cast<unsigned>(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<std::uint32_t>(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<unsigned>(welcome->abiMajor()),
|
||||
static_cast<unsigned>(welcome->abiMinor()),
|
||||
theirStamp);
|
||||
}
|
||||
if (welcome->abiFingerprint() != fingerprint) {
|
||||
FatalAbiMismatch("struct shapes", fingerprint, welcome->abiFingerprint(),
|
||||
theirStamp);
|
||||
|
||||
Reference in New Issue
Block a user