Skip to content

Canonicalize commutative operations before simplification - #726

Draft
twizmwazin wants to merge 1 commit into
masterfrom
feat/commutative-canonicalization
Draft

Canonicalize commutative operations before simplification#726
twizmwazin wants to merge 1 commit into
masterfrom
feat/commutative-canonicalization

Conversation

@twizmwazin

Copy link
Copy Markdown
Member

Summary

Sorts the arguments of commutative operations by hash in simplify() so that expressions differing only in operand order canonicalize to the same AST:

  • x + y and y + x produce the same expression
  • And(a, b) and And(b, a) produce the same expression

This improves expression sharing and simplification hit rates. Ported from the feat/better-canonicalization branch (just the commutative-canonicalization change — independent of #725).

Change

if op in operations.commutative_operations:
    args = tuple(sorted(args, key=lambda x: x.hash()))

plus the from claripy import operations import.

Testing

Verified canonicalization behavior locally (x + y is y + x, And(x==1, y==2) is And(y==2, x==1)). Running the full angr test suite against this branch is in progress; will follow up with results.

🤖 Generated with Claude Code

Sort the arguments of commutative operations by hash in simplify() so
that expressions that differ only in operand order (e.g. x + y and y + x,
or And(a, b) and And(b, a)) canonicalize to the same AST. This improves
expression sharing and simplification hit rates.

Ported from the feat/better-canonicalization branch.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@twizmwazin

Copy link
Copy Markdown
Member Author

⚠️ angr test results: this change causes 12 regressions

Ran the full angr test suite against this branch and triaged each failure against master (baseline):

12 tests pass on baseline but fail with this change — caused by the commutative arg reordering:

  • tests/analyses/cfg/test_jumptables.py::TestJumpTableResolver::test_amd64_win_jumptable_with_secondary_redirections
  • tests/analyses/cfg/test_jumptables.py::TestJumpTableResolver::test_s390x_cfgswitches
  • tests/analyses/cfg/test_cfgfast.py::TestCfgfast::test_cfg_switches_s390x
  • tests/analyses/test_calling_convention_analysis.py::...::test_cdecl_nonconsecutive_stack_args_2
  • tests/analyses/test_calling_convention_analysis.py::...::test_cdecl_nonconsecutive_stack_args_3
  • tests/engines/test_unicorn.py::TestUnicorn::test_similarity_fauxware
  • tests/exploration_techniques/test_oppologist.py::TestOppologist::test_fauxware_oppologist
  • tests/exploration_techniques/test_stochastic.py::TestRun::test_stochastic
  • tests/procedures/libc/test_string.py::...::test_strstr_sym_haystack_sym_needle
  • tests/sim/test_fauxware.py::TestFauxware::test_fastmem
  • tests/test_cli.py::TestCommandLineInterface::test_base_addr_dec
  • tests/test_cli.py::TestCommandLineInterface::test_decompiling

2 pre-existing flakies (fail on baseline too, float-precision asserts, unrelated): test_manyfloatsum_symbolic_i386, test_manyfloatsum_symbolic_x86_64.

Several angr analyses (jumptable resolution, calling-convention recovery, decompilation) pattern-match on expression argument order, so reordering commutative operands during simplification breaks them. This likely needs corresponding fixes in angr before it can land — flagging rather than merging as-is.

@twizmwazin
twizmwazin marked this pull request as draft June 6, 2026 00:52
@angr-bot

angr-bot commented Jun 6, 2026

Copy link
Copy Markdown
Member

Corpus decompilation diffs can be found at angr/dec-snapshots@master...angr/claripy_726

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants