From eea239d6adc0ee3307591016c503210ffd340d14 Mon Sep 17 00:00:00 2001 From: Roland Conybeare Date: Mon, 11 May 2026 09:27:24 -0400 Subject: [PATCH] xo-gc stack: coverage improvement + related tidying --- .../xo/alloc2/alloc/IAllocator_Any.hpp | 6 +- .../xo/alloc2/arena/IAllocator_DArena.hpp | 2 +- xo-alloc2/include/xo/alloc2/gc/RCollector.hpp | 13 +- xo-alloc2/src/alloc2/IAllocator_Any.cpp | 2 + xo-alloc2/src/alloc2/IAllocator_DArena.cpp | 2 + xo-alloc2/utest/CMakeLists.txt | 1 + xo-alloc2/utest/IAllocator_Any.test.cpp | 29 +++++ xo-alloc2/utest/arena.test.cpp | 114 ++++++++++++----- .../xo/gc/detail/IAllocator_DX1Collector.hpp | 2 + xo-gc/src/gc/IAllocator_DX1Collector.cpp | 2 + xo-gc/utest/CMakeLists.txt | 1 + xo-gc/utest/Collector.test.cpp | 22 ++++ xo-gc/utest/MockCollector.test.cpp | 119 ++++++++++++++++++ xo-gc/utest/X1Collector.test.cpp | 12 +- 14 files changed, 283 insertions(+), 44 deletions(-) create mode 100644 xo-alloc2/utest/IAllocator_Any.test.cpp create mode 100644 xo-gc/utest/MockCollector.test.cpp diff --git a/xo-alloc2/include/xo/alloc2/alloc/IAllocator_Any.hpp b/xo-alloc2/include/xo/alloc2/alloc/IAllocator_Any.hpp index 06e7e584..bc91fcac 100644 --- a/xo-alloc2/include/xo/alloc2/alloc/IAllocator_Any.hpp +++ b/xo-alloc2/include/xo/alloc2/alloc/IAllocator_Any.hpp @@ -35,6 +35,8 @@ namespace xo { // builtin methods typeseq _typeseq() const noexcept override { return s_typeseq; } + + // LCOV_EXCL_START void _drop(Opaque) const noexcept override { _fatal(); } // const methods @@ -58,9 +60,6 @@ namespace xo { [[noreturn]] value_type alloc(Opaque, typeseq, std::size_t) const override { _fatal(); } [[noreturn]] value_type super_alloc(Opaque, typeseq, std::size_t) const override { _fatal(); } [[noreturn]] value_type sub_alloc(Opaque, std::size_t, bool) const override { _fatal(); } -#ifdef OBSOLETE - [[noreturn]] value_type alloc_copy(Opaque, value_type) const override { _fatal(); } -#endif [[noreturn]] void clear(Opaque) const override { _fatal(); } [[noreturn]] void barrier_assign_aux(Opaque, void *, @@ -69,6 +68,7 @@ namespace xo { private: [[noreturn]] static void _fatal(); + // LCOV_EXCL_STOP public: static typeseq s_typeseq; diff --git a/xo-alloc2/include/xo/alloc2/arena/IAllocator_DArena.hpp b/xo-alloc2/include/xo/alloc2/arena/IAllocator_DArena.hpp index 5a3ad8fd..a178e5a0 100644 --- a/xo-alloc2/include/xo/alloc2/arena/IAllocator_DArena.hpp +++ b/xo-alloc2/include/xo/alloc2/arena/IAllocator_DArena.hpp @@ -78,7 +78,7 @@ namespace xo { void * parent, AGCObject * lhs_iface, void ** lhs_data, AGCObject * rhs_iface, void * rhs_data); - static void destruct_data(DArena &); + //static void destruct_data(DArena &); }; // template <> diff --git a/xo-alloc2/include/xo/alloc2/gc/RCollector.hpp b/xo-alloc2/include/xo/alloc2/gc/RCollector.hpp index b7c54d44..2818957b 100644 --- a/xo-alloc2/include/xo/alloc2/gc/RCollector.hpp +++ b/xo-alloc2/include/xo/alloc2/gc/RCollector.hpp @@ -52,7 +52,7 @@ public: void * alloc_copy_for(const T * src) noexcept { return O::iface()->alloc_copy(O::data(), (std::byte *)const_cast(src)); } - + /** convenience template for move-constructible T (this is common) **/ template T * std_move_for(T * src) noexcept { @@ -62,28 +62,28 @@ public: } return nullptr; } - + /** forward faceted object pointer in place. Defined in GCObject.hpp to avoid #include cycle **/ template void forward_inplace(obj * p_obj); - + /** another convenience template for forwarding. * Defined in RGCObject.hpp to avoid #include cycle. **/ template void forward_inplace(DRepr ** pp_repr); - + /** convenience template where pointer requires pivot **/ template requires (!std::is_same_v) void forward_pivot_inplace(obj * p_obj); - + /** add root @p p_root **/ template void add_gc_root(obj * p_root) { O::iface()->add_gc_root_poly(O::data(), (obj *)p_root); } - + /** remove root @p p_root **/ template void remove_gc_root(obj * p_root) { @@ -91,6 +91,7 @@ public: } // builtin methods + bool _has_null_vptr() const noexcept { return O::iface()->_has_null_vptr(); } typeseq _typeseq() const noexcept { return O::iface()->_typeseq(); } void _drop() const noexcept { O::iface()->_drop(O::data()); } diff --git a/xo-alloc2/src/alloc2/IAllocator_Any.cpp b/xo-alloc2/src/alloc2/IAllocator_Any.cpp index 813973c8..d8b1aabf 100644 --- a/xo-alloc2/src/alloc2/IAllocator_Any.cpp +++ b/xo-alloc2/src/alloc2/IAllocator_Any.cpp @@ -14,6 +14,7 @@ namespace xo { namespace mm { + // LCOV_EXCL_START void IAllocator_Any::_fatal() { @@ -29,6 +30,7 @@ namespace xo { std::terminate(); } + // LCOV_EXCL_STOP typeseq IAllocator_Any::s_typeseq = typeseq::id(); diff --git a/xo-alloc2/src/alloc2/IAllocator_DArena.cpp b/xo-alloc2/src/alloc2/IAllocator_DArena.cpp index 3f205a94..ea60b69b 100644 --- a/xo-alloc2/src/alloc2/IAllocator_DArena.cpp +++ b/xo-alloc2/src/alloc2/IAllocator_DArena.cpp @@ -173,11 +173,13 @@ namespace xo { *lhs_data = rhs_data; } +#ifdef OBSOLETE void IAllocator_DArena::destruct_data(DArena & s) { s.~DArena(); } +#endif } /*namespace mm*/ } /*namespace xo*/ diff --git a/xo-alloc2/utest/CMakeLists.txt b/xo-alloc2/utest/CMakeLists.txt index af2d3194..c885daa4 100644 --- a/xo-alloc2/utest/CMakeLists.txt +++ b/xo-alloc2/utest/CMakeLists.txt @@ -6,6 +6,7 @@ set(UTEST_SRCS alloc2_utest_main.cpp objectmodel.test.cpp arena.test.cpp + IAllocator_Any.test.cpp DArenaIterator.test.cpp # Collector.test.cpp # DX1CollectorIterator.test.cpp diff --git a/xo-alloc2/utest/IAllocator_Any.test.cpp b/xo-alloc2/utest/IAllocator_Any.test.cpp new file mode 100644 index 00000000..5d23c6ed --- /dev/null +++ b/xo-alloc2/utest/IAllocator_Any.test.cpp @@ -0,0 +1,29 @@ +/** @file IAllocator_Any.test.cpp + * + * @author Roland Conybeare, May 2026 + **/ + +#include +#include +#include +#include +#include + +namespace xo { + using xo::mm::AAllocator; + + namespace ut { + + TEST_CASE("IAllocator_Any", "[alloc2][death]") + { + // null allocator + obj alloc_any; + + // NOTE: tried using a fork() strategy to verify termination, + // but child process doesn't get measured by gcov + } + + } /*namespace ut*/ +} /*namespace xo*/ + +/* end IAllocator_Any.test.cpp */ diff --git a/xo-alloc2/utest/arena.test.cpp b/xo-alloc2/utest/arena.test.cpp index 0a1c3e90..95077f6b 100644 --- a/xo-alloc2/utest/arena.test.cpp +++ b/xo-alloc2/utest/arena.test.cpp @@ -97,6 +97,8 @@ namespace xo { //obj a1o{&arena}; auto a1o = with_facet::mkobj(&arena); + REQUIRE(!a1o._has_null_vptr()); + REQUIRE(a1o); REQUIRE(a1o.iface() != nullptr); REQUIRE(a1o.data() != nullptr); @@ -110,6 +112,12 @@ namespace xo { REQUIRE(a1o.size() == 0); REQUIRE(a1o.committed() == 0); REQUIRE(a1o.allocated() == 0); + + a1o._drop(); + { + REQUIRE(a1o.allocated() == 0); + REQUIRE(a1o.committed() == 0); + } } TEST_CASE("allocator-expand-1", "[alloc2][AAllocator]") @@ -122,6 +130,8 @@ namespace xo { //obj a1o{&arena}; auto a1o = with_facet::mkobj(&arena); + REQUIRE(!a1o._has_null_vptr()); + REQUIRE(a1o.available() == 0); REQUIRE(a1o.allocated() == 0); @@ -141,6 +151,11 @@ namespace xo { REQUIRE(a1o.available() == a1o.committed()); REQUIRE(a1o.allocated() == 0); + a1o._drop(); + { + REQUIRE(a1o.allocated() == 0); + REQUIRE(a1o.committed() == 0); + } } TEST_CASE("allocator-alloc-1", "[alloc2][AAllocator]") @@ -152,6 +167,8 @@ namespace xo { DArena arena = DArena::map(cfg); auto a1o = with_facet::mkobj(&arena); + REQUIRE(!a1o._has_null_vptr()); + REQUIRE(a1o.reserved() >= cfg.size_); REQUIRE(a1o.committed() == 0); REQUIRE(a1o.available() == 0); @@ -180,6 +197,12 @@ namespace xo { REQUIRE(a1o.allocated() <= a1o.committed()); REQUIRE(a1o.allocated() + a1o.available() == a1o.committed()); REQUIRE(a1o.committed() <= a1o.reserved()); + + a1o._drop(); + { + REQUIRE(a1o.allocated() == 0); + REQUIRE(a1o.committed() == 0); + } } TEST_CASE("allocator-alloc-2", "[alloc2][Allocator]") @@ -202,6 +225,8 @@ namespace xo { //obj a1o{&arena}; auto a1o = with_facet::mkobj(&arena); + REQUIRE(!a1o._has_null_vptr()); + REQUIRE(a1o.reserved() >= cfg.size_); REQUIRE(a1o.committed() == 0); REQUIRE(a1o.available() == 0); @@ -244,19 +269,26 @@ namespace xo { } a1o.clear(); + { + // allocated size got reset + REQUIRE(a1o.allocated() == 0); + // committed size unchanged + REQUIRE(a1o.committed() == committed0_z); + REQUIRE(a1o.last_error().error_ == error::ok); + REQUIRE(a1o.last_error().error_seq_ == 0); - // allocated size got reset - REQUIRE(a1o.allocated() == 0); - // committed size unchanged - REQUIRE(a1o.committed() == committed0_z); - REQUIRE(a1o.last_error().error_ == error::ok); - REQUIRE(a1o.last_error().error_seq_ == 0); + // allocator no longer contains m0 (now points to unallocated but committed memory + // (not exposed via AAllocator! + // REQUIRE(a1o.contains_allocated(m0) == false); - // allocator no longer contains m0 (now points to unallocated but committed memory - // (not exposed via AAllocator! - // REQUIRE(a1o.contains_allocated(m0) == false); + REQUIRE(a1o.contains(m0)); + } - REQUIRE(a1o.contains(m0)); + a1o._drop(); + { + REQUIRE(a1o.allocated() == 0); + REQUIRE(a1o.committed() == 0); + } } TEST_CASE("allocator-alloc-3", "[alloc2][Allocator]") @@ -279,6 +311,8 @@ namespace xo { //obj a1o{&arena}; auto a1o = with_facet::mkobj(&arena); + REQUIRE(!a1o._has_null_vptr()); + REQUIRE(a1o.reserved() >= cfg.size_); REQUIRE(a1o.committed() == 0); REQUIRE(a1o.available() == 0); @@ -301,23 +335,30 @@ namespace xo { header_type* header = (header_type*)(m0 - sizeof(header_type)); size_t pad = padding::with_padding(z0) - z0; byte * guard1 = m0 + z0 + pad; + { + REQUIRE(a1o.contains(guard0)); + REQUIRE(a1o.contains(header)); + REQUIRE(cfg.header_.size(*header) == padding::with_padding(z0)); + //REQUIRE(((*header) & cfg.header_size_mask_) == padding::with_padding(z0)); - REQUIRE(a1o.contains(guard0)); - REQUIRE(a1o.contains(header)); - REQUIRE(cfg.header_.size(*header) == padding::with_padding(z0)); - //REQUIRE(((*header) & cfg.header_size_mask_) == padding::with_padding(z0)); + REQUIRE(a1o.last_error().error_ == error::ok); + REQUIRE(a1o.last_error().error_seq_ == 0); - REQUIRE(a1o.last_error().error_ == error::ok); - REQUIRE(a1o.last_error().error_seq_ == 0); + REQUIRE(a1o.allocated() == (cfg.header_.guard_z_ + + sizeof(header_type) + + z0 + + pad + + cfg.header_.guard_z_)); + REQUIRE(a1o.allocated() <= a1o.committed()); + REQUIRE(a1o.allocated() + a1o.available() == a1o.committed()); + REQUIRE(a1o.committed() <= a1o.reserved()); + } - REQUIRE(a1o.allocated() == (cfg.header_.guard_z_ - + sizeof(header_type) - + z0 - + pad - + cfg.header_.guard_z_)); - REQUIRE(a1o.allocated() <= a1o.committed()); - REQUIRE(a1o.allocated() + a1o.available() == a1o.committed()); - REQUIRE(a1o.committed() <= a1o.reserved()); + a1o._drop(); + { + REQUIRE(a1o.allocated() == 0); + REQUIRE(a1o.committed() == 0); + } } TEST_CASE("allocator-fail-1", "[alloc2][AAllocator]") @@ -332,6 +373,8 @@ namespace xo { REQUIRE(cfg.size_ <= cfg.hugepage_z_); + REQUIRE(!a1o._has_null_vptr()); + REQUIRE(a1o.reserved() >= cfg.size_); REQUIRE(a1o.committed() == 0); REQUIRE(a1o.available() == 0); @@ -339,17 +382,22 @@ namespace xo { size_t z0 = cfg.hugepage_z_ + 1; byte * m0 = a1o.alloc(typeseq::sentinel(), z0); - - REQUIRE(!m0); - AllocError err = a1o.last_error(); + { + REQUIRE(!m0); + REQUIRE(err.error_ == error::reserve_exhausted); + REQUIRE(err.error_seq_ == 1); + REQUIRE(err.request_z_ >= z0); + REQUIRE(err.request_z_ < z0 + padding::c_alloc_alignment); + REQUIRE(err.committed_z_ == 0); + REQUIRE(err.reserved_z_ == arena.reserved()); + } - REQUIRE(err.error_ == error::reserve_exhausted); - REQUIRE(err.error_seq_ == 1); - REQUIRE(err.request_z_ >= z0); - REQUIRE(err.request_z_ < z0 + padding::c_alloc_alignment); - REQUIRE(err.committed_z_ == 0); - REQUIRE(err.reserved_z_ == arena.reserved()); + a1o._drop(); + { + REQUIRE(a1o.allocated() == 0); + REQUIRE(a1o.committed() == 0); + } } } /*namespace ut*/ } /*namespace xo*/ diff --git a/xo-gc/include/xo/gc/detail/IAllocator_DX1Collector.hpp b/xo-gc/include/xo/gc/detail/IAllocator_DX1Collector.hpp index f61d9fa9..9fd90bf9 100644 --- a/xo-gc/include/xo/gc/detail/IAllocator_DX1Collector.hpp +++ b/xo-gc/include/xo/gc/detail/IAllocator_DX1Collector.hpp @@ -80,8 +80,10 @@ namespace xo { void ** lhs_data, AGCObject * rhs_iface, void * rhs_data); +#ifdef OBSOLETE /** invoke destructor **/ static void destruct_data(DX1Collector & d); +#endif }; } /*namespace mm*/ diff --git a/xo-gc/src/gc/IAllocator_DX1Collector.cpp b/xo-gc/src/gc/IAllocator_DX1Collector.cpp index cf307e0f..7cc6add5 100644 --- a/xo-gc/src/gc/IAllocator_DX1Collector.cpp +++ b/xo-gc/src/gc/IAllocator_DX1Collector.cpp @@ -144,11 +144,13 @@ namespace xo { d.barrier_assign_aux(parent, lhs_iface, lhs_data, rhs_iface, rhs_data); } +#ifdef OBSOLETE void IAllocator_DX1Collector::destruct_data(DX1Collector & d) { d.~DX1Collector(); } +#endif } /*namespace mm*/ } /*namespace xo*/ diff --git a/xo-gc/utest/CMakeLists.txt b/xo-gc/utest/CMakeLists.txt index 401371f6..f226762e 100644 --- a/xo-gc/utest/CMakeLists.txt +++ b/xo-gc/utest/CMakeLists.txt @@ -14,6 +14,7 @@ set(UTEST_SRCS DMockCollector.cpp ICollector_DMockCollector.cpp + MockCollector.test.cpp MlsTestutil.cpp GcosTestutil.cpp diff --git a/xo-gc/utest/Collector.test.cpp b/xo-gc/utest/Collector.test.cpp index eff2eda1..26d1416e 100644 --- a/xo-gc/utest/Collector.test.cpp +++ b/xo-gc/utest/Collector.test.cpp @@ -58,6 +58,7 @@ namespace xo { /* empty variant collector */ obj gc1; + REQUIRE(!gc1._has_null_vptr()); REQUIRE(!gc1); REQUIRE(gc1.iface() != nullptr); REQUIRE(gc1.data() == nullptr); @@ -150,6 +151,12 @@ namespace xo { /* typed collector -- repr known at compile time */ obj x1(&gc); + REQUIRE(!x1._has_null_vptr()); + REQUIRE(x1.iface()); + REQUIRE(x1.data()); + + x1._drop(); + REQUIRE(x1.iface()); REQUIRE(x1.data()); } @@ -182,6 +189,11 @@ namespace xo { REQUIRE(x1.iface()); REQUIRE(x1.data()); + + x1._drop(); + + REQUIRE(x1.iface()); + REQUIRE(x1.data()); } TEST_CASE("collector-x1-alloc", "[alloc2][gc]") @@ -238,6 +250,11 @@ namespace xo { bool catch_flag = false; REQUIRE(utest::AllocUtil::random_allocs(c_n_alloc, c_max_alloc_payload, catch_flag, &rng, x1alloc)); + + x1gc._drop(); + + REQUIRE(x1gc.iface()); + REQUIRE(x1gc.data()); } TEST_CASE("collector-x1-alloc2", "[alloc2][gc]") @@ -298,6 +315,11 @@ namespace xo { // just testing ability to work as a low-level allocator REQUIRE(utest::AllocUtil::random_allocs(c_n_alloc, c_max_alloc_payload, c_debug_flag, &rng, x1alloc)); + + x1gc._drop(); + + REQUIRE(x1gc.iface()); + REQUIRE(x1gc.data()); } namespace { diff --git a/xo-gc/utest/MockCollector.test.cpp b/xo-gc/utest/MockCollector.test.cpp new file mode 100644 index 00000000..9092d2b1 --- /dev/null +++ b/xo-gc/utest/MockCollector.test.cpp @@ -0,0 +1,119 @@ +/** @file MockCollector.test.cpp + * + * @author Roland Conybeare, May 2026 + **/ + +#include "MockCollector.hpp" +#include +#include +#include +#include +#include + +namespace ut { + using xo::scm::DInteger; + using xo::mm::ACollector; + using xo::mm::DMockCollector; + using xo::mm::MutationLogConfig; + using xo::mm::MutationLogStore; + using xo::mm::GCObjectStoreConfig; + using xo::mm::GCObjectStore; + using xo::mm::AGCObject; + using xo::mm::Generation; + using xo::mm::Role; + using xo::mm::X1VerifyStats; + using xo::mm::AAllocator; + using xo::mm::ArenaConfig; + using xo::mm::DArena; + using xo::facet::obj; + + // Gilding the lily here. + // The only reason to 'test' MockCollector is to suppress + // false positives on coverage reports. + // Don't care about MockCollector itself, but it also shows up + // in ICollector_Xfer, at least on the osx/clang toolchain + + TEST_CASE("MockCollector-1", "[MockCollector]") + { + // need to create a {gcos, mls} pair + + constexpr uint32_t c_space_z = 64*1024; + constexpr uint32_t c_n_gen = 1; + constexpr uint32_t c_n_survive = 0; + X1VerifyStats verify_stats; + GCObjectStoreConfig gcos_config{ArenaConfig() + .with_name("gcos-arena-name-notused") + .with_size(c_space_z) + .with_store_header_flag(true), + c_n_gen, + c_n_survive, + 64*1024 /*object_type_z*/, + false /*debug_flag*/}; + DArena report_arena{ArenaConfig().with_name("report-arena") + .with_size(64*1024) + .with_store_header_flag(true)}; + DArena error_arena{ArenaConfig().with_name("error-arena") + .with_size(64*1024) + .with_store_header_flag(true)}; + MutationLogConfig mls_config{c_n_gen, + 1024 /*mlog_z*/, + false /*mlog_enabled_flag*/, + false /*debug_flag*/}; + + GCObjectStore gcos{gcos_config, &verify_stats}; + MutationLogStore mls{mls_config, &gcos}; + DMockCollector mock(&mls, &gcos); + + auto mockgc = obj(&mock); + auto report_mm = obj(&report_arena); + auto error_mm = obj(&error_arena); + + REQUIRE(mockgc.reserved(Generation::g0(), Role::to_space()) == c_space_z); + + { + int dummy; + + REQUIRE(mockgc.locate_address(&dummy) == -1); + REQUIRE(mockgc.contains(Role::to_space(), &dummy) == false); + } + + { + obj rpt; + { + // stub + REQUIRE(mockgc.report_statistics(report_mm, error_mm, &rpt) == false); + REQUIRE(report_mm.allocated() == 0); + } + { + mockgc.report_object_types(report_mm, error_mm, &rpt); + REQUIRE(rpt); + rpt.reset(); + report_mm.clear(); + } + { + mockgc.report_object_ages(report_mm, error_mm, &rpt); + REQUIRE(rpt); + rpt.reset(); + report_mm.clear(); + } + } + + { + auto g0_from = gcos.from_space(Generation::g0()); + REQUIRE(g0_from); + auto g0_from_mm = obj(g0_from); + + // note: we don't need object types in gcos for this test, + // since we're not traversing the object graph + + auto x = DInteger::box(g0_from_mm, 42); + REQUIRE(x); + + auto x_copy = mockgc.alloc_copy((std::byte*)x.data()); + REQUIRE(x_copy); + } + } + +} /*namespace ut*/ + +/* end MockCollector.test.cpp */ diff --git a/xo-gc/utest/X1Collector.test.cpp b/xo-gc/utest/X1Collector.test.cpp index 421c4639..00999be6 100644 --- a/xo-gc/utest/X1Collector.test.cpp +++ b/xo-gc/utest/X1Collector.test.cpp @@ -119,7 +119,8 @@ namespace ut { DX1Collector gc(cfg); - CollectorTypeRegistry::instance().install_types(obj(&gc)); + CollectorTypeRegistry::instance() + .install_types(obj(&gc)); DArena * to_0 = nullptr; @@ -219,6 +220,9 @@ namespace ut { + sizeof(AllocHeader) + sizeof(DInteger) + sizeof(AllocHeader) + sizeof(DList))); + auto to0_commit_z = to_0->committed(); + auto to0_reserved_z = to_0->reserved(); + { { REQUIRE(x0_o.iface() != nullptr); @@ -285,6 +289,12 @@ namespace ut { REQUIRE((void*)l0_o.data()->head_.data() == (void*)x0_o.data()); REQUIRE((void*)l0_o.data()->rest_ == (void*)DList::_nil()); + Generation g0 = Generation::g0(); + REQUIRE(c_o.committed(g0, Role::to_space()) == to0_commit_z); + REQUIRE(c_o.reserved(g0, Role::to_space()) == to0_reserved_z); + REQUIRE(c_o.locate_address(x0_o.data()) >= 0); + REQUIRE(c_o.contains(Role::to_space(), x0_o.data())); + } catch (std::exception & ex) { std::cerr << "caught exception: " << ex.what() << std::endl; REQUIRE(false);