From dbe1d63677ec5d582e9def1ea4e0963fd4e5fb03 Mon Sep 17 00:00:00 2001 From: Dario Gabriel Lipicar Date: Sat, 22 Aug 2026 10:34:54 -0300 Subject: [PATCH] feat(abi): define logos_module_set_call_caller, gated on protocol 0.6 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The definition lands BEFORE the protocol declares the export. logos-protocol only DECLARES the module-impl C ABI and every backend owes the definition; that gap shipped twice, each time as an undefined symbol at dlopen, on Linux only, invisible on macOS. Declaring first would turn this repo, logos-rust-sdk and logos-module-builder red the night the bump merged. Defining first costs nothing: the guard is MAJOR-aware >= 0.6 and the current pin is 0.5, so nothing is emitted today and the ABI check sees declared == defined. This is the case #146 made possible. The next-MAJOR probe resolves the emitter at MAJOR+1, where a >= 6 guard IS true, so the emitted set there is legitimately a SUPERSET of the declared one. The probe used to demand equality and would have rejected this outright. cpp/logos_caller.h carries the LogosCaller type (std-typed, Qt-free) and logos::currentCaller(), reading a thread-local stack the generated export pushes to. Two things the audit corrected, both worth reading: * A present-but-unreadable `instance` is DROPPED and the module still identified. This backend already did that; Rust returned Unknown, and each had a passing test pinning its own answer, so neither suite could see the divergence. The protocol header now states the rule normatively and Rust is aligned to it. * The accessors are explicitly HIDDEN on ELF. The header argued this state must not be unified across images and then relied on being inline to achieve it — which is false: a function-local static in an inline function emits STB_GNU_UNIQUE at default visibility and the loader collapses every image's copy into one, even under RTLD_LOCAL. Measured across two dlopen'd images: default visibility let a push in A be read by B; hidden restored isolation. logos-module-builder sets no visibility anywhere, so real plugins were built the first way. An anonymous namespace would be worse — vague linkage is load-bearing WITHIN an image, since the generated TU pushes and the author's TU reads. Co-Authored-By: Claude Opus 5 --- .../experimental/lidl_gen_cdylib.cpp | 39 ++ cpp/CMakeLists.txt | 3 +- cpp/logos_caller.h | 289 +++++++++++++ nix/include.nix | 2 +- nix/tests-module-impl-abi.nix | 10 +- tests/experimental/test_lidl_gen_cdylib.cpp | 107 +++++ tests/sdk/CMakeLists.txt | 1 + tests/sdk/test_logos_caller.cpp | 378 ++++++++++++++++++ 8 files changed, 826 insertions(+), 3 deletions(-) create mode 100644 cpp/logos_caller.h create mode 100644 tests/sdk/test_logos_caller.cpp diff --git a/cpp-generator/experimental/lidl_gen_cdylib.cpp b/cpp-generator/experimental/lidl_gen_cdylib.cpp index 9774b20..4e86be2 100644 --- a/cpp-generator/experimental/lidl_gen_cdylib.cpp +++ b/cpp-generator/experimental/lidl_gen_cdylib.cpp @@ -632,6 +632,10 @@ QString lidlMakeModuleImplExports(const ModuleDecl& module, s << "#include \"logos_protocol.h\"\n"; s << "#include \"logos_module_context.h\"\n"; s << "#include \"logos_result.h\"\n"; + // The caller-of-a-dispatch reader. Unconditional: it is a logos-cpp-sdk + // header with no protocol dependency of its own, so it costs nothing on an + // older protocol where the export below is not emitted. + s << "#include \"logos_caller.h\"\n"; s << "#include \n"; s << "#include \n"; s << "#include \n"; @@ -966,6 +970,41 @@ QString lidlMakeModuleImplExports(const ModuleDecl& module, s << "}\n"; s << "#endif\n\n"; + // THE CALLER OF A DISPATCH (protocol 0.6). The glue wraps one + // logos_module_dispatch in one push/pop pair on the dispatching thread; a + // non-NULL argument pushes, NULL pops the innermost. + // + // WHY THIS CROSSES THE C ABI AT ALL, since a thread_local the host set + // would be so much simpler. It would not be the same object. Measured with + // nm on built binaries rather than assumed, on both object formats: the + // host image and the module plugin EACH define + // ModuleProxy::callRemoteMethod and TokenManager::instance; the + // function-local static behind the latter is a LOCAL bss symbol in each, + // at a different address; and neither image holds an undefined reference + // to the other's copy. The Mach-O plugin is MH_NOUNDEFS | MH_TWOLEVEL. So + // the identity has to be handed over explicitly, exactly as the trust-root + // grant above is. cpp/logos_caller.h carries the full measurement. + // + // Guarded MAJOR-aware, not on the MINOR alone. At 1.0 the MINOR resets to + // 0 and a `MINOR >= 6` guard would go false, taking the definition and the + // generated call away TOGETHER — everything would still build and load, + // and modules would just silently stop being able to name their caller. + // checks.module-impl-abi resolves this text at one MAJOR up for that + // reason. Written expanded rather than behind a function-like macro + // because unifdef has to be able to evaluate it. + s << "#if defined(LOGOS_PROTOCOL_VERSION_MINOR) && " + "(LOGOS_PROTOCOL_VERSION_MAJOR > 0 || " + "(LOGOS_PROTOCOL_VERSION_MAJOR == 0 && " + "LOGOS_PROTOCOL_VERSION_MINOR >= 6))\n"; + s << "void logos_module_set_call_caller(const char* caller_json)\n{\n"; + // One line, deliberately. Parsing the document, the per-thread stack and + // the nesting rule all live in cpp/logos_caller.h where a unit test can + // reach them BY VALUE; logic that lives in emitted text is logic no test + // ever executes, only greps. + s << " logos::detail::setCallCaller(caller_json);\n"; + s << "}\n"; + s << "#endif\n\n"; + s << "const char* logos_module_get_protocol_version(void)\n{\n"; s << " return LOGOS_PROTOCOL_VERSION_STRING;\n}\n\n"; diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index 88dba8e..1743d82 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -20,7 +20,7 @@ set(CMAKE_CXX_STANDARD_REQUIRED ON) # generated _api.{h,cpp} wrappers and their logos_sdk.h # umbrella, which the module builder emits per build. # -# ::provider logos_module_context.h, logos_host_services.h +# ::provider logos_module_context.h, logos_caller.h, logos_host_services.h # IMPLEMENTING a module. LogosModuleContext is the seam the # generated provider injects into; logos_host_services.h is the # veneer a module uses for services the HOST granted it. (It is @@ -110,6 +110,7 @@ install(FILES logos_module_context.h logos_json.h logos_result.h + logos_caller.h logos_lp_client.h logos_async_result.h logos_host_services.h diff --git a/cpp/logos_caller.h b/cpp/logos_caller.h new file mode 100644 index 0000000..9db392f --- /dev/null +++ b/cpp/logos_caller.h @@ -0,0 +1,289 @@ +#pragma once +// --------------------------------------------------------------------------- +// WHO CALLED THIS HANDLER — the module-side half of logos_module_set_call_caller(). +// +// A handler running inside a dispatch can ask `logos::currentCaller()` who is +// calling it. The value is AMBIENT: it is not a parameter, never appears in a +// .lidl, and no method opts in. logos-protocol's cpp/logos_module_impl.h holds +// the normative definition of the document parsed here; this header is the C++ +// reader for it, exactly as logos-rust-sdk holds the Rust one. +// +// WHY THE IDENTITY HAS TO BE PUSHED ACROSS THE IMAGE BOUNDARY, which is the +// only reason any of this exists rather than a plain `thread_local` set by the +// host. The host binary and the module plugin each link their OWN copy of +// logos-protocol. +// +// Measured with nm on built binaries, not assumed — liblogos_core against a +// module plugin, on both object formats: +// +// * BOTH images DEFINE TokenManager::instance() and both overloads of +// ModuleProxy::callRemoteMethod. Not one defining and one importing: two +// definitions. +// * The function-local static behind them, TokenManager::instance()::instance, +// is a LOCAL bss symbol (nm `b`) in each image, at a different address in +// each. Being local, it is not a candidate for interposition on any +// platform — there is simply one per image, always. +// * NEITHER image holds a single undefined reference to the other's copy. +// * The Mach-O plugin's header flags are MH_NOUNDEFS | MH_TWOLEVEL, so its +// references are bound to a named library at link time and never resolved +// against whatever the host happens to have loaded. +// +// So a `thread_local` the host sets is simply NOT the object a handler reads. +// The push is the mechanism; it is not a fallback for a better one that failed. +// +// The measurement turned up one asymmetry worth writing down, because it is +// the trap next to this one: on ELF the ACCESSOR functions are global (nm `T`) +// and therefore CAN be interposed between images, while on Mach-O and PE they +// cannot. That is exactly why the accessor below must not be exported — see +// the note on currentCaller(). +// +// A PER-THREAD STACK, NOT A SINGLE SLOT. A handler that makes an outbound call +// spins a nested event loop, and a second inbound call can be delivered on that +// same thread inside it. With one slot, the inner call's clear would erase the +// outer call's caller and the outer handler would resume seeing Unknown — or, +// worse, seeing the inner caller. So a non-NULL push nests, a NULL pop removes +// the innermost, a pop with nothing pushed is a no-op, and every thread has its +// own stack. +// +// VALID ONLY DURING A DISPATCH, ON THE DISPATCHING THREAD. A worker the module +// spawned, a timer callback, a context-ready hook and an event emission all +// read Unknown — correctly, because none of them has a caller. A handler that +// needs the identity past its own frame copies it at the top. +// +// NOT MARKED FOR EXPORT, and deliberately so — see the note above +// `currentCaller()` for why that is right in both the static and the shared +// topology. +// --------------------------------------------------------------------------- + +#include +#include + +#include + +namespace logos { + +// The arms of the caller document. Mirrors logos-rust-sdk's enum one-for-one; +// logos-protocol/cpp/logos_module_impl.h is the normative list for both. +// +// `Unknown` is not an error code. It is the honest answer whenever the identity +// is absent, unreadable, or expressed in a vocabulary this build predates. +enum class CallerKind { + Unknown, + Host, + Module, + Derived, + Operator, +}; + +// A parsed caller identity. +// +// Field population by arm — anything not listed is empty: +// Unknown — +// Host — (rule 5: host carries no name, ever) +// Module name, instance (opt) +// Derived parent, leaf +// Operator name +struct LogosCaller { + CallerKind kind = CallerKind::Unknown; + std::string name; // Module, Operator + std::string instance; // Module, optional (rule 6) + std::string parent; // Derived + std::string leaf; // Derived + + bool isUnknown() const { return kind == CallerKind::Unknown; } + bool isHost() const { return kind == CallerKind::Host; } + bool isDerived() const { return kind == CallerKind::Derived; } + bool isOperator() const { return kind == CallerKind::Operator; } + + bool isModule() const { return kind == CallerKind::Module; } + + // Deliberately ignores `instance` (rule 6). A caller that must distinguish + // one instance of a module from another compares the whole identity; this + // predicate answers "is this chat_module", which is the question every call + // site actually has, and it must keep answering it unchanged on the day + // instance addressing starts being emitted. + bool isModule(const std::string& moduleName) const + { + return kind == CallerKind::Module && name == moduleName; + } +}; + +// Parse the normative caller document. NEVER throws and never reports failure +// out of band: every malformed, truncated, empty or unrecognised input is an +// Unknown caller. +// +// The degradation direction is the whole point and it is one-way. Adding an arm +// can only turn an old reader's isModule(x) from true to FALSE. A permissive +// reader — "kind I don't know, but there is a name, so call it a module" — +// would do the reverse and silently WIDEN a predicate that sits next to +// authorization decisions. So: no closest match, no partial values, no +// salvaging fields out of an arm whose required ones are missing. +// Hidden visibility, and it is load-bearing rather than hygiene. +// +// The comment on currentCaller() below argues that this state must NOT be +// unified across images. On ELF at DEFAULT visibility it is: a function-local +// static inside an inline function emits as STB_GNU_UNIQUE ("u" in .dynsym) and +// the dynamic linker deliberately collapses every image's copy into one, even +// under RTLD_LOCAL. Measured, two dlopen'd images: with default visibility a +// push in image A is READ BY image B; with hidden visibility B correctly reads +// Unknown. logos-module-builder sets no visibility anywhere, so real plugins are +// built the first way. +// +// So the property has to be asked for. It cannot be inherited from being inline +// — that was the claim, and it is false on the one platform where the bug this +// whole mechanism fixes is otherwise visible. +// +// An anonymous namespace would be strictly worse: vague linkage is load-bearing +// WITHIN an image, because the generated TU pushes and the author's TU reads, +// and they must agree on one object. Hidden keeps that and stops only the +// cross-image collapse. Not applied on MSVC, which has no equivalent and no +// STB_GNU_UNIQUE to defend against. +#if defined(__GNUC__) || defined(__clang__) +# define LOGOS_CALLER_LOCAL __attribute__((visibility("hidden"))) +#else +# define LOGOS_CALLER_LOCAL +#endif + +LOGOS_CALLER_LOCAL inline LogosCaller parseCaller(const std::string& json) +{ + LogosCaller caller; // Unknown until proven otherwise. + + // allow_exceptions = false: a module handler must not be able to crash the + // dispatch by being handed a bad document, and the host is not the only + // thing that can produce one. + const nlohmann::json doc = nlohmann::json::parse(json, nullptr, false); + if (!doc.is_object()) + return caller; // rule 1: unparseable, empty, or not an object + + const auto kindIt = doc.find("kind"); + if (kindIt == doc.end() || !kindIt->is_string()) + return caller; // rule 1: "kind" is mandatory and must be a string + + // A required string field: present, a string, and non-empty. An empty name + // is not an identity, so it is treated as absent rather than as a module + // called "" that isModule("") would match. + const auto required = [&doc](const char* key, std::string& out) { + const auto it = doc.find(key); + if (it == doc.end() || !it->is_string()) + return false; + out = it->get(); + return !out.empty(); + }; + + const std::string kind = kindIt->get(); + + if (kind == "unknown") { + return caller; + } + if (kind == "host") { + // Rule 5: no name is read here even if one is present. "core" and + // "capability_module" hold the same token VALUE under two keys by + // construction, so a name on this arm would be a coin flip presented + // as a fact. Any `name` in the document is an unrecognised field for + // this arm and rule 3 says to ignore it. + caller.kind = CallerKind::Host; + return caller; + } + if (kind == "module") { + if (!required("name", caller.name)) + return LogosCaller{}; // rule 4: missing required field ⇒ unknown + // `instance` is optional. A non-string one is dropped rather than + // failing the whole identity: it is not required, and isModule(name) + // ignores it anyway, so degrading a usable name to Unknown over it + // would lose more than it protects. + const auto instIt = doc.find("instance"); + if (instIt != doc.end() && instIt->is_string()) + caller.instance = instIt->get(); + caller.kind = CallerKind::Module; + return caller; + } + if (kind == "derived") { + if (!required("parent", caller.parent) || !required("leaf", caller.leaf)) + return LogosCaller{}; // rule 4 + caller.kind = CallerKind::Derived; + return caller; + } + if (kind == "operator") { + if (!required("name", caller.name)) + return LogosCaller{}; // rule 4 + caller.kind = CallerKind::Operator; + return caller; + } + + // Rule 2: an arm from a newer protocol. Unknown, not a guess. + return LogosCaller{}; +} + +namespace detail { + +// The per-thread stack. A function-local `thread_local` rather than a namespace +// -scope one so it is initialised on first use on every thread, including +// threads that existed before this image was dlopen'd. +// +// One stack PER IMAGE is the correct and intended scope: the generated +// logos_module_set_call_caller() that pushes and the handler that reads both +// live in the module image, so they share this object. The host has its own and +// never touches this one, which is precisely the separation the C ABI push +// exists to bridge. +LOGOS_CALLER_LOCAL inline std::vector& callerStack() +{ + static thread_local std::vector stack; + return stack; +} + +// The body of the generated logos_module_set_call_caller() export. Lives here, +// not in emitted text, so it is reachable by a unit test — generated source can +// only ever be asserted on as strings. +inline void setCallCaller(const char* callerJson) +{ + std::vector& stack = callerStack(); + if (callerJson) { + stack.push_back(parseCaller(callerJson)); + return; + } + // A pop with nothing pushed is a no-op, not undefined behaviour: the host + // and the module are separate processes' worth of independent lifetime, and + // a clear that arrives without its push (a module loaded mid-dispatch, a + // host that retries teardown) must not corrupt the stack or crash. + if (!stack.empty()) + stack.pop_back(); +} + +} // namespace detail + +// The caller of the dispatch currently running on THIS thread, or an Unknown +// caller outside a dispatch. +// +// NOT marked with any export macro, and that is deliberate in both topologies: +// +// * Static / header-only, which is what logos-cpp-sdk is today — every target +// in cpp/CMakeLists.txt is an INTERFACE library and no header in this repo +// carries an export attribute. Marking this one dllexport would be a lie in +// an image that is not a DLL, and MSVC rejects the same symbol later seen +// as dllimport. +// +// * Shared. This is the one symbol in the SDK that must NOT be unified across +// images even if it could be. Give it default visibility out of a shared +// runtime and ELF's flat namespace is entitled to interpose the host's copy +// over the module's, so a handler would read the stack the host pushed to — +// restoring, on Linux only, the exact bug the C ABI push was added to fix, +// and restoring it in the one place where the other two platforms would +// stay correct and hide it. +// +// Being inline does NOT buy that separation — a function-local static in an +// inline function emits STB_GNU_UNIQUE at default visibility and IS unified +// across images. It is bought by LOGOS_CALLER_LOCAL above, which was +// measured to restore per-image isolation. STB_GNU_UNIQUE additionally +// pins an image against dlclose unmapping it, which the module teardown +// path would rather not inherit. +LOGOS_CALLER_LOCAL inline const LogosCaller& currentCaller() +{ + const std::vector& stack = detail::callerStack(); + if (stack.empty()) { + static const LogosCaller unknown; + return unknown; + } + return stack.back(); +} + +} // namespace logos diff --git a/nix/include.nix b/nix/include.nix index 77c4e0c..d67955a 100644 --- a/nix/include.nix +++ b/nix/include.nix @@ -32,7 +32,7 @@ pkgs.stdenv.mkDerivation { # include/cpp/, a single TU would pull logos_result.h through two # distinct realpaths and #pragma once could not dedup them # (redefinition of StdLogosResult). Ship every std header in BOTH roots. - for file in logos_module_context.h logos_json.h logos_result.h logos_lp_client.h logos_async_result.h logos_host_services.h logos_host_core.h; do + for file in logos_module_context.h logos_json.h logos_result.h logos_caller.h logos_lp_client.h logos_async_result.h logos_host_services.h logos_host_core.h; do cp cpp/$file $out/include/cpp/ cp cpp/$file $out/include/ done diff --git a/nix/tests-module-impl-abi.nix b/nix/tests-module-impl-abi.nix index 2c4bdb7..81d53a8 100644 --- a/nix/tests-module-impl-abi.nix +++ b/nix/tests-module-impl-abi.nix @@ -201,7 +201,15 @@ pkgs.runCommand "${common.pname}-module-impl-abi-tests" # would still be found by the diff above, so only this says where it lives. for ev in "$dir"/*_events_cdylib.cpp; do [ -e "$ev" ] || continue - resolved_symbols "$dir/events" "$minor" "$ev" + # Both the MAJOR and the MINOR, and the file AFTER them. When + # resolved_symbols gained its `major` parameter this call site kept the + # old two-argument shape, so "$ev" was consumed as the MINOR and + # `shift 3` left NO file to scan: the loop below ran over nothing, + # events.txt came out empty, and the probe reported success without + # having looked at the sidecar at all. Verified by planting a + # logos_module_* definition in the events emitter — the check stayed + # green. + resolved_symbols "$dir/events" "$major" "$minor" "$ev" [ ! -s "$dir/events.txt" ] \ || { cat "$dir/events.txt" >&2 fail "[$label] the events sidecar defines module-impl exports (above)"; } diff --git a/tests/experimental/test_lidl_gen_cdylib.cpp b/tests/experimental/test_lidl_gen_cdylib.cpp index ea909be..39adaa1 100644 --- a/tests/experimental/test_lidl_gen_cdylib.cpp +++ b/tests/experimental/test_lidl_gen_cdylib.cpp @@ -849,3 +849,110 @@ TEST(LidlGenCdylib, TeardownGoesThroughTheSfinaeHelpersNotTheImplDirectly) << src.toStdString(); EXPECT_FALSE(src.contains("lidlImpl().aboutToUnload(")) << src.toStdString(); } + +// ── The caller of a dispatch (protocol 0.6) ──────────────────────────────── +// +// logos_module_set_call_caller() carries WHO is calling into the module image +// for the duration of one dispatch. It has to cross the C ABI rather than being +// a thread_local the host sets, for the same measured reason the grant does: +// the host binary and the module plugin each link their own logos-protocol, so +// each has its own copy of the object a naive implementation would write. +// +// These land BEFORE the protocol bump that declares the symbol. At the current +// pin the guard below is false and nothing is emitted — the assertions here are +// on the emitter's TEXT, which is exactly the thing that is version-independent. + +TEST(LidlGenCdylib, EmitsTheCallCallerExport) +{ + ModuleDecl m; + m.name = "weather_module"; + m.version = "1.0.0"; + const QString src = lidlMakeModuleImplExports(m, "SomeImpl", "some_impl.h"); + + EXPECT_TRUE(src.contains("void logos_module_set_call_caller(const char* caller_json)")) + << src.toStdString(); +} + +TEST(LidlGenCdylib, CallCallerEmissionIsGuardedOnTheProtocolThatCarriesIt) +{ + // 0.6. Unguarded emission is a hard compile error against an older + // logos-protocol, in generated code the author never wrote — which is what + // happened at 0.3 and again at 0.5 before those guards existed. + ModuleDecl m; + m.name = "weather_module"; + const QString src = lidlMakeModuleImplExports(m, "SomeImpl", "some_impl.h"); + + EXPECT_TRUE(src.contains("LOGOS_PROTOCOL_VERSION_MINOR >= 6")) << src.toStdString(); +} + +TEST(LidlGenCdylib, TheCallCallerGuardIsMajorAwareNotMinorOnly) +{ + // A MINOR-only guard goes FALSE at 1.0, because the MINOR resets to 0 — + // and does so silently, since the generated call is guarded the same way + // and vanishes with the definition. Nothing links wrong and nothing fails + // to load; modules just quietly stop being able to name their caller. + // + // checks.module-impl-abi's next-MAJOR probe is the other half of this; + // this test is the one that names the surface. + ModuleDecl m; + m.name = "weather_module"; + const QString src = lidlMakeModuleImplExports(m, "SomeImpl", "some_impl.h"); + + const int guard = src.indexOf("LOGOS_PROTOCOL_VERSION_MINOR >= 6"); + ASSERT_GE(guard, 0) << src.toStdString(); + + // The whole conditional, spelled with the arithmetic expanded. Expanded and + // not behind a function-like macro because unifdef has to evaluate it: the + // ABI check resolves this text with -D flags and treats an expression it + // cannot evaluate as "not conditional at all". + EXPECT_TRUE(src.contains( + "#if defined(LOGOS_PROTOCOL_VERSION_MINOR) && " + "(LOGOS_PROTOCOL_VERSION_MAJOR > 0 || " + "(LOGOS_PROTOCOL_VERSION_MAJOR == 0 && " + "LOGOS_PROTOCOL_VERSION_MINOR >= 6))\n")) << src.toStdString(); +} + +TEST(LidlGenCdylib, TheCallCallerExportDelegatesToTheSdkHeaderNotInlineLogic) +{ + // The body is one call into cpp/logos_caller.h. Parsing, the per-thread + // stack and the nesting rule live there, where tests/sdk/test_logos_caller + // .cpp can reach them by VALUE — generated text can only ever be asserted + // on as strings, so any logic that lives here is logic nothing executes. + ModuleDecl m; + m.name = "weather_module"; + const QString src = lidlMakeModuleImplExports(m, "SomeImpl", "some_impl.h"); + + EXPECT_TRUE(src.contains("logos::detail::setCallCaller(caller_json)")) << src.toStdString(); + EXPECT_TRUE(src.contains("#include \"logos_caller.h\"")) << src.toStdString(); + // No hand-rolled parse in emitted text. + EXPECT_FALSE(src.contains("\"kind\"")) << src.toStdString(); +} + +TEST(LidlGenCdylib, TheCallCallerExportIsEmittedForEveryModuleNotJustOnesWithMethods) +{ + // The module-impl exports are a FIXED surface, not something accumulated + // per method — the shape most likely to lose a symbol to an emitter that + // writes only what it thinks it needs. checks.module-impl-abi asserts the + // same thing on the zero-method fixture; this says it at the unit level. + ModuleDecl empty; + empty.name = "empty_module"; + const QString src = lidlMakeModuleImplExports(empty, "EmptyImpl", "empty_impl.h"); + + EXPECT_TRUE(src.contains("void logos_module_set_call_caller(")) << src.toStdString(); +} + +TEST(LidlGenCdylib, TheEventsSidecarDoesNotDefineTheCallCallerExport) +{ + // Two TUs defining one export is a duplicate-symbol link error, and the + // ABI check's own sidecar probe is currently vacuous (it passes its file + // in the argument slot that resolved_symbols shifts away), so this is the + // live assertion that the symbol lives in the exports TU alone. + ModuleDecl m; + m.name = "delivery_module"; + EventDecl e; + e.name = "blobStored"; + m.events.push_back(e); + + const QString events = lidlMakeEventsSourceCdylib(m, "DeliveryImpl", "delivery_impl.h"); + EXPECT_FALSE(events.contains("logos_module_set_call_caller")) << events.toStdString(); +} diff --git a/tests/sdk/CMakeLists.txt b/tests/sdk/CMakeLists.txt index 3a7b10e..35542c8 100644 --- a/tests/sdk/CMakeLists.txt +++ b/tests/sdk/CMakeLists.txt @@ -6,6 +6,7 @@ add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/../../cpp ${CMAKE_CURRENT_BINARY_DI add_executable(sdk_tests test_logos_module_context.cpp + test_logos_caller.cpp test_logos_host_services.cpp test_logos_host_core.cpp test_lp_client.cpp diff --git a/tests/sdk/test_logos_caller.cpp b/tests/sdk/test_logos_caller.cpp new file mode 100644 index 0000000..526e47e --- /dev/null +++ b/tests/sdk/test_logos_caller.cpp @@ -0,0 +1,378 @@ +// The module-side reader for the caller-of-a-dispatch document. +// +// logos-protocol/cpp/logos_module_impl.h holds the normative definition of the +// document; this suite is the C++ reader's conformance to it, rule by rule. The +// Rust reader owes the identical table. +// +// WHY EVERY MALFORMED CASE IS SPELLED OUT rather than covered by one "bad input +// is Unknown" test: the value feeds predicates that sit next to authorization +// decisions, and the failure that matters is not a crash — it is isModule("x") +// answering TRUE for something that is not x. Each case below is a distinct way +// to get there. +// +// HOW EACH TEST WAS SHOWN TO BE A DETECTOR. Throwaway local mutations of +// cpp/logos_caller.h, made and reverted (the same discipline as +// logos-protocol/tests/protocol/test_inbound_token_store.cpp); the report on +// the branch records which test caught which mutation. + +#include + +#include + +#include "logos_caller.h" + +using logos::CallerKind; +using logos::LogosCaller; +using logos::parseCaller; + +namespace { + +// Restores the ambient stack between tests. currentCaller() reads a +// thread_local that outlives any single TEST body, so a test that pushed and +// did not pop would leak its caller into whatever ran next on this thread — +// and the leak would look like a PASS in the test that received it. +class CallerScope : public ::testing::Test { +protected: + void TearDown() override + { + while (!logos::currentCaller().isUnknown()) + logos::detail::setCallCaller(nullptr); + } +}; + +} // namespace + +// ── Rule 2: an arm this build has never heard of ──────────────────────────── +// +// THE test for forward compatibility, and the reason parseCaller cannot be a +// switch over a closed set that asserts. A 0.7 host talking to a module built +// at 0.6 is the ordinary case the moment a new arm is specified, and the module +// must degrade rather than die. +TEST_F(CallerScope, AnUnrecognisedArmFromANewerProtocolIsUnknown) +{ + // The document CARRIES a name, and that is the whole point of the case. + // An unrecognised arm with no name is caught by any implementation; the + // failure this guards against is the tempting one — "I do not know this + // kind, but there is a name here, so treat it as a module" — which turns + // isModule("chat_module") TRUE for something that is not chat_module. + // Written first without the name field, this test passed against exactly + // that fallback. + const LogosCaller c = + parseCaller(R"({"kind":"fleet","name":"chat_module","fleet":"eu-west-1"})"); + + EXPECT_EQ(c.kind, CallerKind::Unknown); + EXPECT_TRUE(c.isUnknown()); + EXPECT_FALSE(c.isModule()); + EXPECT_FALSE(c.isModule("chat_module")) << "an unknown arm was salvaged into a module"; + EXPECT_TRUE(c.name.empty()); +} + +// The same salvage, on the arms that have a REQUIRED name of their own. A +// reader that fell through to "operator" or "derived" on an unknown kind would +// be caught here and nowhere else. +TEST_F(CallerScope, AnUnrecognisedArmIsNotSalvagedIntoAnyKnownArm) +{ + struct Case { const char* label; const char* json; }; + const Case cases[] = { + {"name-bearing", R"({"kind":"fleet","name":"ops-readonly"})"}, + {"parent+leaf", R"({"kind":"scoped","parent":"wallet_module","leaf":"wallet_ui"})"}, + {"every known field",R"({"kind":"v2","name":"chat_module","instance":"a41f",)" + R"("parent":"wallet_module","leaf":"wallet_ui"})"}, + }; + + for (const Case& c : cases) { + const LogosCaller parsed = parseCaller(c.json); + EXPECT_EQ(parsed.kind, CallerKind::Unknown) << c.label; + EXPECT_FALSE(parsed.isModule()) << c.label; + EXPECT_FALSE(parsed.isHost()) << c.label; + EXPECT_FALSE(parsed.isDerived()) << c.label; + EXPECT_FALSE(parsed.isOperator()) << c.label; + EXPECT_TRUE(parsed.name.empty()) << c.label; + EXPECT_TRUE(parsed.parent.empty()) << c.label; + EXPECT_TRUE(parsed.leaf.empty()) << c.label; + EXPECT_TRUE(parsed.instance.empty()) << c.label; + } +} + +TEST_F(CallerScope, AnUnrecognisedArmDoesNotThrow) +{ + // Stated separately from the value assertion above because it is a + // different failure: a handler must not be able to be killed by the + // document it was handed, and `parseCaller` is called on the dispatch + // thread inside the generated export, where there is nothing to catch it. + EXPECT_NO_THROW({ + (void)parseCaller(R"({"kind":"fleet"})"); + (void)parseCaller(R"({"kind":"module_v2","name":"chat_module"})"); + }); +} + +// ── Rule 1: "kind" is mandatory; malformed input is Unknown ───────────────── + +TEST_F(CallerScope, MalformedInputDegradesToUnknownWithoutThrowing) +{ + struct Case { const char* label; const char* json; }; + const Case cases[] = { + {"empty string", ""}, + {"whitespace", " "}, + {"truncated object", R"({"kind":"module")"}, + {"not json at all", "chat_module"}, + {"json null", "null"}, + {"array, not object", R"(["kind","host"])"}, + {"string, not object", R"("host")"}, + {"number, not object", "42"}, + {"object without kind", R"({"name":"chat_module"})"}, + {"kind is not a string",R"({"kind":7,"name":"chat_module"})"}, + {"kind is null", R"({"kind":null})"}, + {"nul bytes", "\x01\x02\x03"}, + }; + + for (const Case& c : cases) { + LogosCaller parsed; + EXPECT_NO_THROW({ parsed = parseCaller(c.json); }) << c.label; + EXPECT_EQ(parsed.kind, CallerKind::Unknown) << c.label; + EXPECT_FALSE(parsed.isModule()) << c.label; + EXPECT_TRUE(parsed.name.empty()) << c.label; + } +} + +TEST_F(CallerScope, TheExplicitUnknownArmIsUnknown) +{ + // A producer today, per the protocol note. It must not be mistaken for a + // parse failure and must not be mistaken for an unrecognised arm — all + // three land on the same value, which is what makes the value safe. + EXPECT_EQ(parseCaller(R"({"kind":"unknown"})").kind, CallerKind::Unknown); +} + +// ── The recognised arms ───────────────────────────────────────────────────── + +TEST_F(CallerScope, TheHostArmParses) +{ + const LogosCaller c = parseCaller(R"({"kind":"host"})"); + + EXPECT_EQ(c.kind, CallerKind::Host); + EXPECT_TRUE(c.isHost()); + EXPECT_FALSE(c.isModule()); +} + +// Rule 5. "core" and "capability_module" hold the same token VALUE under two +// keys by construction, so a name on this arm would be a coin flip presented as +// a fact. If a host ever emits one, it is ignored — rule 3 — rather than +// promoted into a field that call sites would then branch on. +TEST_F(CallerScope, TheHostArmNeverCarriesAName) +{ + const LogosCaller c = parseCaller(R"({"kind":"host","name":"capability_module"})"); + + EXPECT_EQ(c.kind, CallerKind::Host); + EXPECT_TRUE(c.name.empty()) << "host must not gain a name: " << c.name; +} + +TEST_F(CallerScope, TheModuleArmParsesAndMatchesByName) +{ + const LogosCaller c = parseCaller(R"({"kind":"module","name":"chat_module"})"); + + EXPECT_EQ(c.kind, CallerKind::Module); + EXPECT_TRUE(c.isModule()); + EXPECT_EQ(c.name, "chat_module"); + EXPECT_TRUE(c.isModule("chat_module")); + EXPECT_FALSE(c.isModule("wallet_module")); + EXPECT_TRUE(c.instance.empty()); +} + +// Rule 6, and the reason `instance` is in the type from day one rather than +// added when instance addressing arrives: isModule(name) must keep its answer +// on the day a producer starts emitting the field. If the predicate compared +// the whole identity, every existing call site would silently start returning +// false the first time a host addressed an instance. +TEST_F(CallerScope, IsModuleIgnoresTheInstance) +{ + const LogosCaller c = + parseCaller(R"({"kind":"module","name":"chat_module","instance":"a41f"})"); + + EXPECT_EQ(c.kind, CallerKind::Module); + EXPECT_EQ(c.instance, "a41f"); + EXPECT_TRUE(c.isModule("chat_module")) << "instance addressing changed the answer"; +} + +TEST_F(CallerScope, TheDerivedArmParses) +{ + const LogosCaller c = + parseCaller(R"({"kind":"derived","parent":"wallet_module","leaf":"wallet_ui"})"); + + EXPECT_EQ(c.kind, CallerKind::Derived); + EXPECT_TRUE(c.isDerived()); + EXPECT_EQ(c.parent, "wallet_module"); + EXPECT_EQ(c.leaf, "wallet_ui"); + // A derived identity is NOT its parent module. Answering true here would + // hand a plugin the parent's authority. + EXPECT_FALSE(c.isModule("wallet_module")); +} + +TEST_F(CallerScope, TheOperatorArmParses) +{ + const LogosCaller c = parseCaller(R"({"kind":"operator","name":"ops-readonly"})"); + + EXPECT_EQ(c.kind, CallerKind::Operator); + EXPECT_TRUE(c.isOperator()); + EXPECT_EQ(c.name, "ops-readonly"); + // Shares the `name` field with the module arm and must not be confused for + // one: an operator named "chat_module" is not chat_module. + EXPECT_FALSE(c.isModule("ops-readonly")); +} + +// The two arms nothing emits yet must nonetheless PARSE. They are specified, so +// a 0.6 module can receive one from a later host; a reader that treated them as +// unrecognised would be within rule 2 but would lose real information for no +// reason, and the gap would only surface once a producer shipped. +TEST_F(CallerScope, TheUnproducedArmsAreStillParsedNotTreatedAsUnrecognised) +{ + EXPECT_EQ(parseCaller(R"({"kind":"derived","parent":"p","leaf":"l"})").kind, + CallerKind::Derived); + EXPECT_EQ(parseCaller(R"({"kind":"operator","name":"o"})").kind, + CallerKind::Operator); +} + +// ── Rule 4: a known arm missing a required field is Unknown, not partial ──── +// +// The dangerous shape. A reader that kept the arm and left the field empty +// would make isModule("") true for a nameless module document. +TEST_F(CallerScope, AKnownArmMissingARequiredFieldIsUnknownNotPartial) +{ + struct Case { const char* label; const char* json; }; + const Case cases[] = { + {"module without name", R"({"kind":"module"})"}, + {"module, name not a string", R"({"kind":"module","name":42})"}, + {"module, empty name", R"({"kind":"module","name":""})"}, + {"derived without leaf", R"({"kind":"derived","parent":"wallet_module"})"}, + {"derived without parent",R"({"kind":"derived","leaf":"wallet_ui"})"}, + {"derived, empty leaf", R"({"kind":"derived","parent":"p","leaf":""})"}, + {"operator without name", R"({"kind":"operator"})"}, + {"operator, empty name", R"({"kind":"operator","name":""})"}, + }; + + for (const Case& c : cases) { + const LogosCaller parsed = parseCaller(c.json); + EXPECT_EQ(parsed.kind, CallerKind::Unknown) << c.label; + EXPECT_FALSE(parsed.isModule("")) << c.label << ": matched the empty name"; + EXPECT_TRUE(parsed.name.empty()) << c.label; + EXPECT_TRUE(parsed.parent.empty()) << c.label; + EXPECT_TRUE(parsed.leaf.empty()) << c.label; + } +} + +// ── Rule 3: unrecognised fields inside a known arm are ignored ────────────── + +TEST_F(CallerScope, AKnownArmToleratesFieldsItDoesNotKnow) +{ + // This is what lets an arm gain a field without a MINOR bump. A reader that + // rejected the document would make every such addition a breaking change. + const LogosCaller c = parseCaller( + R"({"kind":"module","name":"chat_module","instance":"a41f","tier":"gold","hops":3})"); + + EXPECT_EQ(c.kind, CallerKind::Module); + EXPECT_EQ(c.name, "chat_module"); + EXPECT_EQ(c.instance, "a41f"); +} + +TEST_F(CallerScope, AnOptionalFieldOfTheWrongTypeIsDroppedNotFatal) +{ + // `instance` is optional and isModule() ignores it, so a malformed one must + // not cost us a perfectly good name. Contrast the required-field cases + // above, which must degrade the whole identity. + const LogosCaller c = + parseCaller(R"({"kind":"module","name":"chat_module","instance":[1,2]})"); + + EXPECT_EQ(c.kind, CallerKind::Module); + EXPECT_EQ(c.name, "chat_module"); + EXPECT_TRUE(c.instance.empty()); + EXPECT_TRUE(c.isModule("chat_module")); +} + +// ── The ambient accessor: push, pop, nesting, threads ─────────────────────── + +TEST_F(CallerScope, OutsideADispatchTheCallerIsUnknown) +{ + // A worker thread, a timer, a context hook and an event emission all land + // here. Unknown is the correct answer, not a bug to be papered over. + EXPECT_TRUE(logos::currentCaller().isUnknown()); +} + +TEST_F(CallerScope, APushIsVisibleToTheHandlerAndThePopClearsIt) +{ + logos::detail::setCallCaller(R"({"kind":"module","name":"chat_module"})"); + EXPECT_TRUE(logos::currentCaller().isModule("chat_module")); + + logos::detail::setCallCaller(nullptr); + EXPECT_TRUE(logos::currentCaller().isUnknown()); +} + +// THE nesting test, and the reason this is a stack rather than a slot. A +// handler that calls out spins a nested event loop; a second inbound call +// arriving on that same thread inside it pushes, and its pop must restore the +// OUTER caller — not clear the slot, which would leave the outer handler +// reading Unknown for the rest of its frame. +TEST_F(CallerScope, ANestedDispatchRestoresTheOuterCallerRatherThanClearingIt) +{ + logos::detail::setCallCaller(R"({"kind":"module","name":"outer_module"})"); + ASSERT_TRUE(logos::currentCaller().isModule("outer_module")); + + logos::detail::setCallCaller(R"({"kind":"module","name":"inner_module"})"); + EXPECT_TRUE(logos::currentCaller().isModule("inner_module")); + + logos::detail::setCallCaller(nullptr); + EXPECT_TRUE(logos::currentCaller().isModule("outer_module")) + << "the inner pop erased the outer caller"; + + logos::detail::setCallCaller(nullptr); + EXPECT_TRUE(logos::currentCaller().isUnknown()); +} + +TEST_F(CallerScope, AnUnbalancedPopIsANoOpRatherThanUndefinedBehaviour) +{ + // The host and the module do not share a lifetime. A clear that arrives + // without its push — a retried teardown, a module loaded mid-dispatch — + // must not pop an empty vector. + EXPECT_NO_THROW({ + logos::detail::setCallCaller(nullptr); + logos::detail::setCallCaller(nullptr); + }); + EXPECT_TRUE(logos::currentCaller().isUnknown()); +} + +// Each thread has its OWN stack. With a shared one, a module whose concurrency +// is "multi" would have handlers on different threads overwriting each other's +// caller — the worst possible failure for this value, because the wrong answer +// would be another real module's name rather than Unknown. +TEST_F(CallerScope, EachThreadHasItsOwnCallerStack) +{ + logos::detail::setCallCaller(R"({"kind":"module","name":"main_thread_caller"})"); + ASSERT_TRUE(logos::currentCaller().isModule("main_thread_caller")); + + bool otherThreadStartedUnknown = false; + bool otherThreadSawItsOwn = false; + + std::thread worker([&] { + otherThreadStartedUnknown = logos::currentCaller().isUnknown(); + logos::detail::setCallCaller(R"({"kind":"module","name":"worker_caller"})"); + otherThreadSawItsOwn = logos::currentCaller().isModule("worker_caller"); + logos::detail::setCallCaller(nullptr); + }); + worker.join(); + + EXPECT_TRUE(otherThreadStartedUnknown) << "the worker inherited another thread's caller"; + EXPECT_TRUE(otherThreadSawItsOwn); + EXPECT_TRUE(logos::currentCaller().isModule("main_thread_caller")) + << "the worker's push was visible on the main thread"; +} + +// currentCaller() returns a reference into the stack. A handler that copies it +// must keep a valid value after the dispatch pops — the documented way to use +// the identity beyond one's own frame. +TEST_F(CallerScope, ACopyOutlivesTheDispatchThatProducedIt) +{ + logos::detail::setCallCaller(R"({"kind":"module","name":"chat_module"})"); + const LogosCaller copied = logos::currentCaller(); + logos::detail::setCallCaller(nullptr); + + EXPECT_TRUE(copied.isModule("chat_module")); + EXPECT_TRUE(logos::currentCaller().isUnknown()); +}