feat(scanner): add function prologue helper - #82
Conversation
Cheap first-byte blacklist (0x00, 0xCC, 0xC2, 0xC3) gated by Memory::is_readable, with null-address short-circuit. Rejects scan poison (zero pages, alignment pads, bare RET stubs) while still accepting JMP-shaped patched prologues so nested-hook scenarios resolve. Covered by eight ScannerPrologueTest cases including a load-bearing 0xE9 case that pins the no-interference-with-nested-hooks contract.
📝 WalkthroughWalkthroughThis PR introduces ChangesScanner Prologue Detection Helper
🎯 2 (Simple) | ⏱️ ~12 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.
🧹 Nitpick comments (1)
tests/test_scanner.cpp (1)
2127-2204: ⚡ Quick winAdd explicit tests for
0xEBand0xFF 0x25accepted prologue forms.The helper docs promise acceptance for short/indirect JMP-shaped starts, but this suite currently only pins the
0xE9case. Adding both cases will prevent silent regressions in that documented contract.Proposed test additions
+TEST(ScannerPrologueTest, PatchedJmpEbReturnsTrue) +{ + ExecBuffer buf(0x1000); + ASSERT_NE(buf.base, nullptr); + std::memset(buf.base, 0xCC, buf.size); + buf.base[0x100] = 0xEB; + EXPECT_TRUE(Scanner::is_likely_function_prologue( + reinterpret_cast<std::uintptr_t>(buf.base + 0x100))); +} + +TEST(ScannerPrologueTest, PatchedJmpFf25ReturnsTrue) +{ + ExecBuffer buf(0x1000); + ASSERT_NE(buf.base, nullptr); + std::memset(buf.base, 0xCC, buf.size); + buf.base[0x100] = 0xFF; + buf.base[0x101] = 0x25; + EXPECT_TRUE(Scanner::is_likely_function_prologue( + reinterpret_cast<std::uintptr_t>(buf.base + 0x100))); +}🤖 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_scanner.cpp` around lines 2127 - 2204, Add two new unit tests mirroring the existing PatchedJmpE9ReturnsTrue case to cover short JMP 0xEB and indirect JMP opcode pair 0xFF 0x25: create tests (e.g., PatchedJmpEBReturnsTrue and PatchedJmpFF25ReturnsTrue) that allocate an ExecBuffer, fill with 0xCC, write 0xEB at offset 0x100 for the short-jmp case and write 0xFF then 0x25 at offset 0x100 for the indirect-jmp case, and assert EXPECT_TRUE(Scanner::is_likely_function_prologue(reinterpret_cast<std::uintptr_t>(buf.base + 0x100))); to ensure the Scanner::is_likely_function_prologue behavior for these documented prologue forms is covered.
🤖 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.
Nitpick comments:
In `@tests/test_scanner.cpp`:
- Around line 2127-2204: Add two new unit tests mirroring the existing
PatchedJmpE9ReturnsTrue case to cover short JMP 0xEB and indirect JMP opcode
pair 0xFF 0x25: create tests (e.g., PatchedJmpEBReturnsTrue and
PatchedJmpFF25ReturnsTrue) that allocate an ExecBuffer, fill with 0xCC, write
0xEB at offset 0x100 for the short-jmp case and write 0xFF then 0x25 at offset
0x100 for the indirect-jmp case, and assert
EXPECT_TRUE(Scanner::is_likely_function_prologue(reinterpret_cast<std::uintptr_t>(buf.base
+ 0x100))); to ensure the Scanner::is_likely_function_prologue behavior for
these documented prologue forms is covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6c755ee9-54f2-40f5-8f5f-a9a8ae6a7249
📒 Files selected for processing (5)
README.mddocs/misc/aob-signatures.mdinclude/DetourModKit/scanner.hppsrc/scanner.cpptests/test_scanner.cpp
Summary
Adds
Scanner::is_likely_function_prologue(addr)for filtering scan poison after a cascade resolves. Cheap first-byte blacklist (0x00,0xCC,0xC2,0xC3) gated byMemory::is_readable, with a null-address short-circuit. Accepts0xE9/0xEB/0xFF 0x25so nested-hook scenarios (a sibling mod has already overwritten the prologue with a JMP trampoline) still resolve.Test plan
ScannerPrologueTest(8 cases) green: null, zero byte, int3 pad, both bare-RET forms, push rbp, patched JMP,PAGE_NOACCESSDocs
README scanner bullet,
docs/misc/aob-signatures.md7.3 and troubleshooting row updated to point at the helper. Also wrapped the previously-loose "Config Hot-Reload" section in a<details>block so it matches the other feature panels.Summary by CodeRabbit
New Features
Documentation
Tests