Fix merge_stack

Add unit tests
Replace #34
Partially fix #31

Author: montyly

.github/workflows/pytest.yml

@@ -0,0 +1,38 @@
+name: Pytest
+
+defaults:
+  run:
+    # To load bashrc
+    shell: bash -ieo pipefail {0}
+
+on:
+  push:
+    branches:
+      - main
+      - dev
+  pull_request:
+    branches: [main, dev]
+  schedule:
+    # run CI every day even if no PRs/merges occur
+    - cron:  '0 12 * * *'
+
+jobs:
+  tests:
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@v1
+    - name: Set up Python 3.6
+      uses: actions/setup-python@v1
+      with:
+        python-version: 3.6
+
+        # Used by ci_test.sh
+    - name: Install dependencies
+      run: |
+        python setup.py install
+        pip install pytest
+        pip install pytest-cov
+    - name: Run value set analysis tests
+      run: |
+        pytest python_tests/value_set_analysis.py

evm_cfg_builder/__main__.py

@@ -6,9 +6,9 @@
 import pstats
 import sys
 from typing import Optional, Union
+from pkg_resources import require
 
 from crytic_compile import cryticparser, CryticCompile, InvalidCompilation, is_supported
-from pkg_resources import require
 
 from evm_cfg_builder.cfg.cfg import CFG
 from evm_cfg_builder.known_hashes.known_hashes import known_hashes

evm_cfg_builder/cfg/function.py

@@ -169,14 +169,16 @@ def output_to_dot(self, base_filename: str) -> None:
                 instructions_ = [f"{hex(ins.pc)}:{str(ins)}" for ins in basic_block.instructions]
                 instructions = "\n".join(instructions_)
 
-                f.write(f'{basic_block.start.pc}[label="{instructions}"]\n')
+                f.write(f'{basic_block.start.pc}[label="{instructions}", shape=box]\n')
 
                 for son in basic_block.outgoing_basic_blocks(self.key):
                     f.write(f"{basic_block.start.pc} -> {son.start.pc}\n")
 
                 if not basic_block.outgoing_basic_blocks(self.key):
                     if basic_block.ends_with_jump_or_jumpi():
-                        logger.error(f"Missing branches {self.name}:{hex(basic_block.end.pc)}")
+                        logger.error(
+                            f"Missing branches {self.name} ({self.key}):{hex(basic_block.end.pc)}"
+                        )
 
             f.write("\n}")
 

evm_cfg_builder/value_analysis/value_set_analysis.py

@@ -39,12 +39,15 @@ class AbsStackElem:
 
     Thus our analysis is an under-approximation of an over-approximation
     and is not sound.
+
     """
 
     def __init__(
         self, auhtorized_values: Optional[Set[int]], vals: Optional[Set[Optional[int]]] = None
     ):
         if vals:
+            if auhtorized_values:
+                vals = {v if v in auhtorized_values else None for v in vals}
             self._vals: Optional[Set[Optional[int]]] = vals
         else:
             self._vals = set()
@@ -351,29 +354,26 @@ def merge_stack(stacks: List[Stack], authorized_values: Set[int]) -> Stack:
 
     _max_number_of_elements = len(authorized_values) if authorized_values else 100
 
-    found = True
-    i = 0
-    while found:
+    elemss = [stack.get_elems()[::-1] for stack in stacks]
+    max_depth = max(len(elems) for elems in elemss)
+
+    for i in range(max_depth):
         vals: Optional[Set[Optional[int]]] = set()
-        found = False
-        for stack in stacks:
-            elems = stack.get_elems()
-            if len(elems) <= i:
-                continue
-            found = True
-            next_vals = elems[i].get_vals()
-            if next_vals is None:
-                vals = None
-                break
-            assert vals is not None
-            vals |= next_vals
-            if len(vals) > _max_number_of_elements:
-                vals = None
-                break
+        for elems in elemss:
+            if len(elems) > i:
+                next_vals = elems[i].get_vals()
+                if next_vals is None:
+                    vals = None
+                    break
+                assert vals is not None
+                vals |= next_vals
+                if len(vals) > _max_number_of_elements:
+                    vals = None
+                    break
         stack_elements.append(AbsStackElem(authorized_values, vals))
-        i = i + 1
+
     newSt = Stack(authorized_values)
-    newSt.set_elems(stack_elements)
+    newSt.set_elems(stack_elements[::-1])
     return newSt
 
 
@@ -486,7 +486,6 @@ def _transfer_func_ins(self, ins: Instruction, addr: int, stack: Stack) -> Stack
         (is_stub, stub_ret) = self.stub(ins, addr, stack)
         if is_stub:
             return stub_ret
-
         op = ins.name
         if op.startswith("PUSH"):
             stack.push(ins.operand)

python_tests/__init__.py

@@ -0,0 +1,138 @@
+from typing import Set
+
+from evm_cfg_builder.value_analysis.value_set_analysis import (
+    merge_stack,
+    Stack,
+    AbsStackElem,
+)
+
+
+def test_merge_stack_1() -> None:
+    authorized_values = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}
+    st1 = Stack(authorized_values)
+
+    st1.push(1)
+    st1.push(2)
+    st1.push(10)
+    st1.push(3)
+
+    st2 = Stack(authorized_values)
+
+    st2.push(1)
+    st2.push(3)
+    st2.push(5)
+
+    st_merged = merge_stack([st1, st2], authorized_values)
+
+    st_real = Stack(authorized_values)
+    st_real.push(1)
+    st_real.push(AbsStackElem(authorized_values, {1, 2}))
+    st_real.push(AbsStackElem(authorized_values, {3, 10}))
+    st_real.push(AbsStackElem(authorized_values, {3, 5}))
+
+    print(st1)
+    print(st2)
+    print(st_merged)
+    print(st_real)
+    assert st_real.equals(st_merged)
+
+
+def test_merge_stack_2() -> None:
+    authorized_values = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}
+    st1 = Stack(authorized_values)
+
+    st1.push(1)
+    st1.push(AbsStackElem(authorized_values, {2, 4}))
+    st1.push(10)
+    st1.push(3)
+
+    st2 = Stack(authorized_values)
+
+    st2.push(1)
+    st2.push(3)
+    st2.push(5)
+
+    st_merged = merge_stack([st1, st2], authorized_values)
+
+    st_real = Stack(authorized_values)
+    st_real.push(1)
+    st_real.push(AbsStackElem(authorized_values, {1, 2, 4}))
+    st_real.push(AbsStackElem(authorized_values, {3, 10}))
+    st_real.push(AbsStackElem(authorized_values, {3, 5}))
+
+    print(st1)
+    print(st2)
+    print(st_merged)
+    print(st_real)
+    assert st_real.equals(st_merged)
+
+
+def test_merge_stack_no_authorized_value() -> None:
+    authorized_values: Set[int] = set()
+    st1 = Stack(authorized_values)
+
+    st1.push(1)
+    st1.push(2)
+    st1.push(3)
+    st1.push(4)
+    st1.push(5)
+
+    st2 = Stack(authorized_values)
+
+    st2.push(3)
+    st2.push(4)
+    st2.push(5)
+
+    st_merged = merge_stack([st1, st2], authorized_values)
+
+    st_real = Stack(authorized_values)
+    st_real.push(1)
+    st_real.push(2)
+    st_real.push(3)
+    st_real.push(4)
+    st_real.push(5)
+
+    print(st1)
+    print(st2)
+    print(st_merged)
+    print(st_real)
+    assert st_real.equals(st_merged)
+
+
+def test_pop_push() -> None:
+    authorized_values: Set[int] = set()
+    st1 = Stack(authorized_values)
+    st1.push(1)
+    st1.push(2)
+    assert st1.top().equals(AbsStackElem(authorized_values, {2}))
+
+
+def test_merge_stack_diff_size() -> None:
+    authorized_values: Set[int] = set()
+    st1 = Stack(authorized_values)
+    st2 = Stack(authorized_values)
+
+    st1.push(1)
+    st1.push(2)
+
+    st2.push(2)
+
+    st_merged = merge_stack([st1, st2], authorized_values)
+
+    st_real = Stack(authorized_values)
+    st_real.push(1)
+    st_real.push(2)
+
+    print(st1)
+    print(st2)
+    print(st_merged)
+    print(st_real)
+    assert st_real.equals(st_merged)
+
+
+if __name__ == "__main__":
+    test_pop_push()
+    test_merge_stack_diff_size()
+    test_merge_stack_1()
+    test_merge_stack_2()
+    test_merge_stack_no_authorized_value()