Skip to content

Conversation

bradlarsen
Copy link

Using typeguard in local test runs (see #1584), I found several issues where type hints in Manticore are in fact not accurate at runtime.

This pull request fixes nearly all of those, and additionally fixes a couple bugs.

Brad Larsen added 12 commits April 14, 2020 18:50
The `dirfd` and `buf` parameters can be symbolic, and the type-checking
that mypy does has trouble understanding `issymbolic`.  So, un-type-hintify
those parameters.
It turns out that the type of `data` in a couple methods is pretty flexible,
and our tests use more than just `str`, `bytes`, and `List` types for that
parameter.

It is not quite clear what requirements there actually are on `data`, and so at
least for now, get rid of the type hint there.
**kwargs could not have possibly worked in `AMD64Operand`.
Copy link
Contributor

@ekilmer ekilmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bradlarsen
Copy link
Author

Note: the CI / tests (ethereum_bench) check failed a couple times here before it succeeded. I don't think it's related to changes made in this PR, and I was also unable to reproduce the failure locally.

@bradlarsen
Copy link
Author

Actually, I can reproduce the test failure locally, sporadically. It's always this error:

tests/ethereum_bench/test_consensys_benchmark.py FE                                                              [100%]

======================================================== ERRORS ========================================================
_______________________ ERROR at teardown of EthBenchmark.test_integer_overflow_storageinvariant _______________________

self = <tests.ethereum_bench.test_consensys_benchmark.EthBenchmark testMethod=test_integer_overflow_storageinvariant>

    def tearDown(self):
>       self.mevm.finalize()

tests/ethereum_bench/test_consensys_benchmark.py:28:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
manticore/core/manticore.py:128: in newFunction
    return func(self, *args, **kw)
manticore/ethereum/manticore.py:1738: in finalize
    "{:x}: {:2.2f}%\n".format(int(address), self.global_coverage(address))
manticore/ethereum/manticore.py:1814: in global_coverage
    runtime_bytecode = state.solve_one(code)
manticore/core/state.py:344: in solve_one
    return self.solve_one_n(expr, constrain=constrain)[0]
manticore/core/state.py:362: in solve_one_n
    value = self._solver.get_value(self._constraints, expr)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <manticore.core.smtlib.solver.Z3Solver object at 0x7f32e7f27370>
constraints = <manticore.core.smtlib.constraints.ConstraintSet object at 0x7f32e7c23b50>
expressions = (<ArrayProxy at 7f32e7cd4590>,), values = [], start = 1586986530.1181638
temp_cs = <manticore.core.smtlib.constraints.ConstraintSet object at 0x7f32e7c93bb0>

    def get_value(self, constraints, *expressions):
        """
        Ask the solver for one possible result of given expressions using
        given set of constraints.
        """
        values = []
        start = time.time()
        with constraints as temp_cs:
            for expression in expressions:
                if not issymbolic(expression):
                    values.append(expression)
                    continue
                assert isinstance(expression, (Bool, BitVec, Array))
                if isinstance(expression, Bool):
                    var = temp_cs.new_bool()
                elif isinstance(expression, BitVec):
                    var = temp_cs.new_bitvec(expression.size)
                elif isinstance(expression, Array):
                    var = []
                    result = []
                    for i in range(expression.index_max):
                        subvar = temp_cs.new_bitvec(expression.value_bits)
                        var.append(subvar)
                        temp_cs.add(subvar == simplify(expression[i]))

                    self._reset(temp_cs.to_string())
                    if not self._is_sat():
>                       raise SolverError(
                            "Solver could not find a value for expression under current constraint set"
                        )
E                       manticore.exceptions.SolverError: Solver could not find a value for expression under current constraint set

manticore/core/smtlib/solver.py:622: SolverError

Copy link
Contributor

@ehennenfent ehennenfent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes and discussion look good to me!

@ehennenfent ehennenfent merged commit 52f215e into master Apr 17, 2020
@ehennenfent ehennenfent deleted the fix-type-hints branch April 17, 2020 19:08
ekilmer added a commit that referenced this pull request Apr 23, 2020
* master: (28 commits)
  Rework syscall invocation for proper behavior under typeguard (#1672)
  printable_bytes (#1671)
  Fix several incorrect type hints (#1668)
  Add type hints to several parts of Manticore (#1667)
  Fix type confusion in manual CPU tests; delete dead testing code (#1669)
  Rework WASM Imports and Fix Typos (#1666)
  Work on "Model is not Available" errors in tests (#1659)
  Fix TypeGuard Errors in WASM Module (#1601)
  Remove unused arg ans separate output from workspace (#1651)
  Add a badge to README.md for LGTM (#1647)
  Fix 2 problems in Linux `sys_open` support & add type hints (#1657)
  Fix LGTM Errors (#1656)
  Delay WASM Branch Condition Concretization (#1641)
  Add feature for SymbolicSocket through sys_accept (#1618)
  Add a bunch of type hints & fix a few issues with Linux platform emulation (#1645)
  Bump to CheckoutV2 (#1654)
  Add support for `sys_arm_fadvise64_64` (#1648)
  Add __slots__ to expressions (#1635)
  Swap remaining uses of `Z3Solver()` to use the singleton interface (#1649)
  CI: have pytest report 100 slowest tests (#1646)
  ...
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.

4 participants