feat(anchors): declarative self-healing anchor registry - #97
Conversation
Resolve every patch-fragile constant a mod depends on from a module range in one declarative pass, unifying the self-healing backends behind a uniform value+status report: - VtableIdentity: reverse name-to-vtable RTTI resolvers (vtable_for_type, vtables_for_type, TypeIdentity) - RipGlobal: module-scoped AOB/RIP cascade (resolve_cascade_in_module) - CodeOperand: Zydis-backed in-code constant decode (read_code_constant) - Manual: pinned literal, surfaced as at-risk in the report Split the cascade resolver into scanner_cascade.cpp (shared with the scan engine via scanner_internal.hpp). Add test suites for each surface and document the registry.
|
Warning Review limit reached
More reviews will be available in 21 minutes and 36 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR introduces a declarative anchor registry system that consolidates patch-fragile constants into a single table-driven resolver, backed by four independent resolution backends: code-constant operand extraction (Zydis decoding), RTTI reverse vtable lookup by mangled type name, RIP/global cascade scanning, and manual literal pinning. Supporting infrastructure includes RTTI healing telemetry reporting, scanner cascade refactoring, and comprehensive test coverage. ChangesCode-constant operand extraction and anchor consolidation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
tests/test_code_constant.cpp (1)
35-41: ⚡ Quick winMark
CodeRegiondestructor asnoexcept.Please make the destructor signature explicit (
~CodeRegion() noexcept) to match the project’s exception contract for teardown paths.Suggested patch
- ~CodeRegion() + ~CodeRegion() noexcept { if (m_base) { VirtualFree(m_base, 0, MEM_RELEASE); } }As per coding guidelines: "Do not omit
noexcepton destructors, shutdown methods, and const accessors."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_code_constant.cpp` around lines 35 - 41, The destructor for CodeRegion is missing noexcept; update its signature from ~CodeRegion() to ~CodeRegion() noexcept in the CodeRegion class/implementation so teardown cannot throw (adjust both declaration and definition if they are separate), ensuring the destructor matches the project's exception contract and coding guidelines.Source: Coding guidelines
src/scanner_cascade.cpp (1)
66-77: ⚡ Quick winUse the repo's constant naming here.
These new TU-scope constants introduce
kCamelCase, but the project guideline for C++ constants isUPPER_SNAKE_CASE. Renaming them now avoids spreading a second style through the new scanner cascade code.As per coding guidelines, "Use
UPPER_SNAKE_CASEfor constants and macros."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/scanner_cascade.cpp` around lines 66 - 77, Rename the TU-scope constants to the repo's UPPER_SNAKE_CASE style and update all references: change kPrologueFallbackMinTailLiterals to PROLOGUE_FALLBACK_MIN_TAIL_LITERALS and kPrologueFallbackMaxHits to PROLOGUE_FALLBACK_MAX_HITS in src/scanner_cascade.cpp (and any other translation units that reference them), and ensure builds/tests still pass after updating usages in functions/methods that rely on these constants (e.g., any logic that checks the minimum tail literals or the fallback max hits).Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/misc/aob-signatures.md`:
- Around line 478-489: The Contents/TOC at the top of the document was not
updated after adding sections 6.6 ("Host-module convenience overloads") and 6.7
("Reading a code constant (read_code_constant)"); update the contents list to
add entries for these two headings (with matching anchor texts/section numbers),
and verify the anchor links match the exact heading text so navigation works
correctly.
- Around line 493-495: The example AOB in k_stride_site contains an invalid
token "..." which parse_aob will reject; update the entry in k_stride_site to
replace "..." with explicit byte tokens or wildcards (e.g., full hex byte pairs
or "??" for unknown bytes) so the pattern string conforms to parse_aob's allowed
tokens; locate the static constexpr sc::AddrCandidate k_stride_site[] definition
and fix the pattern for "equip-stride" to use concrete bytes/wildcards only (no
ellipses), ensuring the resulting pattern parses correctly by parse_aob.
In `@src/rtti.cpp`:
- Around line 495-497: The fixed-size buffer in scan_vtables_for_name is too
small: change ScanRange ranges[32] to accommodate the full capacity expected by
collect_rtti_scan_ranges (up to 96) — e.g., replace the 32 with 96 or allocate
dynamically based on the max accepted by collect_rtti_scan_ranges — so that
range_count = collect_rtti_scan_ranges(mod, ranges, 96) cannot overflow/clip
sections and miss RTTI; update any accompanying size constant/argument usages to
keep both the buffer and the call-site limit consistent (symbols: ScanRange
ranges, scan_vtables_for_name, collect_rtti_scan_ranges, range_count).
In `@src/scanner_cascade.cpp`:
- Around line 36-56: resolve_candidate_match currently returns 0 on an
unreadable displacement which scan_candidates still treats as a valid hit,
letting invalid matches produce a ResolveHit at address 0; change
resolve_candidate_match to return an optional (e.g.
std::optional<std::uintptr_t>) instead of raw uintptr_t and return std::nullopt
on failure (when DetourModKit::Memory::seh_read fails), then update
scan_candidates to check the optional before constructing a ResolveHit
(skip/continue when nullopt) so non-Direct AddrCandidate failures do not produce
a false hit; update all call sites (including the other occurrence referenced at
lines 273-286) to handle the optional result.
- Around line 238-247: The lambda scan_for currently always calls
Scanner::detail::scan_module_readable when range is set, which breaks
resolve_cascade_in_module's intent to keep module-scoped cascades
executable-only; change scan_for to respect the scanner kind: if range is set
and kind == DetourModKit::Scanner::ScannerKind::Readable call
Scanner::detail::scan_module_readable(*compiled, *range, occurrence), otherwise
call the module-executable counterpart (e.g.,
Scanner::detail::scan_module_executable or the appropriate detail function that
scans executable pages) so executable cascades remain limited to .text; make the
same conditional fix at the other occurrence noted (lines ~487-490).
In `@tests/test_anchors.cpp`:
- Around line 34-40: The Region class destructor is missing a noexcept
specification; update the destructor signature for ~Region() to be noexcept to
comply with the project's rule for destructors and RAII cleanup, keeping the
body that checks m_base and calls VirtualFree(m_base, 0, MEM_RELEASE) unchanged
so the cleanup behavior is preserved.
---
Nitpick comments:
In `@src/scanner_cascade.cpp`:
- Around line 66-77: Rename the TU-scope constants to the repo's
UPPER_SNAKE_CASE style and update all references: change
kPrologueFallbackMinTailLiterals to PROLOGUE_FALLBACK_MIN_TAIL_LITERALS and
kPrologueFallbackMaxHits to PROLOGUE_FALLBACK_MAX_HITS in
src/scanner_cascade.cpp (and any other translation units that reference them),
and ensure builds/tests still pass after updating usages in functions/methods
that rely on these constants (e.g., any logic that checks the minimum tail
literals or the fallback max hits).
In `@tests/test_code_constant.cpp`:
- Around line 35-41: The destructor for CodeRegion is missing noexcept; update
its signature from ~CodeRegion() to ~CodeRegion() noexcept in the CodeRegion
class/implementation so teardown cannot throw (adjust both declaration and
definition if they are separate), ensuring the destructor matches the project's
exception contract and coding guidelines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 376d94ba-76a6-44b6-a2c8-9edd4307ad93
📒 Files selected for processing (23)
AGENTS.mdREADME.mddocs/misc/anchors.mddocs/misc/aob-signatures.mddocs/misc/rtti-self-heal.mddocs/misc/rtti-walker.mdinclude/DetourModKit.hppinclude/DetourModKit/anchors.hppinclude/DetourModKit/rtti.hppinclude/DetourModKit/rtti_dissect.hppinclude/DetourModKit/scanner.hppsrc/anchors.cppsrc/code_constant.cppsrc/rtti.cppsrc/rtti_dissect.cppsrc/scanner.cppsrc/scanner_cascade.cppsrc/scanner_internal.hpptests/test_anchors.cpptests/test_code_constant.cpptests/test_rtti_dissect.cpptests/test_rtti_reverse.cpptests/test_scanner.cpp
resolve_candidate_match now returns std::optional; a faulted displacement read is a miss (nullopt) instead of address 0, which the whole-process cascade path would otherwise accept as a ResolveHit (the module-scoped path was already guarded by Memory::contains). Also: rename the prologue-fallback constants to UPPER_SNAKE_CASE per AGENTS.md; add TOC entries for sections 6.6/6.7 and replace the invalid "..." token in the read_code_constant example with a wildcard in aob-signatures.md.
Summary
Adds a declarative
Anchorsregistry that resolves a mod's patch-fragile constants from a module range in one pass, reporting a uniform{label, kind, status, value}per entry (the drift report). It unifies DMK's self-healing backends behind one table:AnchorKindVtableIdentityRipGlobalCodeOperandManualCallArgHomeSupporting work
Summary by CodeRabbit
New Features
Documentation