feat(rtti): add reverse RTTI dissection and self-healing offset resolver - #95
Conversation
Builds rtti_dissect.hpp on the shared SEH-guarded COL prelude: identify_pointee_type, reverse_scan_block, heal_landmark/heal_offset, and solve_fingerprint. Every entry point is noexcept and fails closed. Adds the gtest suite and the self-heal guide.
|
Warning Review limit reached
More reviews will be available in 32 minutes and 20 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 (4)
📝 WalkthroughWalkthroughThis PR introduces a new RTTI dissection and self-healing module. A shared RTTI ABI infrastructure is extracted into an internal header, existing RTTI code is refactored to use shared helpers, and a comprehensive four-layer API is declared and implemented to identify types, scan blocks for labeled slots, heal field offsets via landmark templates, and solve uniform drift via fingerprinting. Tests and documentation complete the addition. ChangesRTTI Dissection and Offset Healing
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: 3
🧹 Nitpick comments (1)
tests/test_rtti_dissect.cpp (1)
3-18: ⚡ Quick winAdd the standard headers this file uses directly.
This TU uses
std::min,std::is_trivially_copyable_v, andstd::span, but only gets them transitively today. That makes the test brittle against header cleanup ingtestorDetourModKit/rtti_dissect.hpp.Suggested fix
+#include <algorithm> `#include` <array> `#include` <atomic> `#include` <cstddef> `#include` <cstdint> `#include` <cstdlib> `#include` <cstring> `#include` <new> +#include <span> `#include` <string> `#include` <string_view> +#include <type_traits> `#include` <vector>🤖 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_rtti_dissect.cpp` around lines 3 - 18, Add direct includes for the standard headers used by this translation unit: include <algorithm> for std::min, <type_traits> for std::is_trivially_copyable_v, and <span> for std::span at the top of tests/test_rtti_dissect.cpp so the TU does not rely on transitive includes; update the include block near the existing headers (before using any symbols) to ensure functions and type traits used in this file are declared.
🤖 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 `@src/rtti_dissect.cpp`:
- Around line 99-102: The check in descriptor_ok(const Rtti::Landmark &lm)
incorrectly rejects names whose length equals Rtti::MAX_TYPE_NAME_LEN; change
the condition that currently uses ">= Rtti::MAX_TYPE_NAME_LEN" to only reject
lengths greater than MAX_TYPE_NAME_LEN (i.e., use ">" or allow equality) so
Landmark::expected_mangled of length MAX_TYPE_NAME_LEN is accepted; update
descriptor_ok accordingly to match the bounds supported by read_name_seh() and
PointeeType::name_buf.
In `@src/rtti.cpp`:
- Around line 70-86: The code currently only enforces the p_self checks when
head_opt->signature == COL_SIGNATURE_X64 and allows other signatures to fall
through; change the logic in the block handling head_opt->signature so that any
signature other than COL_SIGNATURE_X64 is treated as invalid (return false)
instead of continuing to use the loader/base path — specifically, in the
function that inspects head_opt (referencing head_opt, COL_SIGNATURE_X64,
p_self, p_type_descriptor, mod_range, Memory::contains and TD_NAME_OFFSET), add
an explicit reject branch for non-x64 signatures (return false) before computing
td_addr/name_addr so malformed or unknown signatures cannot be accepted and feed
into type_name_* or the reverse dissector.
In `@tests/test_rtti_dissect.cpp`:
- Around line 237-244: The test helper unmapped_addr currently returns 0 if
VirtualAlloc fails, causing Identify_RejectsUnreadableSlotAddress to exercise
the null-address guard instead of the unreadable-memory path; update
unmapped_addr to abort the test on allocation failure (e.g., use a test fatal
failure/assert like EXPECT_NE/ASSERT_NE or GTEST_FAIL) so it never returns 0 on
failure, and apply the same change to the other identical helper at the 366-370
location; reference the unmapped_addr function and the
Identify_RejectsUnreadableSlotAddress test when making the change.
---
Nitpick comments:
In `@tests/test_rtti_dissect.cpp`:
- Around line 3-18: Add direct includes for the standard headers used by this
translation unit: include <algorithm> for std::min, <type_traits> for
std::is_trivially_copyable_v, and <span> for std::span at the top of
tests/test_rtti_dissect.cpp so the TU does not rely on transitive includes;
update the include block near the existing headers (before using any symbols) to
ensure functions and type traits used in this file are declared.
🪄 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: a73025ea-fa40-4632-84dc-116163006d44
📒 Files selected for processing (9)
AGENTS.mdREADME.mddocs/misc/rtti-self-heal.mdinclude/DetourModKit.hppinclude/DetourModKit/rtti_dissect.hppsrc/rtti.cppsrc/rtti_dissect.cppsrc/rtti_internal.hpptests/test_rtti_dissect.cpp
Harden the shared prelude so an unknown or corrupt signature cannot skip the pSelf cross-check and fall through on the loader base. Add the direct includes the dissect test relies on, guard the unmapped-address precondition, and unwrap the self-heal guide prose.
Summary
rtti_dissect.hpp: reverse RTTI dissection (identify_pointee_type,reverse_scan_block) and a self-healing offset resolver (heal_landmark/heal_offset,solve_fingerprint) built on the walker's shared, SEH-guarded COL prelude.noexceptand fails closed; ships with a full gtest suite and thedocs/misc/rtti-self-heal.mdguide.Summary by CodeRabbit
New Features
Documentation
Tests