Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Portfolio of parallel solvers #2420

Merged
merged 40 commits into from
Jun 10, 2021
Merged

Portfolio of parallel solvers #2420

merged 40 commits into from
Jun 10, 2021

Conversation

ggrieco-tob
Copy link
Contributor

@ggrieco-tob ggrieco-tob commented Apr 3, 2021

This PR contains:

  • A new meta solver called portfolio, that runs several solvers in parallel. Once one solver wins the race, the other ones are stopped, since there is no way to use their results in a meaningful way.
  • As expected, the number of workers is adjusted when using the portfolio to avoid using more than core.procs processes.
  • A new recv implementation based on non-blocking reads from the stdout pipe. I'm still experimenting how each to increase each timeout to avoid busy waiting.
  • All the installed solvers but z3 are going to be used in the portfolio. I excluded z3 because I suspect it's the slowest one in almost all the tests but I need some more hard evidence. I'm tracking this in Collect some benchmark data and determinate the best smt solver to use/recommend #2449.
  • Fixes for smtlib2 tests with multiple solvers use z3 almost always #2421

)


class SolverInfo:
Copy link
Contributor

Choose a reason for hiding this comment

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

Put the config of each solver in each solver class?

Copy link
Contributor

Choose a reason for hiding this comment

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

As Felipe points out, it'd probably be easier to set some abstract defaults on the Solver class. You could use abc.abstractproperty on Solver and override it on YicesSolver, Z3Solver, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe we cannot use abstract classes here since we need the command line for each solver and they depend on global options. For instance, "{consts.z3_bin} -t:{consts.timeout * 1000} -memory:{consts.memory} -smt2 -in depends on consts.z3_bin and consts.timeout, and consts.timeout which are going to be computed when the command line options are parsed.

Copy link
Contributor

@ehennenfent ehennenfent May 11, 2021

Choose a reason for hiding this comment

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

From our discussion this morning, I see what you mean now. The issue you've encountered is that when Manticore starts up, the following happens:

  • The config module is loaded, and default values set for the values therein
  • The solver module is loaded, and the types contained there are created
  • The command line options are parsed (or a user script runs), setting the config values accordingly.

That means that if you try to define the solver commands on the solver type, like this:

class BoolectorSolver(SMTLIBSolver):
    command = f"{consts.boolector_bin} --time={consts.timeout} -i"
    inits = ["(set-logic QF_AUFBV)", "(set-option :produce-models true)"]

then values pulled from consts will have their initial, default values, not whatever the user wanted them to be.

So instead, your solution is to create a SolverInfo object that pulls the values from the config module when it's created, and create one of these SolverInfo objects in the __init__ function of each of the solvers. While this works (and there's something to be said for having all the SMT commands in one location), I do agree with Felipe that it simplifies the code to distribute the commands onto the relevant classes.

The problem ultimately boils down to: you don't want to generate the commands to start the solver until they're needed. Fortunately, this is one of the problems that properties were designed to solve, since they're dynamically evaluated each time they're retrieved instead of being assigned a single value.

So, to split up SolverInfo, you could do something like this:

class SMTLIBSolver(Solver):

    @property
    @abstractmethod
    def command(self) -> str:
        raise NotImplementedError()

    @property
    @abstractmethod
    def inits(self) -> typing.List[str]:
        raise NotImplementedError()

    ...


class BoolectorSolver(SMTLIBSolver):
    sname = "boolector"

    @property
    def command(self) -> str:
        return f"{consts.boolector_bin} --time={consts.timeout} -i"

    @property
    def inits(self) -> typing.List[str]:
        return ["(set-logic QF_AUFBV)", "(set-option :produce-models true)"]

    ...

And then in SMTLIBSolver.__init__ you would be able to retrieve self.command and self.inits and know that their values would accurately reflect the contents of the config module at that time, not at the time the type was created.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I followed your advise, but instead of using a property, I had to use a classmethod. That was the only way to allow to call command and inits from the class, without actually create an instance the solvers. Please take a look to my recent changes.

manticore/core/manticore.py Outdated Show resolved Hide resolved
@ehennenfent ehennenfent linked an issue Apr 6, 2021 that may be closed by this pull request
@ehennenfent ehennenfent added this to In progress in Manticore Apr 6, 2021
@ggrieco-tob ggrieco-tob marked this pull request as ready for review April 9, 2021 21:28
manticore/core/smtlib/solver.py Show resolved Hide resolved
)


class SolverInfo:
Copy link
Contributor

Choose a reason for hiding this comment

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

As Felipe points out, it'd probably be easier to set some abstract defaults on the Solver class. You could use abc.abstractproperty on Solver and override it on YicesSolver, Z3Solver, etc.

@ehennenfent ehennenfent moved this from In progress to Review in progress in Manticore Apr 13, 2021
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.

Exciting PR!

manticore/core/manticore.py Outdated Show resolved Hide resolved
manticore/core/manticore.py Outdated Show resolved Hide resolved
manticore/core/smtlib/solver.py Outdated Show resolved Hide resolved
manticore/core/smtlib/solver.py Outdated Show resolved Hide resolved
manticore/core/smtlib/solver.py Show resolved Hide resolved
manticore/core/smtlib/solver.py Show resolved Hide resolved
Manticore automation moved this from Review in progress to Reviewer approved May 6, 2021
@ekilmer
Copy link
Contributor

ekilmer commented May 6, 2021

Looks good on my end, but there are still comments from other people that should be resolved first before merging. Thank you!!

@ehennenfent ehennenfent moved this from Reviewer approved to In progress in Manticore Jun 1, 2021
@ehennenfent ehennenfent merged commit 5e4d358 into master Jun 10, 2021
Manticore automation moved this from In progress to Done Jun 10, 2021
@ehennenfent ehennenfent deleted the dev-solver-parallel branch June 10, 2021 19:09
ehennenfent pushed a commit that referenced this pull request Jun 10, 2021
ehennenfent pushed a commit that referenced this pull request Jun 10, 2021
* Manticore 0.3.6

* Create release.yml

* Bump point release number for development releases

* Don't include coveralls in release run

* Update nightly build notice

* Update CHANGELOG.md

* Add changelog line for #2420
ekilmer added a commit that referenced this pull request Jul 15, 2021
* master:
  Install pinned version of truffle to fix CI (#2467)
  Use fixed owner and attacker accounts in multi_tx_analysis (#2464)
  Manticore 0.3.6 (#2456)
  Fix IntrospectionAPIPlugin Name (#2459)
  Portfolio of parallel solvers (#2420)
  Replace Quick mode with Thorough mode (#2457)
  Fix incorrect comparison for symbolic file wildcards (#2454)
  Reduce the number of calls to the SMT solver in EVM (#2411)
  Fixes to Unicorn emulation - start/stop/resume (#1796)
  Add support for multiple compilation units (#2444)
  Basic solver stats (#2415)
ekilmer added a commit that referenced this pull request Jul 27, 2021
* master: (35 commits)
  Track last_pc in StateDescriptors (#2471)
  Expose Result Register for Native CPU (#2470)
  Install pinned version of truffle to fix CI (#2467)
  Use fixed owner and attacker accounts in multi_tx_analysis (#2464)
  Manticore 0.3.6 (#2456)
  Fix IntrospectionAPIPlugin Name (#2459)
  Portfolio of parallel solvers (#2420)
  Replace Quick mode with Thorough mode (#2457)
  Fix incorrect comparison for symbolic file wildcards (#2454)
  Reduce the number of calls to the SMT solver in EVM (#2411)
  Fixes to Unicorn emulation - start/stop/resume (#1796)
  Add support for multiple compilation units (#2444)
  Basic solver stats (#2415)
  Fix the generation of EVM tests (#2426)
  Disabled EVM events in testcases by default (#2417)
  added proper timeouts for cvc4 and boolector (#2418)
  Removed use of global solver from Native Memory (#2414)
  Support to use boolector as the SMT solver (#2410)
  Update CI and suggest to use pip3 instead of pip (#2409)
  Expressions use keyword-only arguments for init (#2395)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Manticore
  
Done
Development

Successfully merging this pull request may close these issues.

Add solver parallelism support
4 participants