tests: OOM-injection allocator seam, closes morph#108

.github/workflows/ci.yml

@@ -255,10 +255,22 @@ jobs:
           # on a runner with no display.
           QT_QPA_PLATFORM: offscreen
         run: |
+          # morph::testkit::OomInjector (tests/oom_injector.cpp) overrides
+          # the process-wide operator new/delete to force std::bad_alloc on
+          # demand -- ASan and TSan's own runtimes already interpose
+          # operator new/delete themselves, and linking a second, competing
+          # definition fails with "multiple definition of `operator
+          # new(unsigned long)'" (confirmed in CI). Excluded by test name on
+          # exactly these two legs; every other CI leg (plain clang/gcc,
+          # Windows, ubsan, coverage) runs these tests normally.
+          EXCLUDE_ARGS=()
+          if [ "${{ matrix.preset }}" = "clang-asan" ] || [ "${{ matrix.preset }}" = "clang-tsan" ]; then
+            EXCLUDE_ARGS+=(-E "OomInjector|morph#108")
+          fi
           if [ "${{ matrix.preset }}" = "clang-coverage" ]; then
-            LLVM_PROFILE_FILE="build/clang-coverage/%p.profraw" ctest --preset clang-coverage
+            LLVM_PROFILE_FILE="build/clang-coverage/%p.profraw" ctest --preset clang-coverage "${EXCLUDE_ARGS[@]}"
           else
-            ctest --preset ${{ matrix.preset }}
+            ctest --preset ${{ matrix.preset }} "${EXCLUDE_ARGS[@]}"
           fi
 
       - name: Generate coverage report
@@ -688,14 +700,26 @@ jobs:
               exit 1
             fi
             echo "── memcheck: $suite ──"
+            # [oom-injector]/[issue108]: morph::testkit::OomInjector
+            # overrides the process-wide operator new/delete
+            # (tests/oom_injector.cpp) to force std::bad_alloc on demand --
+            # Valgrind's own memcheck instrumentation intercepts allocations
+            # at a layer this override does not reach, so the injector
+            # silently never fires under Valgrind (confirmed: even its own
+            # self-tests fail here). Skipped for this leg only; every other
+            # CI leg (plain clang/gcc, Windows) runs these tests normally.
+            EXTRA_ARGS=()
+            if [ "$suite" = "tests/morph_tests" ]; then
+              EXTRA_ARGS=("~[oom-injector]" "~[issue108]")
+            fi
             valgrind \
               --tool=memcheck \
               --leak-check=full \
               --show-leak-kinds=definite,indirect \
               --errors-for-leak-kinds=definite,indirect \
               --error-exitcode=1 \
               --suppressions=cmake/valgrind.supp \
-              "$binary" || status=1
+              "$binary" "${EXTRA_ARGS[@]}" || status=1
           done
           exit "$status"
 

tests/CMakeLists.txt

@@ -5,6 +5,8 @@ morph_generate_pinned_facts_header(
 )
 
 add_executable(morph_tests
+    oom_injector.cpp
+    test_oom_injector.cpp
     test_executor.cpp
     test_example.cpp
     test_executor_extra.cpp

tests/oom_injector.cpp

@@ -0,0 +1,170 @@
+// SPDX-License-Identifier: Apache-2.0
+
+#include "oom_injector.hpp"
+
+#include <cstdlib>
+#include <new>
+#include <stdexcept>
+
+// ASan/TSan already define their own operator new/operator new[]/operator
+// delete/operator delete[] inside their runtime (libclang_rt.{asan,tsan}_cxx.a)
+// to track allocations for their own instrumentation. Defining this file's
+// own overloads under either sanitizer fails at *link* time with "multiple
+// definition of `operator new(unsigned long)'" against that runtime archive
+// -- ctest-level test exclusion (see .github/workflows/ci.yml) cannot help
+// here, since the conflict happens before any test ever runs.
+//
+// Detected via nested #ifdef/#if blocks (not one combined boolean
+// expression): MSVC's preprocessor does not define __has_feature at all, and
+// some preprocessors do not short-circuit `defined(__has_feature) &&
+// __has_feature(...)` on a single line the way C++ code would -- they can
+// still try to macro-expand `__has_feature` as a bare identifier and choke
+// on the unmatched parenthesis that follows (`__has_feature(address_
+// sanitizer)`) once `defined(__has_feature)` alone is false. Nesting avoids
+// ever writing `__has_feature` on a line MSVC actually preprocesses.
+//
+// GCC and Clang both define __SANITIZE_ADDRESS__/__SANITIZE_THREAD__
+// whenever the corresponding -fsanitize=address/thread flag is active, which
+// covers this repo's own clang-asan/clang-tsan presets without needing
+// __has_feature at all; the __has_feature branch below only matters for a
+// Clang invocation that enables a sanitizer through some other means.
+#if defined(__SANITIZE_ADDRESS__) || defined(__SANITIZE_THREAD__)
+#define MORPH_TESTKIT_UNDER_ASAN_OR_TSAN
+#elif defined(__has_feature)
+#if __has_feature(address_sanitizer)
+#define MORPH_TESTKIT_UNDER_ASAN_OR_TSAN
+#elif __has_feature(thread_sanitizer)
+#define MORPH_TESTKIT_UNDER_ASAN_OR_TSAN
+#endif
+#endif
+
+namespace {
+
+// injectorArmed off means no injector active on this thread. While armed,
+// the next operator new call with size >= minSizeToFail throws and disarms
+// (one-shot). Plain built-in types only -- these variables' own
+// reads/writes must never themselves allocate, or arming the injector would
+// recurse into itself the moment operator new next runs. thread_local, not
+// a class member: operator new below is a free function with no `this` to
+// hang state off, and every thread needs its own independent state (see the
+// header's own @par Thread safety).
+// NOLINTBEGIN(cppcoreguidelines-avoid-non-const-global-variables) -- this
+// state *is* the seam; there is no non-global way to reach into a bare
+// operator new call from outside.
+thread_local std::size_t minSizeToFail = 0;
+thread_local bool injectorArmed = false;
+// NOLINTEND(cppcoreguidelines-avoid-non-const-global-variables)
+
+}  // namespace
+
+namespace morph::testkit {
+
+OomInjector::OomInjector(std::size_t minSize) {
+#ifdef MORPH_TESTKIT_UNDER_ASAN_OR_TSAN
+    // The operator new/delete overrides below are compiled out under this
+    // build (see the guard around them). Constructing an OomInjector here
+    // would silently do nothing, so this throws instead of letting a test
+    // misread that silence as "the injected failure never happened to
+    // trigger". In practice this never fires: every test using OomInjector
+    // is excluded from the clang-asan/clang-tsan CI legs by tag (see
+    // .github/workflows/ci.yml) -- this is a correctness backstop, not the
+    // primary mechanism.
+    (void)minSize;
+    throw std::logic_error(
+        "OomInjector: unusable under ASan/TSan (operator new/delete overrides are compiled out -- "
+        "see oom_injector.cpp)");
+#else
+    if (injectorArmed) {
+        throw std::logic_error("OomInjector: another instance is already active on this thread");
+    }
+    injectorArmed = true;
+    minSizeToFail = minSize;
+#endif
+}
+
+OomInjector::~OomInjector() {
+    injectorArmed = false;
+    minSizeToFail = 0;
+}
+
+}  // namespace morph::testkit
+
+#ifndef MORPH_TESTKIT_UNDER_ASAN_OR_TSAN
+
+namespace {
+
+// Every operator new overload below funnels through this so the trigger
+// logic lives in one place. Recursion guard: reading/writing the
+// thread_local state above touches only built-ins, never the heap, so this
+// cannot re-enter itself.
+void* allocateOrInject(std::size_t size) {
+    if (injectorArmed && size >= minSizeToFail) {
+        injectorArmed = false;  // one-shot: disarm before throwing, so the
+                                 // catch block itself (and anything else on
+                                 // this thread afterward) allocates normally.
+        throw std::bad_alloc{};
+    }
+    // NOLINTNEXTLINE(cppcoreguidelines-no-malloc, cppcoreguidelines-owning-memory) --
+    // this *is* the process-wide operator new/delete pair; std::malloc/free
+    // is what it has to be built from.
+    if (void* ptr = std::malloc(size == 0 ? 1 : size)) {
+        return ptr;
+    }
+    throw std::bad_alloc{};
+}
+
+}  // namespace
+
+void* operator new(std::size_t size) {
+    return allocateOrInject(size);
+}
+
+void* operator new[](std::size_t size) {
+    return allocateOrInject(size);
+}
+
+void* operator new(std::size_t size, const std::nothrow_t& tag) noexcept {
+    (void)tag;
+    try {
+        return allocateOrInject(size);
+    } catch (...) {
+        return nullptr;
+    }
+}
+
+void* operator new[](std::size_t size, const std::nothrow_t& tag) noexcept {
+    (void)tag;
+    try {
+        return allocateOrInject(size);
+    } catch (...) {
+        return nullptr;
+    }
+}
+
+// NOLINTBEGIN(readability-inconsistent-declaration-parameter-name,
+// cppcoreguidelines-no-malloc, cppcoreguidelines-owning-memory) -- these
+// replace the library's own global operator delete overloads; the "sized"
+// forms' declarations in <new> don't name their second parameter, and
+// std::free is what a hand-written operator delete has to call to release
+// what allocateOrInject's std::malloc above returned.
+void operator delete(void* ptr) noexcept {
+    std::free(ptr);
+}
+
+void operator delete[](void* ptr) noexcept {
+    std::free(ptr);
+}
+
+void operator delete(void* ptr, std::size_t size) noexcept {
+    (void)size;
+    std::free(ptr);
+}
+
+void operator delete[](void* ptr, std::size_t size) noexcept {
+    (void)size;
+    std::free(ptr);
+}
+// NOLINTEND(readability-inconsistent-declaration-parameter-name,
+// cppcoreguidelines-no-malloc, cppcoreguidelines-owning-memory)
+
+#endif  // !MORPH_TESTKIT_UNDER_ASAN_OR_TSAN

tests/oom_injector.hpp

@@ -0,0 +1,135 @@
+// SPDX-License-Identifier: Apache-2.0
+
+#pragma once
+#include <cstddef>
+
+namespace morph::testkit {
+
+/// @brief RAII scope that makes the next `operator new` call whose
+///        requested size is at least @c minSize throw `std::bad_alloc` on
+///        the current thread, so a test can drive a real allocation-failure
+///        path (a `catch (...)` around a `std::string`/container assignment,
+///        for example) without exhausting process memory or forking/mocking
+///        the standard allocator.
+///
+/// Test-only tooling, not part of the shipped library: `oom_injector.cpp`
+/// defines `operator new`/`operator delete` for the whole binary it links
+/// into, so this lives under `tests/` and is only ever linked into test
+/// executables, never into `morph`'s own interface library or any example
+/// app -- a real consumer's process must never get its allocator replaced
+/// by test-only code.
+///
+/// @par Incompatible with ASan, TSan, and Valgrind
+/// ASan and TSan's own runtimes already interpose `operator new`/`operator
+/// delete` themselves; linking this seam's own definitions alongside either
+/// one fails at link time with "multiple definition of `operator
+/// new(unsigned long)'" (confirmed in CI). Valgrind's memcheck intercepts
+/// allocations at a layer this override does not reach, so the injector
+/// silently never fires under it (confirmed: even its own self-tests fail
+/// there). `.github/workflows/ci.yml` excludes every test tagged
+/// `[oom-injector]`/`[issue108]` on the `clang-asan`/`clang-tsan` legs (by
+/// `ctest -E`) and the Valgrind leg (by Catch2 tag filter) for exactly this
+/// reason -- a test using this seam must carry one of those tags so it is
+/// excluded consistently on every leg where the override cannot work.
+///
+/// @par Why this exists
+/// Several `catch (...)` blocks across the codebase (see
+/// `LASTRADA-Software/morph#108`) only ever fire on `std::bad_alloc` from a
+/// real allocation failure -- there is no other way into them. That is not
+/// portably reachable from a unit test without a seam: this class overrides
+/// the process-wide `operator new`/`operator new[]` (defined once in
+/// `oom_injector.cpp`) to consult a thread-local size threshold before
+/// delegating to the real allocator.
+///
+/// @par Why a size predicate, not a raw allocation count
+/// An earlier design counted "the Nth `operator new` call after arming" —
+/// rejected before this shipped, because that count depends on exactly how
+/// many heap allocations the standard library, the test's own setup code,
+/// and even unrelated `std::string`/container growth perform first, which
+/// varies across STL implementations and can shift with an unrelated
+/// compiler/library upgrade. A test would silently start failing the wrong
+/// allocation (or none at all) the moment that count changed, without
+/// necessarily failing loudly. A size predicate instead targets what the
+/// allocation actually *is*: e.g. "the next allocation of at least 64
+/// bytes" reliably matches a specific long string's heap buffer (a
+/// short-string-optimized `std::string` never calls `operator new` at all),
+/// so a test picks a source string long enough to defeat SSO on every
+/// supported standard library and lets the predicate find that allocation
+/// regardless of how many smaller, unrelated allocations happen around it.
+///
+/// @par Why there is no occurrence-count parameter
+/// A call path that copies the same long value more than once before
+/// reaching the statement a test actually wants to fail (e.g. a setup copy
+/// taken before a dispatch call, followed by another copy inside that same
+/// call stack when a callback fires inline) cannot be disambiguated by size
+/// alone. An earlier revision added a "skip the first K-1 matches"
+/// occurrence count to handle exactly that case — and it was removed after
+/// a value tuned against one STL's allocator (MSVC) failed to reproduce on
+/// two others (libstdc++, libc++) in CI: the number of same-shaped copies
+/// before a target line is itself an implementation detail, not something
+/// this seam can portably parameterise around. A call path with that shape
+/// is not a good fit for `OomInjector` — pick a different reproduction (a
+/// path where the target copy is the *first* allocation of its size, as the
+/// out-of-frame test in `test_async_registration.cpp` does) rather than
+/// counting occurrences.
+///
+/// @par Usage
+/// ```cpp
+/// {
+///     // "expected" must be long enough that copying it heap-allocates on
+///     // every supported STL (well past libstdc++/libc++/MSVC's ~15-23
+///     // byte SSO buffers).
+///     std::string expected(128, 'x');
+///     morph::testkit::OomInjector inject{/*minSize=*/64};
+///     REQUIRE_THROWS_AS(codeThatCopiesExpectedSomewhere(expected), std::bad_alloc);
+/// }  // scope ends here: later allocations succeed normally again, even if
+///    // the matching allocation never actually happened.
+/// ```
+/// The first `operator new` call after construction whose `size >= minSize`
+/// throws `std::bad_alloc` instead of allocating; every smaller allocation
+/// before it (and every allocation, of any size, after it fires — including
+/// inside the `catch` block itself) allocates normally: this is a one-shot
+/// trigger, not a standing size filter. Only one `OomInjector` may be active
+/// per thread at a time — nesting two throws `std::logic_error` from the
+/// inner constructor, since both would otherwise race over the same
+/// thread-local state.
+///
+/// @par Thread safety
+/// The injection state is `thread_local`: an `OomInjector` constructed on
+/// one thread only affects `operator new` calls made from that same thread.
+/// Other threads' allocations are never affected, so a test that spins up
+/// worker threads around the code under test must construct the injector on
+/// whichever thread actually performs the allocation it wants to fail.
+///
+/// @par Why this isn't a `morph::core::FileIoOps`-shaped injectable parameter
+/// `FileIoOps` works because `FileActionLog`/`FileOfflineQueue` call the raw
+/// syscall directly and can take an injectable strategy object in their own
+/// constructor. The `catch (...)` blocks this class targets guard a plain
+/// `std::string` copy-assignment with no such seam to thread through -- the
+/// allocation happens inside `std::string::operator=` itself, several layers
+/// below any parameter `Bridge` could plausibly accept. Overriding the
+/// global allocator is the standard technique for exactly this shape of gap.
+class OomInjector {
+  public:
+    /// @brief Arms the injector so the first future `operator new` call on
+    ///        this thread whose requested size is at least @p minSize
+    ///        throws `std::bad_alloc` instead of allocating. Smaller
+    ///        allocations before it succeed normally and do not count
+    ///        against the trigger.
+    /// @param minSize Minimum allocation size, in bytes, that should fail.
+    /// @throws std::logic_error if another `OomInjector` is already active
+    ///         on this thread.
+    explicit OomInjector(std::size_t minSize);
+
+    /// @brief Disarms the injector. Allocations on this thread after this
+    ///        point succeed normally again, whether or not the matching
+    ///        allocation ever actually happened.
+    ~OomInjector();
+
+    OomInjector(const OomInjector&) = delete;
+    OomInjector& operator=(const OomInjector&) = delete;
+    OomInjector(OomInjector&&) = delete;
+    OomInjector& operator=(OomInjector&&) = delete;
+};
+
+}  // namespace morph::testkit