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

Implement symbolic branch tracking in core, add new fork event #433

Merged
merged 3 commits into from
Aug 3, 2017

Conversation

offlinemark
Copy link
Contributor

@offlinemark offlinemark commented Aug 3, 2017

This PR implements tracking of symbolic branches in each State (specifically state.context['branches']). It does this using a new event: forking_state. Similar to will_fork_state, forking_state fires when a state fork occurs, however it fires once for each individual state forked, being passed the specific value that has been concretized. will_fork_state fires once on a general state forking event, being passed the collection of all concretized values. This is unsuitable for tracking symbolic branches per state because the State the event handler has access to during will_fork_state is the parent state, and not a child state. For tracking symbolic branches, we want to only modify the specific child state.

Points of discussion and controversy:

Whether we should track symbolic branches in the core, versus requiring users to do this.

I think this is general enough and useful enough functionality to compute in the core and have available to users "for free".

The name of the event: forking_state.

I'm open to suggestions for other names! forking_child_state, ...

The introduction of State._init_context.

I thought it would be useful to have a central place where all the entries of state.context are initialized, versus them largely being initialized ad hoc in various places in hooks in manticore.py. Initializing entries of state.context here allows code that uses state.context to be less cautious about whether a entry actually exists or not. For example:

state_visited = state.context.get('visited_since_last_fork', set())

state_visited = state.context.get('visited_since_last_fork', set())

state.context.setdefault('visited_since_last_fork', set()).add(address)

Currently only context['branches'] is initialized here, but the idea is to eventually port other usees of state.context in the core to initialize those members here.

@offlinemark offlinemark requested a review from feliam August 3, 2017 17:44
#and set the PC of the new state to the concrete pc-dest
#(or other register or memory address to concrete)
setstate(new_state, new_value)
#enqueue new_state

with self._lock:
Copy link
Contributor

Choose a reason for hiding this comment

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

Unnecessary lock

@offlinemark
Copy link
Contributor Author

Summary of discussion in chat regarding locks for posterity

  • no calls to publish need to be wrapped with a self._lock (the fact that will_fork_state publish above did this was a bug)
  • all callbacks execute in separate worker threads, so there is only process unsafety considerations if they access shared data. shared data refers to the global executor context (Executor._shared_context, accessed through Executor.locked_context` (see executor._fork_state_callback for an example)
  • if a callback accessed global shared data, it is the callback's responsibility to use the aforementioned lock when accessing it
  • currently only callbacks at the executor level even have access to the shared data. a deeper callback at the state, platform, cpu level for example could not even do process unsafe things if it wanted to

@offlinemark offlinemark merged commit 7b83272 into master Aug 3, 2017
@offlinemark offlinemark deleted the dev-fork-state-event branch August 3, 2017 21:13
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.

None yet

2 participants