Fix ast/base.py hash issues

[zwimer]

This fixes multiple issues:

1. Non-integer (and non-str) ._hash values for ASTs. Notice how

   claripy/claripy/ast/base.py

   Line 226 in 51d1753

   h = (op, kwargs.get('length', None), a_args)

   can propagate to

   claripy/claripy/ast/base.py

   Line 239 in 51d1753

   self._hash = h

   
   Fundamentally, if ._hash is never used as a hash of the AST, this isn't an issue; i.e. if only hash(my_ast) is used it is ok because we do convert in that function on demand. The problem is, that ._hash is used as the hash.
2. Differing behavior if annotations exist / do not. At this if statement

   claripy/claripy/ast/base.py

   Line 218 in 51d1753

   elif op in {'BVS', 'BVV', 'BoolS', 'BoolV', 'FPS', 'FPV'} and not annotations:

   notice we have special cases for floats like fixing the problem that hash(0.0) == hash(-0.0) here:

   claripy/claripy/ast/base.py

   Line 221 in 51d1753

   h = (op, kwargs.get('length', None), ("-",) + a_args)

   But this only happens if not annotations.
3. Fix anything that uses _calc_hash to determine what key to enter into a hash cache for looking up existing ASTs; since for some cases our cache uses the tuples made via our fast-path here:

   claripy/claripy/ast/base.py

   Line 218 in 51d1753

   elif op in {'BVS', 'BVV', 'BoolS', 'BoolV', 'FPS', 'FPV'} and not annotations:

   Ex:

   claripy/claripy/ast/base.py

   Line 253 in 51d1753

   self = cache.get(h, None)

4. Reduce code duplication between Base.__new__ and Base.__init_with_annotations__.
5. Fix a bug in Base.__init_with_annotations__ where we didn't use the leaf cache at all and instead were looking for leaves in the non-leaf cache:

   claripy/claripy/ast/base.py

   Line 251 in 51d1753

   cache = cls._hash_cache

6. Non-reproducible hashes; since self._hash was not always calculated via _calc_hash but sometimes as a tuple (see point 1).
7. When deserializing via base.py's _d function

   claripy/claripy/ast/base.py

   Line 58 in 51d1753

   def _d(h, cls, state):

   this bypasses the leaf cache and incorrectly stores the leaves in the non-leaf cache.
8. Serialization is incomplete; it looses information such as +0.0 vs -0.0; thus deserialization may be incorrect.
9. (Unknown if applicable) If _d always takes an integer (or str) as a hash, deserialization will lead to ASTs with corrupt _hashs because of point 1.

Before merging:

• Add fastpath for annotation-free children (this was removed temporarily to get hashing work first)

Linked: angr/binaries#101

[github-actions]

Unit Test Results

     94 files  +     84       94 suites  +84   1h 49m 17s ⏱️ + 1h 48m 49s
1 436 tests +1 130  1 342 ✔️ +1 096  90 💤 +30  0 ❌ ±0  4 🔥 +4 
1 442 runs  +1 136  1 348 ✔️ +1 102  90 💤 +30  0 ❌ ±0  4 🔥 +4 

For more details on these errors, see this check.

Results for commit 17f9151. ± Comparison against base commit adbf300.

♻️ This comment has been updated with latest results.

[zardus]

__new_with_annotations__?

[zwimer]

That is a better name for this function, in my opinion. If we want to change that I'll need to see where else it is used and do a synced change

[zwimer]

Ping @ltfish Regenerate all caches please

[ltfish]

The reason why you couldn't regenerate caches locally: https://github.com/angr/claripy/blob/fix/hash-fail/claripy/ast/base.py#L250

Can you fix it (by removing the breakpoint() call) and then try generating caches across all projects again?

[ltfish]

ROP caches updated. You will want to rebase this branch on master to re-trigger the CI.

[zwimer]

@ltfish The same issues I was seeing before still seem to occur in the CI.

[ltfish]

@zwimer what issue?

[ltfish]

The four decompiler test cases fail because the fix/hash-fail branch in angr/binaries misses some new binaries that I pushed over the weekend. So they are totally expected.

[ltfish]

You should only pay attention to the "push" tests since "PR" tests do not synchronize repos based on the same branch name.

[zwimer]

@ltfish The push CI is failing too: https://github.com/angr/claripy/actions/runs/3841788720/jobs/6576714321

[ltfish]

@ltfish The push CI is failing too: https://github.com/angr/claripy/actions/runs/3841788720/jobs/6576714321

That's exactly what my reply was about.