Skip to content

Commit f96d4aa

Browse files
authored
refactor: namespace-block migration and verified audit fixes (#129)
1 parent 1d9be21 commit f96d4aa

14 files changed

Lines changed: 6297 additions & 5997 deletions

AGENTS.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ scripts/ # Repo tooling (check_comment_style.py: advisory commen
190190
Follow the file you are in: match its established short-name convention rather than introducing a lone outlier, and do not leave a cryptic domain `a`/`r`/`c` in new code whose neighbours are spelled out.
191191
- **Braces:** Allman style -- opening brace on its own line for functions and classes, same line for control flow within a function body using K&R-style indented blocks. Single-statement control bodies are not required to be braced: the formatter's `InsertBraces` is intentionally left unset, so a brace-less guard clause (`if (cond) return;`) is fine; brace any multi-line or non-obvious body.
192192
- **Indentation:** 4 spaces, no tabs.
193-
- **Namespaces:** All public API lives in `namespace DetourModKit`. No `using namespace` in headers. In `.cpp` files, wrap the library's own definitions in an explicit `namespace DetourModKit { ... }` block rather than a file-scope `using namespace DetourModKit;`; the block form is the house default (the few remaining files that still open with a file-scope `using namespace DetourModKit;` are tolerated and should migrate to the block form when next touched). Every closing namespace brace must have a trailing comment: `} // namespace DetourModKit`. Implementation-only statics in `.cpp` files must be in an anonymous namespace (not a named internal namespace) to guarantee internal linkage.
193+
- **Namespaces:** All public API lives in `namespace DetourModKit`. No `using namespace` in headers. In `.cpp` files, wrap the library's own definitions in an explicit `namespace DetourModKit { ... }` block rather than a file-scope `using namespace DetourModKit;`; the block form is the house default and is used by every `.cpp` in the library; no translation unit opens with a file-scope `using namespace DetourModKit;`. Every closing namespace brace must have a trailing comment: `} // namespace DetourModKit`. Implementation-only statics in `.cpp` files must be in an anonymous namespace (not a named internal namespace) to guarantee internal linkage.
194194
- **Include guards:** All headers use `#ifndef DETOURMODKIT_<MODULE>_HPP` / `#define` / `#endif` guards (not `#pragma once`). Guard names must be prefixed with `DETOURMODKIT_` to avoid collisions with consumer projects.
195195
- **Includes:** Project headers use `"DetourModKit/header.hpp"`. System/external headers use `<angle brackets>`. Group: project headers, then external, then standard library.
196196

@@ -390,7 +390,7 @@ These are called at 60+ fps from game hook callbacks. Never add allocations, exc
390390
- `Logger::log()` level check -- single atomic load
391391
- `Logger::log()` async enqueue -- atomic shared_ptr load + lock-free queue push
392392
- `Memory::is_readable()` -- sharded SRWLOCK reader + cache lookup
393-
- `Memory::is_readable_nonblocking()` -- try_lock_shared + cache lookup (returns Unknown on contention)
393+
- `Memory::is_readable_nonblocking()` -- try_lock_shared + cache lookup (returns Unknown on lock contention, a cache miss, or the init-publication window; falls back to a blocking VirtualQuery before `init_cache()`)
394394
- `Memory::read_ptr_unsafe()` -- SEH-protected raw dereference (MSVC), vectored-handler-guarded read on MinGW (no per-call VirtualQuery; the fault guard also closes the stale-cache dereference)
395395
- `Memory::read_ptr_unchecked()` -- inline pointer dereference with source and result user-mode range guards (a low-address floor plus a `USERSPACE_PTR_MAX` ceiling that rejects kernel-range and non-canonical values, which also subsumes pointer-arithmetic wraparound), no SEH (caller must guarantee structural pointer validity); debug builds add an `is_readable` assert that catches a stale or unmapped source pointer, compiled out in release so the hot path stays a single guarded memcpy
396396
- `Memory::seh_read<T>()` / `seh_read_bytes()` -- typed and raw SEH-guarded reads; single `__try` frame on MSVC, and on MinGW a single `rep movsb` copy under a process-wide vectored exception handler (installed lazily / by `init_cache`, removed by `shutdown_cache`) that recovers via a non-unwinding `__builtin_setjmp`/`__builtin_longjmp`, so the success path runs no syscall. Both toolchains swallow the same foreign-read fault set -- `EXCEPTION_ACCESS_VIOLATION`, `STATUS_GUARD_PAGE_VIOLATION`, and `EXCEPTION_IN_PAGE_ERROR` (a file-backed or image-mapped page failing to page in, e.g. during an RTTI / section walk) -- via the shared predicate `Memory::detail::is_guarded_read_fault`, and let any other fault continue the handler search; the MinGW handler additionally claims only faults whose address lies in the foreign range being read. A guarded read uses a single per-thread guard, so it must not nest on one thread (DMK reads are synchronous, so it does not); if `AddVectoredExceptionHandler` ever fails the reads fall back to VirtualQuery validation. Used by `Rtti` for chained RTTI walks
@@ -407,7 +407,7 @@ These are called at 60+ fps from game hook callbacks. Never add allocations, exc
407407

408408
- **Do not modify** files under `external/` -- these are git submodules.
409409
- **Do not add** `using namespace` directives in header files.
410-
- **Prefer** an explicit `namespace DetourModKit { ... }` block over a file-scope `using namespace DetourModKit;` in `.cpp` files (the block form is the majority house style).
410+
- **Use** an explicit `namespace DetourModKit { ... }` block, never a file-scope `using namespace DetourModKit;`, in `.cpp` files. The block form is the house style across every translation unit.
411411
- **Do not add** heap allocations on hot paths (see list above).
412412
- **Do not break** the lock ordering documented in class headers.
413413
- **Do not weaken** atomic memory orderings without proving correctness.
@@ -437,4 +437,5 @@ These are called at 60+ fps from game hook callbacks. Never add allocations, exc
437437
- **Do not let** a queue or backlog fed at an external event rate grow without bound -- clamp the pending count to a documented ceiling (see `Input` wheel-notch `MAX_WHEEL_PENDING`).
438438
- **Do not let** an async/deferred sink diverge from its synchronous counterpart's configuration (timestamp format, level, etc.) -- carry the same settings through, as `Logger::enable_async_mode` does for the timestamp format.
439439
- **Do not open** a shared append-mode file with `GENERIC_WRITE` plus a one-time `SetFilePointer(FILE_END)` seek -- request `FILE_APPEND_DATA` so the OS positions every write at the current end of file atomically and concurrent appenders cannot interleave or overwrite each other (see `WinFileStreamBuf::open`). Truncating (`out`) mode keeps `GENERIC_WRITE` + `CREATE_ALWAYS`.
440+
- **Do not assume** a single `WriteFile` drains the whole request -- it can report success with `bytes_written < count` (a short write on a pipe, a near-full volume, or an interrupted write). Loop over the unwritten remainder so a buffered tail is never silently dropped, resetting the put area only after the buffer fully drains or the write hard-fails (see `WinFileStreamBuf::flush_buffer`).
440441
- **Do not drop** the device source when formatting an off-table input code -- a non-keyboard code must serialize in the source-tagged hex form (`Mouse:0xNN`) that `parse_input_name` reads back, or it silently round-trips to a keyboard key (see `format_input_code` / `parse_input_name`).

include/DetourModKit/async_logger.hpp

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,23 @@
1919

2020
namespace DetourModKit
2121
{
22+
/// Default capacity (slot count) of the bounded MPMC message queue.
2223
inline constexpr size_t DEFAULT_QUEUE_CAPACITY = 8192;
24+
/// Default number of messages the writer drains per write batch.
2325
inline constexpr size_t DEFAULT_BATCH_SIZE = 64;
26+
/// Default interval between periodic writer flushes.
2427
inline constexpr auto DEFAULT_FLUSH_INTERVAL = std::chrono::milliseconds(100);
28+
/// Maximum length, in bytes, of a single log message.
2529
inline constexpr size_t MAX_MESSAGE_SIZE = 16777216;
30+
/// Default spin-backoff iteration count before a producer yields/parks.
2631
inline constexpr size_t DEFAULT_SPIN_BACKOFF_ITERATIONS = 32;
32+
/// Default timeout for a blocking flush to complete.
2733
inline constexpr auto DEFAULT_FLUSH_TIMEOUT = std::chrono::milliseconds(500);
34+
/// Byte size of one StringPool overflow block.
2835
inline constexpr size_t MEMORY_POOL_BLOCK_SIZE = 4096;
36+
/// Number of overflow blocks the StringPool preallocates.
2937
inline constexpr size_t MEMORY_POOL_BLOCK_COUNT = 64;
38+
/// Number of allocation slots carved from each StringPool block.
3039
inline constexpr size_t POOL_SLOTS_PER_BLOCK = 16;
3140

3241
/**
@@ -116,7 +125,11 @@ namespace DetourModKit
116125
/**
117126
* @struct LogMessage
118127
* @brief A log entry with inline buffer optimization and overflow handling.
119-
* @details Messages <= 512 bytes are stored inline. Larger messages use heap allocation via StringPool.
128+
* @details Messages <= 512 bytes are stored inline. Larger messages use heap allocation via StringPool. This is a
129+
* move-only transport/value type (copy deleted, move defined) carried by value through the queue; it
130+
* keeps plain field names rather than the m_ member prefix, per the POD-struct naming convention, even
131+
* though @ref overflow is an owned heap pointer -- its lifetime is self-contained, released by reset()
132+
* and the destructor, so no class invariant rides on member encapsulation.
120133
*/
121134
struct LogMessage
122135
{

include/DetourModKit/memory.hpp

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,18 @@ namespace DetourModKit
5454
}
5555
}
5656

57-
// Maximum write size for write_bytes (64 MiB)
57+
/// Maximum write size for write_bytes (64 MiB).
5858
inline constexpr size_t MAX_WRITE_SIZE = 64 * 1024 * 1024;
5959

60-
// Memory cache configuration defaults
60+
/// Default number of region entries the memory cache holds.
6161
inline constexpr size_t DEFAULT_CACHE_SIZE = 256;
62+
/// Default cache entry lifetime, in milliseconds, before a re-query.
6263
inline constexpr unsigned int DEFAULT_CACHE_EXPIRY_MS = 50;
64+
/// Minimum permitted cache size.
6365
inline constexpr size_t MIN_CACHE_SIZE = 1;
66+
/// Default number of cache shards, striped to reduce reader contention.
6467
inline constexpr size_t DEFAULT_CACHE_SHARD_COUNT = 16;
68+
/// Default multiplier bounding the cache's maximum size relative to its configured size.
6569
inline constexpr size_t DEFAULT_MAX_CACHE_SIZE_MULTIPLIER = 2;
6670

6771
namespace Memory
@@ -188,11 +192,18 @@ namespace DetourModKit
188192

189193
/**
190194
* @brief Non-blocking readability check that avoids contention on shared locks.
191-
* @details Attempts a try-lock on the cache shard. Returns Unknown if the lock cannot be acquired, allowing
192-
* callers on latency-sensitive threads to fall back to SEH instead of stalling.
195+
* @details Non-blocking only once init_cache() has published the cache: it tries a shared try-lock on the
196+
* owning shard and answers from the cached protection on a hit. It returns Unknown -- so a
197+
* latency-sensitive caller can fall back to an SEH-guarded read instead of stalling -- in three
198+
* cases: the shard try-lock is contended, the shard holds no live entry for the range (a cache
199+
* miss; it does not issue a blocking VirtualQuery here), or the cache is mid-publication (the
200+
* initialized flag is set but the shard array is not yet visible). Before init_cache() (or after
201+
* shutdown_cache()) there is no cache to consult, so it falls back to a single blocking
202+
* VirtualQuery and returns a definite Readable or NotReadable, never Unknown.
193203
* @param address Starting address of the memory region.
194204
* @param size Number of bytes in the memory region to check.
195-
* @return ReadableStatus indicating readable, not readable, or unknown (lock busy).
205+
* @return ReadableStatus: Readable, NotReadable, or Unknown (shard lock busy, cache miss, or the
206+
* init-publication window; Unknown is only returned once the cache is initialized).
196207
*/
197208
[[nodiscard]] ReadableStatus is_readable_nonblocking(const void *address, size_t size);
198209

include/DetourModKit/scanner.hpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -955,12 +955,19 @@ namespace DetourModKit
955955
* all-offset shape scan and additionally recognize the rarer RIP-relative shapes (`cmp [rip+d], imm`,
956956
* `push [rip+d]`, a no-REX `lea`/`mov`) via a Zydis-verified sweep. Either way, a shape the active scans
957957
* do not model reports NoReference rather than a guess.
958+
* @note Phase 2 scans each execute-readable window from the image independently and, unlike @ref
959+
* find_pattern, carries no cross-window overlap. A RIP-relative reference whose bytes span the
960+
* boundary between two separate executable windows (a protection split inside .text, say) is decoded
961+
* in neither and reports NoReference: a fail-closed miss, never a wrong match. The common case, one
962+
* contiguous executable section, has no interior boundary.
958963
* @note XrefReturn::StringPointerSlot requires the unique reference to be a REX.W `lea reg, [rip+string]` and
959964
* returns the effective address of the global slot the first `mov [rip+slot], reg` (same source register)
960965
* within a bounded forward window stores the loaded pointer into. It resolves a cached global string
961966
* pointer rather than the load site. A `mov reg, [rip+string]` load, a broad-only reference, or no
962-
* matching store reports StoreNotFound. The store match is first-within-window (compilers emit it next
963-
* to the load), not uniqueness-checked, and intervening reuse of the register is not modelled.
967+
* matching store reports StoreNotFound. The store match is first-within-window (compilers emit it
968+
* next to the load) and is not uniqueness-checked; the forward scan bails to StoreNotFound on a CALL,
969+
* on any write to the loaded register (at any width), or on a decode failure, so a clobbered or
970+
* post-call store is never misattributed.
964971
* @note Choose a string referenced exactly once (a long, specific literal such as a format or assert message);
965972
* short, common strings are pooled and shared and will report StringAmbiguous / AmbiguousReference.
966973
* @note StringEncoding::Utf16le widens each query byte to a little-endian 16-bit code unit (the byte

0 commit comments

Comments
 (0)