match BinExport2 instructions using patterns #2353
Conversation
add pruning phase to expression tree building to remove known-bad branches. This might address some of the data we're seeing due to: NationalSecurityAgency/ghidra#6821 Also introduces a --instruction optional argument to dump the details of a specific instruction.
williballenthin
force-pushed
the
push-xmurxlmxyyyn
branch
5 times, most recently
from
d15bca5
to
99767ff
Compare
| return | ||
|
|
||
| instruction: BinExport2.Instruction = be2.instruction[ii.instruction_index] | ||
| # guaranteed to be simple int/reg operands | ||
| # so we don't have to realize the tree/list. |
There was a problem hiding this comment.
nice, great comments here throughout!
There was a problem hiding this comment.
This is great @williballenthin . I've left some small comments for your review but overall this is a big improvement to the nested checks that we were using. LGTM 🚀
tests/test_binexport_accessors.py
Outdated
There was a problem hiding this comment.
Thank you for all of the tests! This helps breakdown what the various functions are supposed to do.
| children_tree_indexes: List[int] = expression_tree[tree_index] | ||
|
|
||
| if expression.type == BinExport2.Expression.OPERATOR: | ||
| if len(children_tree_indexes) == 0 and expression.symbol in ("lsl", "lsr"): |
There was a problem hiding this comment.
Do we need to handle other extended register operands? see https://devblogs.microsoft.com/oldnewthing/20220727-00/?p=106907
| # - #int | ||
| # - #int0 | ||
| # and a limited set of supported operators. | ||
| assert re.match(r"^(reg|#int)[0-9]?(\(stack\)|\(not-stack\))?$", o) or o in ("[", ",", "!", "+", "*") |
There was a problem hiding this comment.
Any concern with a performance hit from repeated regex matching here?
There was a problem hiding this comment.
This is only used when parsing the patterns, which we expect to happen once at startup, O(1). I figured it wasn't worth optimizing that path (such a tiny impact) versus line count/context here. Would have to move the regex declaration out of the function, which makes it harder to follow as a human.
| """ | ||
| ldr|ldrb|ldrh|ldrsb|ldrsh|ldrex|ldrd|str|strb|strh|strex|strd reg, [reg(not-stack), #int] ; capture #int | ||
| ldr|ldrb|ldrh|ldrsb|ldrsh|ldrex|ldrd|str|strb|strh|strex|strd reg, [reg(not-stack), #int]! ; capture #int | ||
| ldr|ldrb|ldrh|ldrsb|ldrsh|ldrex|ldrd|str|strb|strh|strex|strd reg, [reg(not-stack)], #int ; capture #int | ||
| ldp|ldpd|stp|stpd reg, reg, [reg(not-stack), #int] ; capture #int | ||
| ldp|ldpd|stp|stpd reg, reg, [reg(not-stack), #int]! ; capture #int | ||
| ldp|ldpd|stp|stpd reg, reg, [reg(not-stack)], #int ; capture #int | ||
| """ |
williballenthin
force-pushed
the
push-xmurxlmxyyyn
branch
from
99767ff
to
7302be5
Compare
Introduce intruction pattern matching to declaratively describe the instructions and operands that we want to extract. While there's a bit more code, its much more thoroughly tested, and is less brittle than the prior if/else/if/else/if/else implementation.
williballenthin
force-pushed
the
push-xmurxlmxyyyn
branch
from
7302be5
to
5d2ebcb
Compare
This PR against the BinExport2 PR introduces a way to match instructions and extract expression values. We use this mini-language to specify how to extract features from BinExport2 instructions, such as Number, Offset, nzxor, etc. While there's a fair amount of code (~500 lines, with tests) to implement this matching language, it is:
To understand all the changes here, I'd recommend reading:
lsl #0expressions that have no effect. We prune these from the expression tree.Checklist