From 61c1446eb5e369c831d73ae28a078d1f33dc61ed Mon Sep 17 00:00:00 2001 From: Daniel Polo <106583643+danielPoloWork@users.noreply.github.com> Date: Thu, 9 Jul 2026 19:07:20 +0200 Subject: [PATCH 1/3] =?UTF-8?q?feat(pool):=20add=20opt-in=20debug=20harden?= =?UTF-8?q?ing=20=E2=80=94=20poisoning,=20guard=20word,=20safe-linking?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Harden the intrusive free list against the classic use-after-free / pointer-corruption primitives an in-band next-link exposes (ADR-0009 §1), behind a single compile-time knob `PBR_MEMORY_POOL_HARDENING` (OFF by default; a `harden` CMake preset turns it on). Three layered protections: - Freed-block poisoning (0xDE), verified on the next allocation — use-after-free detection. - A per-slot trailing guard word: neither-constant on free is a buffer overflow past block_size, still-freed is a double-free (closing the ADR-0012 gap). The guard lives in *added* slot stride, so the user-visible block_size and the ADR-0009 alignment guarantee are unchanged. - glibc-style free-list safe-linking (ptr XOR (slot_addr >> 12)): a leaked/overwritten next-link is neither directly usable nor silently followed; corruption surfaces as an alignment fault on reveal. On detection a swappable HardeningViolationHandler fires; the default prints a diagnostic and abort()s (the ADR-0012 defined-loud-failure stance), and tests install a recording handler to assert detection without terminating the process. The knob is fully compiled out when off, so the default build is byte-for-byte and cycle-for-cycle unchanged (read_next/write_next/ reveal_next/slot_stride inline to the exact prior load/store). Works with fixed and dynamic pools across all three thread-safety policies; composes under the InstrumentedPool decorator. Purely additive and ABI-compatible (SemVer MINOR, a v1.2.0 candidate); a hardened build is deliberately not layout-compatible with a non-hardened one. A `harden` CI matrix cell builds and runs the detection tests on each Tier-1 platform (the memory-safety net where ASan is unavailable, e.g. MSVC). Decided in ADR-0043; ROADMAP item 9.2. Closes #109. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/ci.yml | 8 + CHANGELOG.md | 18 + CMakeLists.txt | 15 + CMakePresets.json | 13 +- ROADMAP.md | 2 +- SECURITY.md | 2 + docs/adr/0043-opt-in-debug-hardening.md | 100 +++++ docs/adr/README.md | 1 + docs/doxygen/Doxyfile | 1 + docs/specs/01_spec_cpp_memory_pool.md | 10 +- .../cpp/it/d4np/memorypool/memory_pool.cpp | 357 ++++++++++++++++-- src/main/cpp/it/d4np/memorypool/memory_pool.h | 16 + .../cpp/it/d4np/memorypool/pool_hardening.hpp | 84 +++++ .../cpp/it/d4np/memorypool/CMakeLists.txt | 25 ++ .../memorypool/free_list_iterator_test.cpp | 13 +- .../d4np/memorypool/pool_hardening_test.cpp | 182 +++++++++ 16 files changed, 803 insertions(+), 44 deletions(-) create mode 100644 docs/adr/0043-opt-in-debug-hardening.md create mode 100644 src/main/cpp/it/d4np/memorypool/pool_hardening.hpp create mode 100644 src/test/cpp/it/d4np/memorypool/pool_hardening_test.cpp diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f656e5c..359a125 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -64,14 +64,22 @@ jobs: - { os: ubuntu-24.04, compiler: clang, preset: release } - { os: ubuntu-24.04, compiler: clang, preset: asan } - { os: ubuntu-24.04, compiler: clang, preset: ubsan } + # Opt-in debug hardening (ADR-0043) — cross-platform (no sanitizer + # flags); builds the hardened configuration and runs its detection + # tests, the "clean in both configurations" acceptance for #109. + - { os: ubuntu-24.04, compiler: gcc, preset: harden } + - { os: ubuntu-24.04, compiler: clang, preset: harden } # Windows x86_64 — sanitizer presets are POSIX-only. - { os: windows-2022, compiler: msvc, preset: debug } - { os: windows-2022, compiler: msvc, preset: release } + # MSVC has no ASan in this matrix, so hardening is the memory-safety net here. + - { os: windows-2022, compiler: msvc, preset: harden } # macOS arm64 - { os: macos-14, compiler: apple-clang, preset: debug } - { os: macos-14, compiler: apple-clang, preset: release } - { os: macos-14, compiler: apple-clang, preset: asan } - { os: macos-14, compiler: apple-clang, preset: ubsan } + - { os: macos-14, compiler: apple-clang, preset: harden } steps: - name: Check out the source tree uses: actions/checkout@v6 diff --git a/CHANGELOG.md b/CHANGELOG.md index bf212b1..6ecc09d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,24 @@ dated version block (`## [X.Y.Z] — YYYY-MM-DD`) when a release PR closes a mil ### Added +- **Opt-in debug hardening — freed-block poisoning, a guard word, and free-list + safe-linking.** A compile-time knob `PBR_MEMORY_POOL_HARDENING` (OFF by default; a `harden` + CMake preset turns it on) hardens the intrusive free list against the classic + use-after-free / pointer-corruption primitives an in-band next-link exposes: freed blocks are + **poisoned** (`0xDE`) and verified on the next allocation (use-after-free), a per-slot + trailing **guard word** detects a contiguous write past `block_size` (buffer overflow) and a + repeated free (double-free — the [ADR-0012](docs/adr/0012-foreign-pointer-and-out-of-range-pointer-policy.md) + gap), and the next-link is stored with glibc-style **safe-linking** + (`ptr XOR (slot_addr >> 12)`) so a leaked/overwritten link is neither usable nor silently + followed. On detection a swappable `HardeningViolationHandler` + ([`pool_hardening.hpp`](src/main/cpp/it/d4np/memorypool/pool_hardening.hpp)) fires — the + default prints a diagnostic and `abort()`s. The guard lives in *added* slot stride, so the + user-visible `block_size` and the [ADR-0009](docs/adr/0009-free-list-layout-block-size-constraints-and-alignment-guarantee.md) + alignment guarantee are unchanged, and the default build is byte-for-byte and cycle-for-cycle + unchanged (the knob is fully compiled out). Works with fixed and dynamic pools across all + three thread-safety policies. Purely additive and ABI-compatible; a hardened build is + deliberately not layout-compatible with a non-hardened one (never mix configurations). + [ADR-0043](docs/adr/0043-opt-in-debug-hardening.md). Closes #109. - **`std::pmr::memory_resource` adapter — `PoolMemoryResource`.** A new header-only Adapter ([`pool_memory_resource.hpp`](src/main/cpp/it/d4np/memorypool/pool_memory_resource.hpp)) binds one `Pool` behind the runtime `std::pmr::memory_resource` interface, so a single diff --git a/CMakeLists.txt b/CMakeLists.txt index 16d3db1..42fff53 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -119,6 +119,21 @@ if(PBR_MEMORY_POOL_ENABLE_DIAGNOSTICS) target_compile_definitions(pbr_memory_pool PUBLIC PBR_MEMORY_POOL_DIAGNOSTICS=1) endif() +# --------------------------------------------------------------------------- +# Opt-in debug hardening (ADR-0043). When ON, the free list gains freed-block +# poisoning, a per-slot guard word (buffer-overflow + double-free detection), +# and next-pointer safe-linking (see pool_hardening.hpp). OFF by default — a +# release build is byte-for-byte unchanged. Defined PUBLIC because a hardened +# build changes the on-disk free-list encoding AND the physical slot stride, so +# the library and every consumer / test linking it must agree; never mix a +# hardened and a non-hardened build. +# --------------------------------------------------------------------------- +option(PBR_MEMORY_POOL_HARDENING + "Enable opt-in debug hardening: poisoning, guard word, free-list safe-linking (ADR-0043)" OFF) +if(PBR_MEMORY_POOL_HARDENING) + target_compile_definitions(pbr_memory_pool PUBLIC PBR_MEMORY_POOL_HARDENING=1) +endif() + # --------------------------------------------------------------------------- # Thread-safety policy (ADR-0020). Selects how the implementation # synchronizes the free-list head, fixed library-wide at build time. The diff --git a/CMakePresets.json b/CMakePresets.json index 19f97be..1286502 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -85,6 +85,15 @@ "rhs": "Windows" } }, + { + "name": "harden", + "inherits": "debug", + "displayName": "Debug + Hardening", + "description": "Debug + opt-in debug hardening: freed-block poisoning, per-slot guard word (overflow + double-free), and free-list safe-linking (ADR-0043). Cross-platform (no sanitizer flags).", + "cacheVariables": { + "PBR_MEMORY_POOL_HARDENING": "ON" + } + }, { "name": "bench", "displayName": "Benchmark", @@ -104,6 +113,7 @@ {"name": "asan", "configurePreset": "asan"}, {"name": "ubsan", "configurePreset": "ubsan"}, {"name": "tsan", "configurePreset": "tsan"}, + {"name": "harden", "configurePreset": "harden"}, {"name": "bench", "configurePreset": "bench"} ], "testPresets": [ @@ -111,6 +121,7 @@ {"name": "release", "configurePreset": "release", "output": {"outputOnFailure": true}, "execution": {"jobs": 0}}, {"name": "asan", "configurePreset": "asan", "output": {"outputOnFailure": true}, "execution": {"jobs": 0}}, {"name": "ubsan", "configurePreset": "ubsan", "output": {"outputOnFailure": true}, "execution": {"jobs": 0}}, - {"name": "tsan", "configurePreset": "tsan", "output": {"outputOnFailure": true}, "execution": {"jobs": 0}} + {"name": "tsan", "configurePreset": "tsan", "output": {"outputOnFailure": true}, "execution": {"jobs": 0}}, + {"name": "harden", "configurePreset": "harden", "output": {"outputOnFailure": true}, "execution": {"jobs": 0}} ] } diff --git a/ROADMAP.md b/ROADMAP.md index a1a8ffe..548a116 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -137,7 +137,7 @@ Goal: *post-`v1.0.0`*, stand up a **modular, professional documentation-translat Goal: a coherent, post-`v1.0.0` wave of **additive, ABI-compatible** capabilities on top of the frozen public surface — richer standard-library interop, opt-in memory-safety hardening, a fuzzing harness, and a broader benchmark baseline. Every item is additive, so the milestone targets `v1.2.0` (SemVer `MINOR`); each ships independently under the §6.1 one-PR-at-a-time rule, and closing the milestone is the `v1.2.0` bump ([ADR-0037](docs/adr/0037-new-feature-roadmap-placement.md)). Each item comes from a tracking issue. - [x] 9.1 **`std::pmr::memory_resource` adapter** (`PoolMemoryResource`) — the "door left open" in [ADR-0018](docs/adr/0018-stl-allocator-adapter.md): a `std::pmr::memory_resource` subclass binding one `Pool` so any `std::pmr`-aware container can draw from it through `std::pmr::polymorphic_allocator`, without the `PoolAllocator` per-type rebind. Deterministic `(bytes, alignment)` routing to the bound pool — over-sized / over-aligned requests delegate to a configurable upstream resource, and exhaustion of a pool-eligible request throws `std::bad_alloc` rather than falling back (preserving the deterministic deallocate routing) — with `is_equal` by `(pool, upstream)` identity, gated behind `PBR_MEMORY_POOL_HAS_PMR` where `` is available. Header-only, additive, ABI-compatible. Decided in [ADR-0042](docs/adr/0042-pmr-memory-resource-adapter.md); implemented in [`pool_memory_resource.hpp`](src/main/cpp/it/d4np/memorypool/pool_memory_resource.hpp) with [`pool_memory_resource_test.cpp`](src/test/cpp/it/d4np/memorypool/pool_memory_resource_test.cpp) (issue #107). -- [ ] 9.2 **Opt-in debug hardening** — freed-block poisoning, canaries, and free-list safe-linking (which also yields double-free detection); zero cost when the gate is off (issue #109). +- [x] 9.2 **Opt-in debug hardening** — freed-block poisoning, canaries, and free-list safe-linking (which also yields double-free detection); zero cost when the gate is off (issue #109). Decided in [ADR-0043](docs/adr/0043-opt-in-debug-hardening.md); implemented in [`memory_pool.cpp`](src/main/cpp/it/d4np/memorypool/memory_pool.cpp) behind the compile-time `PBR_MEMORY_POOL_HARDENING` knob (a `harden` CMake preset), with the swappable violation-handler surface in [`pool_hardening.hpp`](src/main/cpp/it/d4np/memorypool/pool_hardening.hpp) and [`pool_hardening_test.cpp`](src/test/cpp/it/d4np/memorypool/pool_hardening_test.cpp) (CTest `pool_hardening`). The "canary" is realized as one trailing **guard word** living in *added* slot stride — so the user-visible `block_size` and the ADR-0009 alignment guarantee are unchanged and the default build is byte-for-byte unchanged (the mechanism is fully compiled out). Poisoning (`0xDE`) catches use-after-free on the next allocation, the guard word catches a write past `block_size` and a double-free, and glibc-style safe-linking (`ptr XOR (slot_addr >> 12)`) protects the in-band next-link. A `harden` CI matrix cell builds and tests the hardened configuration on each Tier-1 platform. Works with fixed and dynamic pools across all three thread-safety policies. - [ ] 9.3 **Coverage-guided fuzzing harness** for the pool surface — a libFuzzer target under `src/fuzz/`, time-boxed in CI (issue #108). - [ ] 9.4 **Benchmark extension** — external allocator baselines (jemalloc / tcmalloc) and p99 tail-latency reporting (issue #111). diff --git a/SECURITY.md b/SECURITY.md index 4450dff..5bf4ff0 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -43,6 +43,8 @@ This is a single-maintainer reference project, so timelines are **best-effort**, In scope: memory-safety and correctness defects in the library's own code reachable through its public API or documented build options — e.g. out-of-bounds access, use-after-free, double-free, leaks, data races in the `MUTEX` / `LOCKFREE` policies, or integer-overflow in size computations. +For defense-in-depth and bug-finding, the library ships an **opt-in debug-hardening build** (compile-time knob `PBR_MEMORY_POOL_HARDENING`, OFF by default; [ADR-0043](docs/adr/0043-opt-in-debug-hardening.md)) that turns use-after-free, buffer-overflow-past-`block_size`, and double-free into deterministic, loud failures via freed-block poisoning, a per-slot guard word, and glibc-style free-list safe-linking. It complements the sanitizer matrix (and works where ASan does not, e.g. MSVC); it is a debugging aid, not a substitute for the sanitizers, and a hardened build is intentionally not layout-compatible with a default one. + Out of scope: misuse that the documentation explicitly calls undefined behaviour (e.g. pairing storage and object-lifetime verbs incorrectly, or a moved-from wrapper used after move), issues in a consumer's own code, and vulnerabilities in third-party toolchains. The default single-threaded build is intentionally not thread-safe (spec §2.4) — concurrent use of a `NONE`-policy pool is not a vulnerability. ## See also diff --git a/docs/adr/0043-opt-in-debug-hardening.md b/docs/adr/0043-opt-in-debug-hardening.md new file mode 100644 index 0000000..eaaaded --- /dev/null +++ b/docs/adr/0043-opt-in-debug-hardening.md @@ -0,0 +1,100 @@ +# ADR-0043: Opt-in debug hardening — poisoning, a guard word, and free-list safe-linking + +- **Status:** Accepted +- **Date:** 2026-07-08 +- **Deciders:** Daniel Polo (maintainer / project architect) +- **Related:** [ADR-0009](0009-free-list-layout-block-size-constraints-and-alignment-guarantee.md) (the intrusive free-list layout, the `block_size >= sizeof(void*)` + `alignof(std::max_align_t)` constraints, and the strict-aliasing-safe access idiom this ADR extends), [ADR-0012](0012-foreign-pointer-and-out-of-range-pointer-policy.md) (the "defined, loud failure" stance and the accepted no-default-double-free-detection trade-off this hardens), [ADR-0015](0015-metadata-overhead-budget-and-introspection.md) (the metadata-overhead budget kept intact when the gate is off), [ADR-0020](0020-thread-safety-strategy-and-compile-time-knob.md) (the three synchronization policies the hardening threads through), [ADR-0022](0022-dynamic-growth-policy-and-chunk-linking.md) (the growth chunks the stride change must respect), [ADR-0025](0025-decorator-for-instrumented-pool.md) (the `InstrumentedPool` decorator that composes over the hardened pool), [ADR-0037](0037-new-feature-roadmap-placement.md) (roadmap Milestone 9, which this item continues), [ADR-0004](0004-versioning-and-release-policy.md) (the SemVer **MINOR** this lands under), issue #109, origin issue #105, spec [§4.1](../specs/01_spec_cpp_memory_pool.md#41-constraints--guarantees) / [§5.3](../specs/01_spec_cpp_memory_pool.md#53-error-semantics) + +## Context + +The pool's free list is **intrusive** ([ADR-0009](0009-free-list-layout-block-size-constraints-and-alignment-guarantee.md) §1): a free block stores the address of the next free block *inside its own first `sizeof(void*)` bytes*. That is what makes the metadata overhead of a free block zero (spec §3.2) — and it is also a classic memory-corruption exploitation primitive. A use-after-free write into a freed block silently overwrites a live next-link; a linear buffer overflow past `block_size` corrupts the following slot; a leaked free-list pointer is a ready-made write target. The default build detects none of this: it deliberately trades detection for speed and a zero-overhead free block, and it does **not** detect a double-free of an in-range block ([ADR-0012](0012-foreign-pointer-and-out-of-range-pointer-policy.md), spec §5.3). + +The rest of the repository holds an enterprise/security posture — a [`SECURITY.md`](../../SECURITY.md), a sanitizer matrix ([ADR-0005](0005-toolchain-matrix-and-supported-platforms.md)), Valgrind-clean verification (spec §6.2). The specification review (#105) flagged the gap: "no mention of pointer obfuscation, freed-block poisoning, or debug canaries." AddressSanitizer covers much of this on the platforms where it exists — but it is unavailable on MSVC ([ADR-0005](0005-toolchain-matrix-and-supported-platforms.md) §3), and it cannot instrument the pool's *own* sub-block boundaries: to ASan a pool is one large `operator new` region, so an overflow from one pool block into the next is invisible to its redzones. + +We want a **self-contained, opt-in** hardening mode that works everywhere the library builds, catches the intrusive-free-list failure modes deterministically, demonstrates a real shipping allocator-hardening technique (glibc ≥ 2.32 safe-linking), and costs **nothing** — not a byte of footprint, not a cycle on the hot path — when it is off. + +Four forces shape the design: + +1. **Zero cost when off.** The default build and the [ADR-0014](0014-microbenchmark-methodology-pool-vs-malloc.md) benchmark numbers and the [ADR-0015](0015-metadata-overhead-budget-and-introspection.md) overhead budget must be byte-for-byte and cycle-for-cycle unchanged. That rules out any runtime flag on the allocate/deallocate path; the knob must be **compile-time**. +2. **Don't disturb the frozen contract.** [ADR-0009](0009-free-list-layout-block-size-constraints-and-alignment-guarantee.md) fixes `block_size >= sizeof(void*)`, `block_size` a multiple of `alignof(std::max_align_t)`, and uniform `alignof(std::max_align_t)` alignment of every returned pointer. Hardening must preserve all three and must not shrink the user-visible `block_size`. +3. **Uniform application.** The transform must be applied at *every* free-list access site (init, push, pop, the debug walk) across all three thread-safety policies and across dynamic-growth chunks, or a single un-transformed access corrupts the list. +4. **Defined behaviour on detection.** A detected violation is a bug in the *consumer*; the response must be a defined, loud failure ([ADR-0012](0012-foreign-pointer-and-out-of-range-pointer-policy.md)) — and it must be observable by a test without terminating the test process. + +## Decision + +Add an **opt-in, debug-only hardening mode** behind a single compile-time knob, `PBR_MEMORY_POOL_HARDENING` (a CMake `option`, **OFF** by default; a `harden` CMake preset turns it on). When it is off the entire mechanism is preprocessed out and this ADR's code is a no-op. The mode adds three protections, all layered onto the existing intrusive free list. + +### 1. Physical slot stride, decoupled from `block_size` + +Hardening reserves a trailing **guard word** (`std::uint64_t`) after each block's usable bytes. Rather than steal bytes from the user, the physical **slot stride** grows: `stride = roundup(block_size + sizeof(guard), alignof(std::max_align_t))`. Because `block_size` is already a multiple of `alignof(std::max_align_t)` (ADR-0009 §2), the stride becomes `block_size + alignof(std::max_align_t)` — the guard and its padding live in *added* storage, so: + +- the user-visible `block_size` and the ADR-0009 §5 alignment guarantee are **unchanged** (every slot start remains `alignof(std::max_align_t)`-aligned); +- the cost is memory footprint **in the hardened build only** — one extra `alignof(std::max_align_t)` per slot. + +A single `slot_stride(block_size)` helper is the identity (`== block_size`) when the knob is off, and *every* addressing site — `initialize_free_list`, `block_in_chunk` (range end + modulo), `grow_pool`, and `memory_pool_create` (including their `size_t`-overflow guards) — computes offsets from the stride, not from `block_size`. + +### 2. Freed-block poisoning (use-after-free) + +On free, after the next-link is threaded into the first `sizeof(void*)` bytes, the remainder of the block (`[sizeof(void*), block_size)`) is filled with a recognizable byte pattern (`0xDE`). On the next allocation of that slot, the pattern is verified intact before the block is handed out; a broken pattern is a **use-after-free** write and is reported. (The link bytes themselves are not poisoned — they are protected by safe-linking, below. When `block_size == sizeof(void*)` the block is entirely the link and has no payload to poison — see *Consequences*.) + +### 3. Guard word (buffer overflow + double-free) + +The trailing guard word is stamped `GUARD_ALLOCATED` when a block is vended and `GUARD_FREED` when it is returned. On free, the guard is validated first: still-`GUARD_FREED` ⇒ **double-free**; any value that is neither constant ⇒ a contiguous **buffer overflow** past `block_size` clobbered it. This is the "canary" the origin issue asked for — realized as one guard word living in the added stride (so, unlike a classic in-band canary, it costs no usable bytes and needs no separate sub-knob), and it makes double-free detection (the [ADR-0012](0012-foreign-pointer-and-out-of-range-pointer-policy.md) gap) fall out for free. + +### 4. Free-list next-pointer safe-linking + +The in-band next-link is stored **XORed with a per-slot key** derived from the slot's own address (`ptr XOR (slot_addr >> 12)`) — glibc's `PROTECT_PTR`/`REVEAL_PTR` transform, which is symmetric (protect == reveal). Two consequences: a next-link leaked out of band is not a directly usable address, and a corrupted stored value reveals to a **misaligned** pointer, which an alignment check on reveal catches as **free-list corruption**. Access stays strict-aliasing-safe: the value is still read through `void* const*` and written through `void**` (ADR-0009 §3), only the bits stored differ. + +### Detection policy — a swappable violation handler + +On any detected violation the library calls an installed `HardeningViolationHandler` (declared in the new [`pool_hardening.hpp`](../../src/main/cpp/it/d4np/memorypool/pool_hardening.hpp), present only in hardened builds). The **default** handler prints a diagnostic to `std::cerr` and calls `std::abort()` — the [ADR-0012](0012-foreign-pointer-and-out-of-range-pointer-policy.md) defined-loud-failure stance. `set_hardening_violation_handler` swaps it (thread-safe, via an atomic) and returns the previous one; passing `nullptr` restores the default. The handler is `noexcept` (it is called from the pool's `noexcept` (de)allocate path). A handler that **returns** — which the default never does — puts the pool on a best-effort path so a test can *assert* a violation without terminating the process; the tests install such a recording handler. Because the default aborts at the point of detection, the production path never continues past a violation. + +### Compatibility + +The knob is defined **PUBLIC** on the CMake target: a hardened build changes both the on-disk free-list encoding (safe-linking) *and* the physical slot stride, so the library and every consumer / test linking it must agree on the value. A hardened build is therefore deliberately **not** memory-layout-compatible with a non-hardened one — never mix the two configurations. The public C ABI and C++ types are otherwise untouched; the default build is byte-for-byte unchanged. This is purely additive → SemVer **MINOR** (a `v1.2.0` candidate, [ADR-0004](0004-versioning-and-release-policy.md); roadmap item 9.2 under [ADR-0037](0037-new-feature-roadmap-placement.md)). + +## Alternatives Considered + +- **A runtime flag instead of a compile-time knob.** Rejected. A per-operation branch on the hot path violates force 1 (the zero-cost-when-off guarantee and the ADR-0014/0015 numbers). A compile-time gate produces *identical* codegen when off. +- **Steal the guard/canary bytes from `block_size` (a distinct "canary" sub-mode with its own knob, as #109 sketched).** Rejected. Shrinking the usable block would change the ADR-0009 §2 block-size math and the user-visible contract, and it needed a second knob to manage the trade-off. Putting the guard in *added* stride keeps the user contract identical, costs no usable bytes, and folds overflow + double-free detection into one knob. +- **Head + tail canaries (two guard words).** Rejected as unnecessary for the fixed-block model: a *forward* linear overflow past `block_size` is the realistic case, one trailing guard catches it and doubles as the free/allocated state stamp, and a second word would only add footprint. (Underflow into the preceding slot is instead caught by *that* slot's poison/guard on its next cycle.) +- **Poison the whole block including the link bytes.** Rejected: the first `sizeof(void*)` bytes legitimately hold the (safe-linked) next-link while free, so they cannot also hold poison. Safe-linking guards those bytes instead; poison covers the rest. +- **Raw next-pointer with only poisoning (no safe-linking).** Rejected. Safe-linking is the didactic centrepiece (a real, shipping glibc technique; force in [ADR-0003](0003-design-patterns-policy.md)'s "exercise production-grade techniques and justify them" mandate), it makes a leaked link non-trivial to weaponize, and its alignment-on-reveal check gives cheap free-list-integrity detection that poisoning alone does not. +- **Lean on AddressSanitizer only.** Rejected as insufficient, not as competing: ASan is absent on MSVC (ADR-0005 §3) and cannot see *inside* the pool's own region (block-to-block overflow). This mode complements ASan and runs everywhere the library builds; the two are used together in CI. +- **A fixed abort with no handler hook.** Rejected. A swappable handler is what lets the test suite *prove* each violation is detected without killing the process, at zero cost to the production default (which still aborts). + +## Consequences + +**Positive** + +- Deterministic detection of the three intrusive-free-list failure modes — use-after-free, forward buffer overflow past `block_size`, and double-free — everywhere the library builds, including MSVC where ASan is unavailable. +- The user-visible `block_size` and the ADR-0009 alignment guarantee are unchanged; the default build and its benchmark/overhead numbers are byte-for-byte and cycle-for-cycle unaffected (verified by the existing zero-overhead check and the CI `debug`/`release` cells). +- Demonstrates safe-linking end to end — the threat model, the `PROTECT_PTR`/`REVEAL_PTR` transform, and the alignment-on-reveal integrity check — in a shipping-quality reference, satisfying the #105 hardening ask and the ADR-0003 mandate. +- Works with fixed **and** dynamic pools and all three thread-safety policies; composes under the `InstrumentedPool` decorator (ADR-0025). + +**Negative** + +- A hardened build is a *separate configuration*: incompatible on-disk layout, and one extra `alignof(std::max_align_t)` of footprint per slot. This is stated explicitly and is the point (never mix configurations). +- The hardening runs validation work on every allocate/deallocate — it is a debug/bug-finding mode, not a production speed path. The compile-time gate is what keeps that cost strictly opt-in. +- Poisoning covers `[sizeof(void*), block_size)`; a use-after-free write that lands *only* in the first `sizeof(void*)` link bytes is caught by safe-linking's reveal check on the next pop rather than by poison. Where `block_size == sizeof(void*)` — reachable only where `alignof(std::max_align_t)` equals the pointer size (e.g. MSVC x64, both 8) — the block is *entirely* the link and has no payload to poison, so use-after-free of such a slot is covered by safe-linking (and the guard word / double-free check) rather than by poison. Larger blocks carry poisoned payload bytes. This is an inherent property of the layout, documented rather than papered over: rejecting such block sizes was considered and rejected because it would break `TypedPool` / `PoolAllocator` for pointer-sized `T` under hardening, and the [ADR-0009](0009-free-list-layout-block-size-constraints-and-alignment-guarantee.md) block-size contract stays identical in both configurations. + +**Testing / tooling / documentation (landing in the same PR)** + +- [`pool_hardening.hpp`](../../src/main/cpp/it/d4np/memorypool/pool_hardening.hpp) — the violation-handler surface (get/set + the `HARDENING_*` kind constants), present only in hardened builds. +- [`pool_hardening_test.cpp`](../../src/test/cpp/it/d4np/memorypool/pool_hardening_test.cpp) — a dedicated doctest binary (CTest `pool_hardening`), gated like `free_list_iterator_test` so it builds (with one placeholder case) in the default build, and under the `harden` preset asserts no-false-positive cycles plus deterministic detection of use-after-free, overflow, and double-free via a recording handler. +- [`CMakeLists.txt`](../../CMakeLists.txt) / [`CMakePresets.json`](../../CMakePresets.json) — the `PBR_MEMORY_POOL_HARDENING` option (PUBLIC) and a `harden` configure/build/test preset. +- [`.github/workflows/ci.yml`](../../.github/workflows/ci.yml) — a `harden` matrix cell on each Tier-1 platform so the hardened configuration is built and its tests run under the quality bar (the "clean in *both* configurations" acceptance criterion). +- [`docs/specs/01_spec_cpp_memory_pool.md`](../specs/01_spec_cpp_memory_pool.md) — §4.1 gains a hardening note, §5.3 records that opt-in detection now exists, and §7 maps it to this ADR / §7.1 no longer defers it. (The spec is a translated source but is already `stale` from the prior spec PRs, so this edit adds no new i18n debt.) +- [`docs/doxygen/Doxyfile`](../doxygen/Doxyfile) `PREDEFINED` gains `PBR_MEMORY_POOL_HARDENING=1` so the gated handler surface is documented on the API site. +- [`SECURITY.md`](../../SECURITY.md) — the scope note records the opt-in hardening build as a defense-in-depth / bug-finding aid. +- [`ROADMAP.md`](../../ROADMAP.md) — item 9.2 flips to done. +- [`CHANGELOG.md`](../../CHANGELOG.md) `Unreleased` — an *Added* entry. +- **Deferred to the `v1.2.0` release PR** to keep this feature PR off the *translated* docs surface (editing them trips the `i18n-freshness` gate, and their rows are currently `translated`): the [`README.md`](../../README.md) security-section refresh and Milestone-9 table row, plus any [`docs/patterns/README.md`](../patterns/README.md) touch. The release PR refreshes them and re-syncs the `zh-Hans` / `ja` translations in one pass — the same M8.8 / ADR-0042 sequencing. + +## References + +- glibc `malloc` safe-linking (`PROTECT_PTR` / `REVEAL_PTR`), introduced in glibc 2.32 — the XOR-with-`(addr >> 12)` transform this mirrors, and its alignment-on-reveal integrity check. +- [ADR-0009](0009-free-list-layout-block-size-constraints-and-alignment-guarantee.md) — the intrusive free-list layout, the block-size/alignment constraints, and the strict-aliasing-safe access idiom. +- [ADR-0012](0012-foreign-pointer-and-out-of-range-pointer-policy.md) — the defined-loud-failure stance and the default no-double-free-detection trade-off this hardens. +- [ADR-0015](0015-metadata-overhead-budget-and-introspection.md) — the metadata-overhead budget kept intact when the gate is off. +- [`SECURITY.md`](../../SECURITY.md) — the project's security policy and scope. diff --git a/docs/adr/README.md b/docs/adr/README.md index 1f86548..a35c9d5 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -75,5 +75,6 @@ Do **not** write one for purely local implementation details, formatting, or tri | 0040 | [Pull-request metadata policy (assignee, labels, milestone)](0040-pull-request-metadata-policy.md) | Accepted | | 0041 | [Mermaid as the in-repo diagram tooling](0041-mermaid-diagram-tooling.md) | Accepted | | 0042 | [`std::pmr::memory_resource` adapter — `PoolMemoryResource`](0042-pmr-memory-resource-adapter.md) | Accepted | +| 0043 | [Opt-in debug hardening — poisoning, a guard word, and free-list safe-linking](0043-opt-in-debug-hardening.md) | Accepted | When adding a new ADR, append a row to this table in the same PR. diff --git a/docs/doxygen/Doxyfile b/docs/doxygen/Doxyfile index debd797..c44a263 100644 --- a/docs/doxygen/Doxyfile +++ b/docs/doxygen/Doxyfile @@ -66,6 +66,7 @@ EXPAND_ONLY_PREDEF = YES SEARCH_INCLUDES = YES INCLUDE_PATH = src/main/cpp PREDEFINED = PBR_MEMORY_POOL_DIAGNOSTICS=1 \ + PBR_MEMORY_POOL_HARDENING=1 \ __cpp_lib_memory_resource=201603L \ __cplusplus=201703L diff --git a/docs/specs/01_spec_cpp_memory_pool.md b/docs/specs/01_spec_cpp_memory_pool.md index d7e5909..a9ff7d9 100644 --- a/docs/specs/01_spec_cpp_memory_pool.md +++ b/docs/specs/01_spec_cpp_memory_pool.md @@ -86,6 +86,8 @@ Storing a next-pointer in-band imposes constraints, all enforced by the implemen The intrusive-free-list design was chosen over a bitmap allocator; the rejected alternative and its rationale are recorded in [ADR-0009](../adr/0009-free-list-layout-block-size-constraints-and-alignment-guarantee.md). +An in-band next-link is also a classic use-after-free / pointer-corruption primitive. An **opt-in debug-hardening mode** (compile-time knob `PBR_MEMORY_POOL_HARDENING`, OFF by default; [ADR-0043](../adr/0043-opt-in-debug-hardening.md)) hardens the free list against it: freed-block **poisoning** (use-after-free), a trailing per-slot **guard word** (buffer overflow past `block_size`, and double-free), and glibc-style **safe-linking** of the next-link (`ptr XOR (slot_addr >> 12)`). The guard lives in *added* slot stride, so the constraints above — the usable `block_size` and the uniform alignment — are unchanged, and the default build is byte-for-byte and cycle-for-cycle unaffected (the mechanism is fully compiled out). + ### 4.2 Component diagram (C4) The C4-model **Component** view below relates the public surface, the core engine (which lives behind the opaque `memory_pool_t` handle / Pimpl), and the operating-system backing store. It is authored in Mermaid per [ADR-0041](../adr/0041-mermaid-diagram-tooling.md); a viewer without Mermaid support sees the equivalent source. Solid arrows are runtime call / ownership paths; dashed arrows are optional or diagnostic relationships. Bracketed tags name the realizing technology or the design pattern applied. @@ -176,7 +178,7 @@ A layered, idiomatic C++ surface built on the C core: - **Freeing `NULL`** — no-op (mirrors C `free(NULL)`). - **Freeing a foreign / out-of-range pointer** — defined, silent no-op detected in $O(1)$; no corruption ([ADR-0012](../adr/0012-foreign-pointer-and-out-of-range-pointer-policy.md)). -- **Double-free** — a double-free of an in-range, currently-free block is **not** detected by the default build (accepted trade-off, documented in [ADR-0012](../adr/0012-foreign-pointer-and-out-of-range-pointer-policy.md)); opt-in detection is tracked as future hardening (Section 7). +- **Double-free** — a double-free of an in-range, currently-free block is **not** detected by the default build (accepted trade-off, documented in [ADR-0012](../adr/0012-foreign-pointer-and-out-of-range-pointer-policy.md)); the opt-in debug-hardening mode ([§4.1](#41-constraints--guarantees), [ADR-0043](../adr/0043-opt-in-debug-hardening.md)) detects it deterministically via the per-slot guard word. - **C++ exhaustion** — throws per [ADR-0016](../adr/0016-exception-policy-at-the-c-cpp-boundary.md); exceptions never cross the C ABI. ### 5.4 Introspection @@ -206,7 +208,7 @@ valgrind --leak-check=full --show-leak-kinds=all ./test_pool ### 6.4 Sanitizers & CI -ASan, UBSan and TSan run via dedicated CMake presets; TSan covers the thread-safe configurations. All of the above is wired into a multi-OS CI matrix (warnings-as-errors, `clang-tidy`, Valgrind). A coverage-guided fuzz target is deferred (Section 7). +ASan, UBSan and TSan run via dedicated CMake presets; TSan covers the thread-safe configurations. The opt-in debug-hardening configuration ([§4.1](#41-constraints--guarantees), [ADR-0043](../adr/0043-opt-in-debug-hardening.md)) runs via a `harden` preset — a Tier-1 CI matrix cell builds it and runs its detection tests, so the quality bar holds in both the default and the hardened configuration. All of the above is wired into a multi-OS CI matrix (warnings-as-errors, `clang-tidy`, Valgrind). A coverage-guided fuzz target is deferred (Section 7). --- @@ -220,6 +222,7 @@ Every requirement above is realized and recorded. The table maps the spec to its | §2.4 thread safety (mutex / lock-free + ABA tag) | [ADR-0020](../adr/0020-thread-safety-strategy-and-compile-time-knob.md) | | §3.2 overhead budget & introspection | [ADR-0015](../adr/0015-metadata-overhead-budget-and-introspection.md) | | §4 free-list layout, constraints, alignment, intrusive-vs-bitmap | [ADR-0009](../adr/0009-free-list-layout-block-size-constraints-and-alignment-guarantee.md) | +| §4.1 / §5.3 opt-in debug hardening (poisoning, guard word, safe-linking) | [ADR-0043](../adr/0043-opt-in-debug-hardening.md) | | §4.2 component (C4) diagram & diagram tooling | [ADR-0041](../adr/0041-mermaid-diagram-tooling.md) | | §5.1–5.2 API, RAII, Pimpl, builder, typed pool, STL adapter | [ADR-0010](../adr/0010-raii-pool-wrapper-and-pimpl-across-the-c-cpp-boundary.md), [ADR-0011](../adr/0011-factory-method-and-builder-for-pool-construction.md), [ADR-0017](../adr/0017-typed-pool-design.md), [ADR-0018](../adr/0018-stl-allocator-adapter.md) | | §5.2 `std::pmr` adapter (PoolMemoryResource) | [ADR-0042](../adr/0042-pmr-memory-resource-adapter.md) | @@ -233,7 +236,8 @@ Every requirement above is realized and recorded. The table maps the spec to its These are explicitly out of the current build and tracked as issues: - **Coverage-guided fuzzing harness** (issue #108). -- **Opt-in debug hardening** — freed-block poisoning, canaries, free-list safe-linking; would also add double-free detection (issue #109). - **Benchmark extension** — external baselines (jemalloc/tcmalloc) and p99 percentile reporting (issue #111). +**Opt-in debug hardening** (freed-block poisoning, a guard word for overflow + double-free detection, and free-list safe-linking), once deferred here, now ships behind the `PBR_MEMORY_POOL_HARDENING` compile-time knob — see [§4.1](#41-constraints--guarantees) and [ADR-0043](../adr/0043-opt-in-debug-hardening.md) (issue #109). + The **C4 component diagram** of the pool internals, once deferred here, now ships in [Section 4.2](#42-component-diagram-c4) (its tooling decision is [ADR-0041](../adr/0041-mermaid-diagram-tooling.md)). diff --git a/src/main/cpp/it/d4np/memorypool/memory_pool.cpp b/src/main/cpp/it/d4np/memorypool/memory_pool.cpp index a2e8baa..c9e99ca 100644 --- a/src/main/cpp/it/d4np/memorypool/memory_pool.cpp +++ b/src/main/cpp/it/d4np/memorypool/memory_pool.cpp @@ -26,6 +26,7 @@ #include #include +#include #include #include @@ -34,6 +35,12 @@ #include #include +#if PBR_MEMORY_POOL_HARDENING +#include +#include +#include +#endif + namespace { // Drop-in `malloc` alignment is the contract ADR-0009 §5 commits to. @@ -73,29 +80,212 @@ void release_backing(void* backing) noexcept { ::operator delete(backing, std::align_val_t{POOL_ALIGNMENT}); } +// --------------------------------------------------------------------------- +// Opt-in debug hardening (ADR-0043). Everything here is compiled only when +// PBR_MEMORY_POOL_HARDENING is set; a release build is byte-for-byte unchanged. +// The next-link accessors read_next / write_next are used at EVERY free-list +// site below so the safe-linking transform is applied uniformly; when hardening +// is off they inline to the exact `*static_cast(slot)` load/store the +// pool has always used, so the codegen is identical. +// --------------------------------------------------------------------------- + +// Physical slot stride. Hardening reserves a trailing guard word after the +// user-visible block_size and rounds the stride up to POOL_ALIGNMENT so every +// slot start stays aligned (ADR-0009 §5). Off, this is the identity — the +// stride IS block_size, exactly as before. +constexpr std::size_t slot_stride(std::size_t block_size) noexcept { +#if PBR_MEMORY_POOL_HARDENING + // Guard both intermediate additions against size_t overflow: a block_size + // within a guard-word (or a round-up) of SIZE_MAX would otherwise wrap and + // collapse the stride to 0 / a tiny value, sailing past the caller's + // would_overflow_product guard and causing an out-of-bounds guard/poison + // write. Saturating to SIZE_MAX instead makes that guard reject the input + // (return NULL / false) exactly as the non-hardened identity path does. + constexpr std::size_t SIZE_LIMIT = std::numeric_limits::max(); + if (block_size > SIZE_LIMIT - sizeof(std::uint64_t)) { + return SIZE_LIMIT; + } + const std::size_t raw = block_size + sizeof(std::uint64_t); + if (raw > SIZE_LIMIT - (POOL_ALIGNMENT - 1U)) { + return SIZE_LIMIT; + } + return ((raw + POOL_ALIGNMENT - 1U) / POOL_ALIGNMENT) * POOL_ALIGNMENT; +#else + return block_size; +#endif +} + +#if PBR_MEMORY_POOL_HARDENING + +namespace hardening_detail { + +constexpr std::size_t SAFE_LINK_SHIFT = 12U; // glibc PROTECT_PTR page shift +constexpr unsigned char POISON_BYTE = 0xDEU; // freed-block payload fill +constexpr std::uint64_t GUARD_FREED = 0xF3EEF3EEF3EEF3EEULL; +constexpr std::uint64_t GUARD_ALLOCATED = 0xA110CA7EA110CA7EULL; + +// glibc safe-linking transform (symmetric: protect == reveal). Keying the +// stored value on the slot's own address means an out-of-band leaked pointer is +// not directly usable, and a corrupted store reveals to a misaligned address. +// The (slot, value) pair models distinct roles (the keying address vs. the +// pointer being (un)masked); the transform is symmetric so a swap is harmless, +// and a config struct for two pointers would only obscure the call sites. +// NOLINTNEXTLINE(bugprone-easily-swappable-parameters) +void* xor_link(const void* slot, void* value) noexcept { + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) + const auto key = reinterpret_cast(slot) >> SAFE_LINK_SHIFT; + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) + const auto raw = reinterpret_cast(value); + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast,performance-no-int-to-ptr) + return reinterpret_cast(key ^ raw); +} + +void report(const char* kind, const void* block) noexcept { + it::d4np::memorypool::hardening_violation_handler()(kind, block); +} + +std::uint64_t read_guard(const void* slot, std::size_t block_size) noexcept { + std::uint64_t value = 0U; + std::memcpy(&value, static_cast(slot) + block_size, sizeof(value)); + return value; +} + +void write_guard(void* slot, std::size_t block_size, std::uint64_t value) noexcept { + std::memcpy(static_cast(slot) + block_size, &value, sizeof(value)); +} + +void poison_payload(void* slot, std::size_t block_size) noexcept { + // The first sizeof(void*) bytes hold the (safe-linked) next-link; poison + // only the payload beyond it. + if (block_size > sizeof(void*)) { + std::memset(static_cast(slot) + sizeof(void*), POISON_BYTE, block_size - sizeof(void*)); + } +} + +bool payload_is_poison(const void* slot, std::size_t block_size) noexcept { + const auto* const payload = static_cast(slot) + sizeof(void*); + const std::size_t count = (block_size > sizeof(void*)) ? (block_size - sizeof(void*)) : 0U; + for (std::size_t i = 0; i < count; ++i) { + if (payload[i] != POISON_BYTE) { + return false; + } + } + return true; +} + +// A fresh (never-user-owned) slot from initialize_free_list: poison its payload +// and stamp it free. Its next-link is written by initialize_free_list itself. +void on_init_slot(void* slot, std::size_t block_size) noexcept { + poison_payload(slot, block_size); + write_guard(slot, block_size, GUARD_FREED); +} + +// On free: validate the guard word — a still-freed stamp is a double-free, any +// other non-allocated value is a write past block_size — then poison the +// payload and re-stamp the slot free. +// +// @return `true` if the caller should enqueue the block, `false` on a detected +// double-free. A double-freed block is *already* on the free list (from its +// first, legitimate free), so re-linking it would duplicate it / cycle the +// list. The default handler aborts before returning, so this only matters for a +// returning handler — but there it upholds the documented "no-further- +// corruption" contract (pool_hardening.hpp) by dropping the redundant push. +[[nodiscard]] bool on_free(void* slot, std::size_t block_size) noexcept { + const std::uint64_t guard = read_guard(slot, block_size); + bool double_free = false; + if (guard == GUARD_FREED) { + report(it::d4np::memorypool::HARDENING_DOUBLE_FREE, slot); + double_free = true; + } else if (guard != GUARD_ALLOCATED) { + report(it::d4np::memorypool::HARDENING_OVERFLOW, slot); + } + poison_payload(slot, block_size); + write_guard(slot, block_size, GUARD_FREED); + return !double_free; +} + +// On alloc: validate the free stamp (free-list integrity) and the payload +// poison (use-after-free), then stamp the slot allocated. +void on_alloc(void* slot, std::size_t block_size) noexcept { + if (read_guard(slot, block_size) != GUARD_FREED) { + report(it::d4np::memorypool::HARDENING_FREELIST_CORRUPTION, slot); + } + if (!payload_is_poison(slot, block_size)) { + report(it::d4np::memorypool::HARDENING_USE_AFTER_FREE, slot); + } + write_guard(slot, block_size, GUARD_ALLOCATED); +} + +} // namespace hardening_detail + +#endif // PBR_MEMORY_POOL_HARDENING + +// Reveal the next-free link (the safe-linking transform when hardened, else the +// plain load) WITHOUT the integrity check. Used where the slot may be read +// speculatively: the lock-free pop reads the head's link *before* the CAS +// establishes ownership, so those bytes may momentarily be another thread's +// user data — running the alignment check there would false-positive and abort +// a correct program (ADR-0043). The stale value is discarded when the CAS fails. +void* reveal_next(const void* slot) noexcept { +#if PBR_MEMORY_POOL_HARDENING + return hardening_detail::xor_link(slot, *static_cast(slot)); +#else + return *static_cast(slot); +#endif +} + +// Read the next-free link out of a genuinely-free slot, adding the safe-linking +// integrity check when hardened. Callers must own or quiesce the slot: the +// single-thread and mutex pops hold exclusivity and the diagnostic walk runs on +// a stable list. The lock-free pop uses reveal_next instead (see above) and +// defers integrity to on_alloc's guard-word check once the slot is owned. +void* read_next(const void* slot) noexcept { + void* const revealed = reveal_next(slot); +#if PBR_MEMORY_POOL_HARDENING + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) + if (revealed != nullptr && (reinterpret_cast(revealed) % POOL_ALIGNMENT) != 0U) { + hardening_detail::report(it::d4np::memorypool::HARDENING_FREELIST_CORRUPTION, slot); + } +#endif + return revealed; +} + +// Write the next-free link into a free slot — applying the safe-linking protect +// transform when hardened, else the plain store. +void write_next(void* slot, void* next) noexcept { +#if PBR_MEMORY_POOL_HARDENING + // `slot` is a free-list slot inside a pool backing sized `stride * count` + // (stride >= sizeof(void*)); the first sizeof(void*) bytes are always in + // bounds. The analyzer cannot prove this with a symbolic block_size, so it + // reports a false out-of-bounds store — suppressed narrowly. + // NOLINTNEXTLINE(clang-analyzer-security.ArrayBound) + *static_cast(slot) = hardening_detail::xor_link(slot, next); +#else + *static_cast(slot) = next; +#endif +} + void initialize_free_list(void* backing, std::size_t block_size, std::size_t block_count) noexcept { - // Implicit free list per ADR-0009 §1, ascending address order. Each - // free slot stores the address of the next free slot in its own first - // `sizeof(void*)` bytes; the last slot stores a null pointer. - // - // The `*static_cast(slot) = ptr` idiom is the canonical - // pool-allocator write: it expresses "treat the slot's first bytes - // as storage for a void* and assign the next-link value there" with - // a single, named conversion. It is more concise than std::memcpy, - // sidesteps clang-tidy's bugprone-multi-level-implicit-pointer- - // conversion check (no const-void* destination conversion in the - // expression), and compiles to the same single store. The slots are - // raw storage from `::operator new`; writing a `void*` through a - // `void**` lvalue is the well-defined implicit-object-creation path - // every Tier-1 toolchain accepts. + // Implicit free list per ADR-0009 §1, ascending address order. Each free + // slot stores the address of the next free slot in its own first + // `sizeof(void*)` bytes (via write_next — a plain store, or the safe-linked + // store when hardened); the last slot stores a null pointer. `stride` is the + // physical slot size (== block_size unless hardening reserves a guard word). + const std::size_t stride = slot_stride(block_size); auto* const base = static_cast(backing); for (std::size_t i = 0; i + 1U < block_count; ++i) { - void* const this_slot = base + (i * block_size); - void* const next_slot = base + ((i + 1U) * block_size); - *static_cast(this_slot) = next_slot; + void* const this_slot = base + (i * stride); + void* const next_slot = base + ((i + 1U) * stride); + write_next(this_slot, next_slot); +#if PBR_MEMORY_POOL_HARDENING + hardening_detail::on_init_slot(this_slot, block_size); +#endif } - void* const last_slot = base + ((block_count - 1U) * block_size); - *static_cast(last_slot) = nullptr; + void* const last_slot = base + ((block_count - 1U) * stride); + write_next(last_slot, nullptr); +#if PBR_MEMORY_POOL_HARDENING + hardening_detail::on_init_slot(last_slot, block_size); +#endif } } // namespace @@ -201,14 +391,18 @@ bool block_in_chunk(const memory_pool* pool, const void* backing, std::size_t co const auto base_addr = reinterpret_cast(backing); // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) const auto block_addr = reinterpret_cast(block); - const std::uintptr_t end_addr = base_addr + (pool->block_size_ * count); + // Slots are `stride` apart (== block_size_ unless hardening reserves a + // guard word — ADR-0043); the range and modulo checks must use the physical + // stride, not the user-visible block_size_. + const std::size_t stride = slot_stride(pool->block_size_); + const std::uintptr_t end_addr = base_addr + (stride * count); if (block_addr < base_addr) { return false; } if (block_addr >= end_addr) { return false; } - if (((block_addr - base_addr) % pool->block_size_) != 0U) { + if (((block_addr - base_addr) % stride) != 0U) { return false; } return true; @@ -255,10 +449,13 @@ bool grow_pool(memory_pool* pool) noexcept { if (add == 0U) { return false; } - if (would_overflow_product(add, pool->block_size_)) { + // Allocate by the physical stride (== block_size_ unless hardening reserves + // a guard word — ADR-0043), and guard that product against size_t overflow. + const std::size_t stride = slot_stride(pool->block_size_); + if (would_overflow_product(add, stride)) { return false; } - const std::size_t bytes = add * pool->block_size_; + const std::size_t bytes = add * stride; void* backing = nullptr; try { @@ -314,16 +511,28 @@ struct SingleThreadedPolicy { void* const block = pool->head_; // The returned block still carries the next-link in its first // sizeof(void*) bytes; that is fine — the slot is now user-owned - // and its contents are documented as indeterminate. - pool->head_ = *static_cast(block); + // and its contents are documented as indeterminate. read_next reveals + // the safe-linked link when hardened, else a plain load (ADR-0043). + pool->head_ = read_next(block); +#if PBR_MEMORY_POOL_HARDENING + hardening_detail::on_alloc(block, pool->block_size_); +#endif return block; } static void push_head(memory_pool* pool, void* block) noexcept { // Mirrors the *static_cast(slot) = ptr idiom used in - // initialize_free_list: store the current head into the block's - // first sizeof(void*) bytes, then make this block the new head. - *static_cast(block) = pool->head_; + // initialize_free_list, through write_next: store the current head into + // the block's first sizeof(void*) bytes, then make this block the new + // head. When hardened, on_free validates the guard (double-free / + // overflow) and poisons the payload first; a detected double-free is + // dropped rather than re-linked (ADR-0043). +#if PBR_MEMORY_POOL_HARDENING + if (!hardening_detail::on_free(block, pool->block_size_)) { + return; + } +#endif + write_next(block, pool->head_); pool->head_ = block; } }; @@ -345,13 +554,24 @@ struct MutexPolicy { } } void* const block = pool->head_; - pool->head_ = *static_cast(block); + pool->head_ = read_next(block); +#if PBR_MEMORY_POOL_HARDENING + hardening_detail::on_alloc(block, pool->block_size_); +#endif return block; } static void push_head(memory_pool* pool, void* block) noexcept { const std::lock_guard guard{pool->mutex_}; - *static_cast(block) = pool->head_; + // Hardening validation + poison run under the held lock; a detected + // double-free is dropped rather than re-linked (ADR-0043). The early + // return releases the lock on scope exit as usual. +#if PBR_MEMORY_POOL_HARDENING + if (!hardening_detail::on_free(block, pool->block_size_)) { + return; + } +#endif + write_next(block, pool->head_); pool->head_ = block; } }; @@ -377,19 +597,37 @@ struct LockFreePolicy { if (expected.ptr_ == nullptr) { return nullptr; } - void* const next = *static_cast(expected.ptr_); + // reveal_next, NOT read_next: this read is speculative — the slot + // is not owned until the CAS below succeeds, so another thread may + // have popped it and filled it with user data. Running the + // safe-link integrity check here would false-positive; a genuinely + // corrupt link is instead caught by on_alloc's guard check once the + // slot is owned (ADR-0043). + void* const next = reveal_next(expected.ptr_); const TaggedHead desired{next, expected.tag_ + 1U}; if (pool->head_.compare_exchange_weak(expected, desired, std::memory_order_acq_rel)) { +#if PBR_MEMORY_POOL_HARDENING + hardening_detail::on_alloc(expected.ptr_, pool->block_size_); +#endif return expected.ptr_; } } } static void push_head(memory_pool* pool, void* block) noexcept { + // The guard check + poison depend only on `block`, so run them once + // before the publish loop; the safe-linked next-link (write_next) + // depends on `expected` and is rewritten on each CAS retry. A detected + // double-free is dropped rather than re-linked (ADR-0043). +#if PBR_MEMORY_POOL_HARDENING + if (!hardening_detail::on_free(block, pool->block_size_)) { + return; + } +#endif TaggedHead expected = pool->head_.load(std::memory_order_relaxed); TaggedHead desired{}; do { - *static_cast(block) = expected.ptr_; + write_next(block, expected.ptr_); desired = TaggedHead{block, expected.tag_ + 1U}; } while (!pool->head_.compare_exchange_weak(expected, desired, std::memory_order_acq_rel)); } @@ -445,11 +683,14 @@ memory_pool_t* memory_pool_create(std::size_t block_size, std::size_t block_coun if (block_count == 0U) { return nullptr; } - if (would_overflow_product(block_size, block_count)) { + // Allocate by the physical stride (== block_size unless hardening reserves + // a trailing guard word — ADR-0043) and guard that product for overflow. + const std::size_t stride = slot_stride(block_size); + if (would_overflow_product(stride, block_count)) { return nullptr; } - const std::size_t total_bytes = block_size * block_count; + const std::size_t total_bytes = stride * block_count; // Step 1 — over-aligned contiguous backing for the slots (ADR-0009 §4). // The hand-managed owning pointer is unavoidable across the C ABI; @@ -642,7 +883,9 @@ const void* memory_pool_debug_free_list_next(const memory_pool_t* pool, const vo if (pool == nullptr || current == nullptr) { return nullptr; } - return *static_cast(current); + // read_next reveals the safe-linked next-link under hardening (else a plain + // load), so the diagnostic walk is correct in both configurations. + return read_next(current); } std::size_t memory_pool_debug_free_count(const memory_pool_t* pool) { @@ -659,7 +902,7 @@ std::size_t memory_pool_debug_free_count(const memory_pool_t* pool) { #endif while (slot != nullptr) { ++count; - slot = *static_cast(slot); + slot = read_next(slot); } return count; } @@ -670,6 +913,48 @@ std::size_t memory_pool_debug_free_count(const memory_pool_t* pool) { namespace it::d4np::memorypool { +#if PBR_MEMORY_POOL_HARDENING + +namespace { + +// The handler is noexcept (it is called from the pool's noexcept (de)allocate +// path). `operator<<` on std::cerr is technically permitted to throw, which the +// analyzer flags as an exception escaping a noexcept function; here it cannot +// matter — a throw would call std::terminate, and this handler unconditionally +// std::abort()s one line later, so both outcomes are the same loud crash. +// NOLINTNEXTLINE(bugprone-exception-escape) +void default_hardening_violation_handler(const char* kind, const void* block) noexcept { + // ADR-0043 / ADR-0012 — a hardening violation is a defined, loud failure: + // emit a diagnostic and abort. std::cerr (not a vararg printf) keeps the + // handler clang-tidy-clean. Tests install a recording handler to assert a + // violation without terminating the process. + std::cerr << "[pbr-memory-pool] hardening violation: " << kind << " at " << block << '\n'; + std::abort(); +} + +std::atomic& handler_slot() noexcept { + // A function-local static avoids a mutable namespace-scope global + // (cppcoreguidelines-avoid-non-const-global-variables) while keeping the + // handler process-wide and safe to swap from any thread. + static std::atomic slot{&default_hardening_violation_handler}; + return slot; +} + +} // namespace + +HardeningViolationHandler set_hardening_violation_handler(HardeningViolationHandler handler) noexcept { + if (handler == nullptr) { + handler = &default_hardening_violation_handler; + } + return handler_slot().exchange(handler, std::memory_order_acq_rel); +} + +HardeningViolationHandler hardening_violation_handler() noexcept { + return handler_slot().load(std::memory_order_acquire); +} + +#endif // PBR_MEMORY_POOL_HARDENING + Pool::Pool(std::size_t block_size, std::size_t block_count) : handle_(::memory_pool_create(block_size, block_count)) { // ADR-0016 §3 — the throwing construction path. Precondition // violations (ADR-0009 §2/§3) and backing-storage OOM both collapse diff --git a/src/main/cpp/it/d4np/memorypool/memory_pool.h b/src/main/cpp/it/d4np/memorypool/memory_pool.h index 210e32b..ca162b2 100644 --- a/src/main/cpp/it/d4np/memorypool/memory_pool.h +++ b/src/main/cpp/it/d4np/memorypool/memory_pool.h @@ -68,6 +68,22 @@ #endif /* NOLINTEND(cppcoreguidelines-macro-usage) */ +/* + * Opt-in debug-hardening gate (ADR-0043). When 1, the intrusive free list + * gains freed-block poisoning, a per-slot guard word (overflow + double-free + * detection), and next-pointer safe-linking; see pool_hardening.hpp. OFF by + * default — a release build is byte-for-byte unchanged. Set library-wide at + * build time via the PBR_MEMORY_POOL_HARDENING CMake option (defined PUBLIC, + * because a hardened build changes the on-disk free-list layout, so the + * library and every consumer must agree). Preprocessor gate → a macro, not a + * constexpr constant; hence the cppcoreguidelines-macro-usage suppression. + */ +/* NOLINTBEGIN(cppcoreguidelines-macro-usage) */ +#ifndef PBR_MEMORY_POOL_HARDENING +#define PBR_MEMORY_POOL_HARDENING 0 +#endif +/* NOLINTEND(cppcoreguidelines-macro-usage) */ + #ifdef __cplusplus extern "C" { #endif diff --git a/src/main/cpp/it/d4np/memorypool/pool_hardening.hpp b/src/main/cpp/it/d4np/memorypool/pool_hardening.hpp new file mode 100644 index 0000000..95ab243 --- /dev/null +++ b/src/main/cpp/it/d4np/memorypool/pool_hardening.hpp @@ -0,0 +1,84 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 Daniel Polo + +#ifndef IT_D4NP_MEMORYPOOL_POOL_HARDENING_HPP_ +#define IT_D4NP_MEMORYPOOL_POOL_HARDENING_HPP_ + +/** + * @file pool_hardening.hpp + * @brief Opt-in debug-hardening surface for the free list — ADR-0043. + * + * When the library is built with `PBR_MEMORY_POOL_HARDENING` (a compile-time + * knob, OFF by default — see the CMake option of the same name), the pool's + * intrusive free list gains three self-contained protections against the + * classic use-after-free / pointer-corruption primitives an intrusive free + * list exposes (ADR-0009 §1): + * + * 1. **Freed-block poisoning** — a freed block's payload is filled with a + * recognizable byte pattern; a write to freed memory is caught on the next + * allocation of that block (use-after-free). + * 2. **Guard word** — a trailing guard word per slot detects a contiguous + * write past `block_size` (buffer overflow) and a repeated free of the same + * block (double-free), deterministically. + * 3. **Free-list safe-linking** — the in-band next-pointer is stored XORed with + * a per-slot key (glibc's `PROTECT_PTR`/`REVEAL_PTR`), so a leaked or + * overwritten next-link is neither directly usable nor silently followed; + * corruption surfaces as an alignment fault on reveal. + * + * Release builds are byte-for-byte and cycle-for-cycle unchanged — the entire + * mechanism is compiled out when the knob is off, and this header is then a + * no-op. A hardened build changes the on-disk free-list encoding **and** the + * physical slot stride, so it is deliberately **not** memory-layout-compatible + * with a non-hardened build: never mix the two configurations. + * + * This header exposes only the *violation policy* hook. On a detected + * violation the library calls the installed handler; the default handler + * prints a diagnostic and calls `std::abort()` (the ADR-0012 "defined, + * loud failure" stance). Tests install a recording handler so a violation can + * be asserted without terminating the process. + */ + +#include + +#if PBR_MEMORY_POOL_HARDENING + +namespace it::d4np::memorypool { + +/** + * @brief Handler invoked when the hardening layer detects a violation. + * + * @param kind A stable, static string naming the violation — one of the + * `HARDENING_*` constants below. + * @param block Address of the offending block (for the diagnostic). + * + * The handler is `noexcept`: it is called from the pool's `noexcept` + * allocate/deallocate path. The default handler does not return (it aborts); + * a handler that *does* return lets the operation continue on a best-effort, + * no-further-corruption path (used by the tests). + */ +using HardeningViolationHandler = void (*)(const char* kind, const void* block) noexcept; + +/** A write to a freed block's poisoned payload was detected on allocation. */ +inline constexpr const char* HARDENING_USE_AFTER_FREE = "use-after-free"; +/** A contiguous write past `block_size` corrupted the trailing guard word. */ +inline constexpr const char* HARDENING_OVERFLOW = "buffer-overflow"; +/** The same block was freed twice (its guard still read as freed). */ +inline constexpr const char* HARDENING_DOUBLE_FREE = "double-free"; +/** A free-list slot's guard or next-link was corrupted (integrity check). */ +inline constexpr const char* HARDENING_FREELIST_CORRUPTION = "free-list-corruption"; + +/** + * Install @p handler as the hardening violation handler and return the + * previous one. Passing `nullptr` restores the default (diagnostic + abort). + * Thread-safe. Present only in hardened builds. + */ +HardeningViolationHandler set_hardening_violation_handler(HardeningViolationHandler handler) noexcept; + +/** @return The currently installed hardening violation handler. */ +[[nodiscard]] HardeningViolationHandler hardening_violation_handler() noexcept; + +} // namespace it::d4np::memorypool + +#endif // PBR_MEMORY_POOL_HARDENING + +#endif // IT_D4NP_MEMORYPOOL_POOL_HARDENING_HPP_ diff --git a/src/test/cpp/it/d4np/memorypool/CMakeLists.txt b/src/test/cpp/it/d4np/memorypool/CMakeLists.txt index ab55a8c..ec730ae 100644 --- a/src/test/cpp/it/d4np/memorypool/CMakeLists.txt +++ b/src/test/cpp/it/d4np/memorypool/CMakeLists.txt @@ -222,6 +222,31 @@ add_test( set_tests_properties(pool_memory_resource PROPERTIES LABELS "smoke;milestone-9") +# ROADMAP item 9.2 — opt-in debug hardening (ADR-0043). The hardening surface +# is gated behind PBR_MEMORY_POOL_HARDENING, which the library defines PUBLIC +# (unlike the PRIVATE thread-safety macro), so linking pbr::memory_pool +# propagates the gate to this target automatically — no cache-variable mirror is +# needed. The test TU compiles in both states (a placeholder case when the +# surface is gated out, mirroring free_list_iterator_test), so this binary +# builds in every configuration and, under the `harden` preset, proves that a +# use-after-free, a buffer overflow past block_size, and a double-free are each +# detected deterministically. +add_executable(pool_hardening_test + pool_hardening_test.cpp) + +target_link_libraries(pool_hardening_test PRIVATE + pbr::memory_pool + doctest::doctest) + +target_compile_features(pool_hardening_test PRIVATE cxx_std_17) + +add_test( + NAME pool_hardening + COMMAND pool_hardening_test) + +set_tests_properties(pool_hardening PROPERTIES + LABELS "smoke;milestone-9") + # ROADMAP item 2.8 — spec §6.2 Valgrind demonstrative test. The directory # carries its own CMakeLists.txt because the target consumes a strict ANSI # C89 .c source and overrides the project-wide C99 floor on its own scope. diff --git a/src/test/cpp/it/d4np/memorypool/free_list_iterator_test.cpp b/src/test/cpp/it/d4np/memorypool/free_list_iterator_test.cpp index 91d0260..5577a34 100644 --- a/src/test/cpp/it/d4np/memorypool/free_list_iterator_test.cpp +++ b/src/test/cpp/it/d4np/memorypool/free_list_iterator_test.cpp @@ -67,12 +67,19 @@ TEST_CASE("FreeListView walks every slot of a fresh pool in ascending, strided o CHECK(static_cast(std::distance(view.begin(), view.end())) == BLOCK_COUNT); // ADR-0009 §1 — a fresh pool initialises the list in ascending address - // order, so slot i sits exactly i * block_size above the head, and all - // addresses are distinct. + // order at a fixed physical slot stride, so slot i sits exactly i strides + // above the head, and all addresses are distinct. The stride equals + // block_size in a default build; the opt-in hardening build (ADR-0043) + // widens it by a trailing guard word, so derive it from the first gap + // rather than assuming block_size — the walked-order property is what this + // case asserts, not the exact stride. const std::uintptr_t head = to_uint(slots.front()); + const std::uintptr_t stride = (slots.size() > 1U) ? (to_uint(slots.at(1)) - head) : BLOCK_SIZE; + CHECK(stride >= BLOCK_SIZE); + CHECK((stride % alignof(std::max_align_t)) == 0U); std::set distinct; for (std::size_t i = 0; i < slots.size(); ++i) { - CHECK(to_uint(slots.at(i)) == head + (i * BLOCK_SIZE)); + CHECK(to_uint(slots.at(i)) == head + (i * stride)); distinct.insert(slots.at(i)); } CHECK(distinct.size() == BLOCK_COUNT); diff --git a/src/test/cpp/it/d4np/memorypool/pool_hardening_test.cpp b/src/test/cpp/it/d4np/memorypool/pool_hardening_test.cpp new file mode 100644 index 0000000..ae342da --- /dev/null +++ b/src/test/cpp/it/d4np/memorypool/pool_hardening_test.cpp @@ -0,0 +1,182 @@ +// SPDX-License-Identifier: MIT +// Copyright (c) 2026 Daniel Polo + +/** + * @file pool_hardening_test.cpp + * @brief Tests for the opt-in debug hardening mode (M9.2 / ADR-0043). + * + * The suite is gated behind `PBR_MEMORY_POOL_HARDENING` so the binary still + * builds (with a single placeholder case) in the default, non-hardened build. + * When hardening is on, the cases install a *recording* violation handler (in + * place of the default diagnostic-and-abort one) so a detected violation can be + * asserted without terminating the process, and prove that: + * - correct alloc/free cycles raise no violation (no false positives); + * - a write to a freed block's poisoned payload is caught on the next + * allocation (use-after-free); + * - a contiguous write past `block_size` is caught on free (buffer overflow); + * - freeing the same block twice is caught (double-free). + */ + +#define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN + +#include +#include + +#include +#include +#include +#include + +// doctest is included last (after the project headers and the standard +// library), mirroring the sibling test TUs: doctest forward-declares a few +// std types (incl. `std::tuple`) in namespace std, so the real headers must be +// seen first or a newer toolchain rejects the redeclaration. +#include + +#if PBR_MEMORY_POOL_HARDENING + +using it::d4np::memorypool::Pool; +namespace hv = it::d4np::memorypool; + +namespace { + +constexpr std::size_t BLOCK_SIZE = 64U; +constexpr std::size_t BLOCK_COUNT = 8U; + +struct Violation { + const char* kind_; + std::size_t count_; +}; + +// Function-local static (not a namespace-scope global) records the handler's +// last observation across a single test's operations. +Violation& last_violation() noexcept { + static Violation state{nullptr, 0U}; + return state; +} + +void recording_handler(const char* kind, const void* /*block*/) noexcept { + last_violation().kind_ = kind; + ++last_violation().count_; +} + +// Reset the record and install the recording handler. Called at the top of +// each case so a detected violation is captured instead of aborting. +void arm() noexcept { + last_violation() = Violation{nullptr, 0U}; + hv::set_hardening_violation_handler(&recording_handler); +} + +} // namespace + +TEST_CASE("correct alloc/free cycles raise no hardening violation") { + arm(); + Pool pool(BLOCK_SIZE, BLOCK_COUNT); + + std::vector blocks; + for (std::size_t i = 0; i < BLOCK_COUNT; ++i) { + blocks.push_back(pool.allocate()); + } + for (void* const block : blocks) { + pool.deallocate(block); + } + // Recycle once more — every slot goes free -> allocated -> free cleanly. + for (std::size_t i = 0; i < BLOCK_COUNT; ++i) { + void* const block = pool.allocate(); + pool.deallocate(block); + } + + CHECK(last_violation().count_ == 0U); +} + +TEST_CASE("use-after-free is detected on the next allocation") { + arm(); + Pool pool(BLOCK_SIZE, BLOCK_COUNT); + + void* const block = pool.allocate(); + pool.deallocate(block); + + // Scribble on the freed block's poisoned payload (past the in-band + // next-link). This is the use-after-free the poison exists to catch. + auto* const bytes = static_cast(block); + bytes[sizeof(void*)] = 0x00U; + + // Re-allocating pops the same slot (LIFO); on_alloc verifies the poison and + // finds it broken. + void* const reused = pool.allocate(); + CHECK(last_violation().count_ >= 1U); + REQUIRE(last_violation().kind_ != nullptr); + CHECK(std::strcmp(last_violation().kind_, hv::HARDENING_USE_AFTER_FREE) == 0); + + pool.deallocate(reused); +} + +TEST_CASE("a contiguous write past block_size is detected on free") { + arm(); + Pool pool(BLOCK_SIZE, BLOCK_COUNT); + + void* const block = pool.allocate(); + + // Write one byte past the usable block_size — into the trailing guard word. + auto* const bytes = static_cast(block); + bytes[BLOCK_SIZE] = 0xFFU; + + pool.deallocate(block); // on_free finds the guard neither allocated nor free + CHECK(last_violation().count_ >= 1U); + REQUIRE(last_violation().kind_ != nullptr); + CHECK(std::strcmp(last_violation().kind_, hv::HARDENING_OVERFLOW) == 0); +} + +TEST_CASE("a double free is detected") { + arm(); + Pool pool(BLOCK_SIZE, BLOCK_COUNT); + + void* const block = pool.allocate(); + pool.deallocate(block); // legitimate free — guard stamped free + pool.deallocate(block); // second free of the same block — guard still free + + CHECK(last_violation().count_ >= 1U); + REQUIRE(last_violation().kind_ != nullptr); + CHECK(std::strcmp(last_violation().kind_, hv::HARDENING_DOUBLE_FREE) == 0); + + // No-further-corruption contract (ADR-0043): the rejected double-free must + // NOT re-link the already-freed block, so the free list still holds exactly + // BLOCK_COUNT distinct slots. Draining the pool must yield BLOCK_COUNT + // unique pointers and then exhaust — a duplicate or a self-cycle would show + // up as a repeated pointer or an over-count. The loop is bounded so a + // regression that cycles the list cannot hang the test. + std::vector drained; + for (std::size_t i = 0; i < BLOCK_COUNT + 1U; ++i) { + // try_allocate (noexcept, nullptr on exhaustion) — not allocate(), + // which throws std::bad_alloc when the pool is drained (ADR-0016). + void* const slot = pool.try_allocate(); + if (slot == nullptr) { + break; + } + drained.push_back(slot); + } + CHECK(drained.size() == BLOCK_COUNT); + const std::set distinct(drained.begin(), drained.end()); + CHECK(distinct.size() == drained.size()); +} + +TEST_CASE("the violation handler is swappable and restorable") { + const hv::HardeningViolationHandler previous = hv::set_hardening_violation_handler(&recording_handler); + CHECK(hv::hardening_violation_handler() == &recording_handler); + // Passing nullptr restores the default handler. + hv::set_hardening_violation_handler(nullptr); + CHECK(hv::hardening_violation_handler() != &recording_handler); + // Leave the previous handler as we found it. + hv::set_hardening_violation_handler(previous); +} + +#else // PBR_MEMORY_POOL_HARDENING + +TEST_CASE("debug hardening is compiled out in this build") { + // The default (non-hardened) build compiles the mechanism out entirely + // (ADR-0043); this placeholder keeps the binary and its CTest registration + // valid, mirroring the free_list_iterator_test pattern. + CHECK(true); +} + +#endif // PBR_MEMORY_POOL_HARDENING From cda57ebd8fff6f329b770c35a29d2d5ffbe2f64b Mon Sep 17 00:00:00 2001 From: Daniel Polo <106583643+danielPoloWork@users.noreply.github.com> Date: Thu, 9 Jul 2026 19:11:59 +0200 Subject: [PATCH 2/3] fix(pool): mark read_next [[maybe_unused]] for the LOCKFREE build Under the LOCKFREE thread-safety policy the pop path uses reveal_next (the integrity-check-free reveal for its speculative read), so the shared read_next helper has no caller unless the diagnostic surface is also compiled in. A LOCKFREE + diagnostics-off build (the `bench` preset policy-smoke cell) therefore tripped -Werror=unused-function on GCC. MSVC's equivalent (C4505) is off by default, so a local MSVC build did not surface it. [[maybe_unused]] is the portable suppression. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/main/cpp/it/d4np/memorypool/memory_pool.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main/cpp/it/d4np/memorypool/memory_pool.cpp b/src/main/cpp/it/d4np/memorypool/memory_pool.cpp index c9e99ca..40c50c7 100644 --- a/src/main/cpp/it/d4np/memorypool/memory_pool.cpp +++ b/src/main/cpp/it/d4np/memorypool/memory_pool.cpp @@ -239,7 +239,12 @@ void* reveal_next(const void* slot) noexcept { // single-thread and mutex pops hold exclusivity and the diagnostic walk runs on // a stable list. The lock-free pop uses reveal_next instead (see above) and // defers integrity to on_alloc's guard-word check once the slot is owned. -void* read_next(const void* slot) noexcept { +// +// [[maybe_unused]]: under the LOCKFREE policy the pop path uses reveal_next, so +// read_next has no caller unless the diagnostic surface is also compiled in +// (PBR_MEMORY_POOL_DIAGNOSTICS). Without this, a LOCKFREE + diagnostics-off +// build trips -Wunused-function under warnings-as-errors. +[[maybe_unused]] void* read_next(const void* slot) noexcept { void* const revealed = reveal_next(slot); #if PBR_MEMORY_POOL_HARDENING // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) From 69b21b3d5ce4e38dd704f81709011d311ae07b3b Mon Sep 17 00:00:00 2001 From: Daniel Polo <106583643+danielPoloWork@users.noreply.github.com> Date: Thu, 9 Jul 2026 19:18:23 +0200 Subject: [PATCH 3/3] fix(tidy): satisfy the clang-tidy diff gate on the touched files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings the CI diff gate surfaces on the files this branch touches (they had lain latent on master since the tidy gate last ran on memory_pool.cpp — the intervening PR was header-only): - initialize_free_list's two size_t parameters trip bugprone-easily-swappable-parameters. Suppress it as the create / factory entry points already do (they mirror the same frozen argument order), rather than diverging into a config struct. - The FreeListView walk test crossed the cognitive-complexity threshold (41 > 40) once the stride-derivation and its assertions were added. Extract the stride derivation into a derive_stride helper and move the stride/alignment assertions into their own focused TEST_CASE, returning the walk case to its prior complexity while keeping full coverage. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../cpp/it/d4np/memorypool/memory_pool.cpp | 5 +++ .../memorypool/free_list_iterator_test.cpp | 38 ++++++++++++++++--- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/main/cpp/it/d4np/memorypool/memory_pool.cpp b/src/main/cpp/it/d4np/memorypool/memory_pool.cpp index 40c50c7..a483615 100644 --- a/src/main/cpp/it/d4np/memorypool/memory_pool.cpp +++ b/src/main/cpp/it/d4np/memorypool/memory_pool.cpp @@ -270,6 +270,11 @@ void write_next(void* slot, void* next) noexcept { #endif } +// The two size_t parameters (block_size, block_count) mirror the frozen +// memory_pool_create argument order; a config struct would diverge from that +// established shape, so the swappable-parameters check is suppressed here as it +// is on the create/factory entry points. +// NOLINTNEXTLINE(bugprone-easily-swappable-parameters) void initialize_free_list(void* backing, std::size_t block_size, std::size_t block_count) noexcept { // Implicit free list per ADR-0009 §1, ascending address order. Each free // slot stores the address of the next free slot in its own first diff --git a/src/test/cpp/it/d4np/memorypool/free_list_iterator_test.cpp b/src/test/cpp/it/d4np/memorypool/free_list_iterator_test.cpp index 5577a34..c0d248d 100644 --- a/src/test/cpp/it/d4np/memorypool/free_list_iterator_test.cpp +++ b/src/test/cpp/it/d4np/memorypool/free_list_iterator_test.cpp @@ -51,6 +51,17 @@ std::uintptr_t to_uint(const void* ptr) noexcept { return reinterpret_cast(ptr); } +// The physical slot stride, derived from the first gap between slots rather +// than assumed to equal block_size: a default build strides by BLOCK_SIZE, the +// opt-in hardening build (ADR-0043) widens it by a trailing guard word. +// Extracting it keeps the walk case below the cognitive-complexity threshold. +std::uintptr_t derive_stride(const std::vector& slots) noexcept { + if (slots.size() < 2U) { + return BLOCK_SIZE; + } + return to_uint(slots.at(1)) - to_uint(slots.front()); +} + } // namespace TEST_CASE("FreeListView walks every slot of a fresh pool in ascending, strided order") { @@ -70,13 +81,10 @@ TEST_CASE("FreeListView walks every slot of a fresh pool in ascending, strided o // order at a fixed physical slot stride, so slot i sits exactly i strides // above the head, and all addresses are distinct. The stride equals // block_size in a default build; the opt-in hardening build (ADR-0043) - // widens it by a trailing guard word, so derive it from the first gap - // rather than assuming block_size — the walked-order property is what this - // case asserts, not the exact stride. + // widens it by a trailing guard word (see derive_stride) — the walked-order + // property is what this case asserts, not the exact stride. const std::uintptr_t head = to_uint(slots.front()); - const std::uintptr_t stride = (slots.size() > 1U) ? (to_uint(slots.at(1)) - head) : BLOCK_SIZE; - CHECK(stride >= BLOCK_SIZE); - CHECK((stride % alignof(std::max_align_t)) == 0U); + const std::uintptr_t stride = derive_stride(slots); std::set distinct; for (std::size_t i = 0; i < slots.size(); ++i) { CHECK(to_uint(slots.at(i)) == head + (i * stride)); @@ -85,6 +93,24 @@ TEST_CASE("FreeListView walks every slot of a fresh pool in ascending, strided o CHECK(distinct.size() == BLOCK_COUNT); } +TEST_CASE("the free-list slot stride is block_size, or a wider aligned hardening stride") { + // The physical stride equals block_size in a default build and widens by a + // trailing guard word under the opt-in hardening build (ADR-0043); either + // way it stays a multiple of max_align_t so every slot start is aligned. + Pool pool(BLOCK_SIZE, BLOCK_COUNT); + const FreeListView view(pool); + + std::vector slots; + for (const void* const slot : view) { + slots.push_back(slot); + } + REQUIRE(slots.size() == BLOCK_COUNT); + + const std::uintptr_t stride = derive_stride(slots); + CHECK(stride >= BLOCK_SIZE); + CHECK((stride % alignof(std::max_align_t)) == 0U); +} + TEST_CASE("allocation shrinks the walked free list") { Pool pool(BLOCK_SIZE, BLOCK_COUNT);