fix(common): make getDetailed span-consistent for WAIT-prefixed x87 instructions#134
Open
r0ny123 wants to merge 2 commits into
Open
fix(common): make getDetailed span-consistent for WAIT-prefixed x87 instructions#134r0ny123 wants to merge 2 commits into
r0ny123 wants to merge 2 commits into
Conversation
…nstructions SmdaInstruction.getDetailed() blindly returned the last Capstone instruction when the stored bytes decoded to more than one. For WAIT-prefixed x87 forms (e.g. `9bd93c24` -> `wait` + `fnstcw word ptr [esp]`, recorded as one instruction by SMDA/IDA) this happened to land on the right detail but did so by accident and warned on every occurrence. Select the detail deliberately: drop standalone WAIT/FWAIT prefix instructions and return the trailing x87 operation, whose operands and (address + size) match the stored byte span. The known prefix pattern is now silent; only genuinely unexpected splits warn. Replace the strip-able `assert` with an explicit ValueError for the zero-instruction case. Closes danielplohmann#114 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adversarial self-review caught a cross-file regression from changing getDetailed() to raise ValueError on undecodable bytes (replacing the old strip-able assert): escapeBinaryPtrRef() only caught (AttributeError, AssertionError, NotImplementedError) to gracefully default its pack_format, so the new ValueError would propagate and crash escaping for an instruction with a [disp] operand whose stored bytes cannot be decoded. Add ValueError to that except tuple and a regression test. NON_OPERATIONAL_EXCEPTION_TYPES is intentionally left unchanged: an undecodable byte span is an operational data issue, not a programming error, so treating it as swallowable during analysis is correct. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Addresses #114.
SmdaInstruction.getDetailed()re-disassembles the stored instruction bytes with Capstone, which can split a single SMDA/IDA instruction into two. The classic case is a WAIT-prefixed x87 op:The old code took
with_details[-1]and logged a warning every time. That happened to return the right detail here, but only by luck, and it would mis-select for any non-trailing split.Fix
wait/fwaitprefix instructions, then return the trailing operation instruction. Its operands and itsaddress + sizeline up with the end of the stored byte span (0x4d3f1f1 + 3 == 0x4d3f1f0 + 4), so the returned detail is span-consistent.assert len(...) == 1(strippable underpython -O) with an explicitValueErrorfor the zero-instruction case.Tests
New
tests/testSmdaInstructionDetail.py:9bd93c24regression: returnsfnstcw, carries the memory operand,address + sizereaches the span end, and emits no warningValueErrorNotImplementedErrormake testgreen locally (incl.testIntegration, which exercisesgetDetailed()via data-ref recovery);ruffclean.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests