-
Notifications
You must be signed in to change notification settings - Fork 273
Feature/context sensitive ait merge 2 #2622
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
Merged
martin-cs
merged 5 commits into
diffblue:develop
from
martin-cs:feature/context-sensitive-ait-merge-2
Aug 2, 2018
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
afe32b7
Refactor the methods that access "the abstract domain at a location".
1fe0796
Convert various older domains to use the more recent ait API.
e65f027
Add comments to the abstract interpreter interface.
773bc86
Convert various comments, asserts and throws into invariants.
28ba192
Strengthen the invariant on what are acceptable function calls.
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,8 +15,9 @@ Author: Daniel Kroening, [email protected] | |
#include <memory> | ||
#include <sstream> | ||
|
||
#include <util/std_expr.h> | ||
#include <util/invariant.h> | ||
#include <util/std_code.h> | ||
#include <util/std_expr.h> | ||
|
||
#include "is_threaded.h" | ||
|
||
|
@@ -50,7 +51,7 @@ void ai_baset::output( | |
out << "**** " << i_it->location_number << " " | ||
<< i_it->source_location << "\n"; | ||
|
||
find_state(i_it).output(out, *this, ns); | ||
abstract_state_before(i_it)->output(out, *this, ns); | ||
out << "\n"; | ||
#if 1 | ||
goto_program.output_instruction(ns, identifier, out, *i_it); | ||
|
@@ -101,7 +102,8 @@ jsont ai_baset::output_json( | |
json_numbert(std::to_string(i_it->location_number)); | ||
location["sourceLocation"]= | ||
json_stringt(i_it->source_location.as_string()); | ||
location["abstractState"]=find_state(i_it).output_json(*this, ns); | ||
location["abstractState"] = | ||
abstract_state_before(i_it)->output_json(*this, ns); | ||
|
||
// Ideally we need output_instruction_json | ||
std::ostringstream out; | ||
|
@@ -162,7 +164,7 @@ xmlt ai_baset::output_xml( | |
"source_location", | ||
i_it->source_location.as_string()); | ||
|
||
location.new_element(find_state(i_it).output_xml(*this, ns)); | ||
location.new_element(abstract_state_before(i_it)->output_xml(*this, ns)); | ||
|
||
// Ideally we need output_instruction_xml | ||
std::ostringstream out; | ||
|
@@ -219,7 +221,7 @@ void ai_baset::finalize() | |
ai_baset::locationt ai_baset::get_next( | ||
working_sett &working_set) | ||
{ | ||
assert(!working_set.empty()); | ||
PRECONDITION(!working_set.empty()); | ||
|
||
working_sett::iterator i=working_set.begin(); | ||
locationt l=i->second; | ||
|
@@ -247,6 +249,7 @@ bool ai_baset::fixedpoint( | |
{ | ||
locationt l=get_next(working_set); | ||
|
||
// goto_program is really only needed for iterator manipulation | ||
if(visit(l, working_set, goto_program, goto_functions, ns)) | ||
new_data=true; | ||
} | ||
|
@@ -322,6 +325,8 @@ bool ai_baset::do_function_call( | |
// initialize state, if necessary | ||
get_state(l_return); | ||
|
||
PRECONDITION(l_call->is_function_call()); | ||
|
||
const goto_functionst::goto_functiont &goto_function= | ||
f_it->second; | ||
|
||
|
@@ -387,69 +392,27 @@ bool ai_baset::do_function_call_rec( | |
const goto_functionst &goto_functions, | ||
const namespacet &ns) | ||
{ | ||
assert(!goto_functions.function_map.empty()); | ||
PRECONDITION(!goto_functions.function_map.empty()); | ||
|
||
// This is quite a strong assumption on the well-formedness of the program. | ||
// It means function pointers must be removed before use. | ||
DATA_INVARIANT( | ||
function.id() == ID_symbol, | ||
"Function pointers and indirect calls must be removed before analysis."); | ||
|
||
bool new_data=false; | ||
|
||
if(function.id()==ID_symbol) | ||
{ | ||
const irep_idt &identifier = to_symbol_expr(function).get_identifier(); | ||
const irep_idt &identifier = to_symbol_expr(function).get_identifier(); | ||
|
||
goto_functionst::function_mapt::const_iterator it= | ||
goto_functions.function_map.find(identifier); | ||
goto_functionst::function_mapt::const_iterator it = | ||
goto_functions.function_map.find(identifier); | ||
|
||
if(it==goto_functions.function_map.end()) | ||
throw "failed to find function "+id2string(identifier); | ||
DATA_INVARIANT( | ||
it != goto_functions.function_map.end(), | ||
"Function " + id2string(identifier) + "not in function map"); | ||
|
||
new_data=do_function_call( | ||
l_call, l_return, | ||
goto_functions, | ||
it, | ||
arguments, | ||
ns); | ||
} | ||
else if(function.id()==ID_if) | ||
{ | ||
if(function.operands().size()!=3) | ||
throw "if has three operands"; | ||
|
||
bool new_data1= | ||
do_function_call_rec( | ||
l_call, l_return, | ||
function.op1(), | ||
arguments, | ||
goto_functions, | ||
ns); | ||
|
||
bool new_data2= | ||
do_function_call_rec( | ||
l_call, l_return, | ||
function.op2(), | ||
arguments, | ||
goto_functions, | ||
ns); | ||
|
||
if(new_data1 || new_data2) | ||
new_data=true; | ||
} | ||
else if(function.id()==ID_dereference) | ||
{ | ||
// We can't really do this here -- we rely on | ||
// these being removed by some previous analysis. | ||
} | ||
else if(function.id() == ID_null_object) | ||
{ | ||
// ignore, can't be a function | ||
} | ||
else if(function.id()==ID_member || function.id()==ID_index) | ||
{ | ||
// ignore, can't be a function | ||
} | ||
else | ||
{ | ||
throw "unexpected function_call argument: "+ | ||
function.id_string(); | ||
} | ||
new_data = | ||
do_function_call(l_call, l_return, goto_functions, it, arguments, ns); | ||
|
||
return new_data; | ||
} | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,8 @@ Author: Daniel Kroening, [email protected] | |
|
||
#include "ai_domain.h" | ||
|
||
/// The basic interface of an abstract interpreter. This should be enough | ||
/// to create, run and query an abstract interpreter. | ||
// don't use me -- I am just a base class | ||
// use ait instead | ||
class ai_baset | ||
|
@@ -41,6 +43,7 @@ class ai_baset | |
{ | ||
} | ||
|
||
/// Running the interpreter | ||
void operator()( | ||
const goto_programt &goto_program, | ||
const namespacet &ns) | ||
|
@@ -82,17 +85,26 @@ class ai_baset | |
finalize(); | ||
} | ||
|
||
/// Accessing individual domains at particular locations | ||
/// (without needing to know what kind of domain or history is used) | ||
/// A pointer to a copy as the method should be const and | ||
/// there are some non-trivial cases including merging domains, etc. | ||
/// Intended for users of the abstract interpreter; don't use internally. | ||
|
||
/// Returns the abstract state before the given instruction | ||
virtual const ai_domain_baset & abstract_state_before( | ||
goto_programt::const_targett t) const = 0; | ||
/// PRECONDITION(l is dereferenceable) | ||
virtual std::unique_ptr<statet> abstract_state_before(locationt l) const = 0; | ||
|
||
/// Returns the abstract state after the given instruction | ||
virtual const ai_domain_baset & abstract_state_after( | ||
goto_programt::const_targett t) const | ||
virtual std::unique_ptr<statet> abstract_state_after(locationt l) const | ||
{ | ||
return abstract_state_before(std::next(t)); | ||
/// PRECONDITION(l is dereferenceable && std::next(l) is dereferenceable) | ||
/// Check relies on a DATA_INVARIANT of goto_programs | ||
INVARIANT(!l->is_end_function(), "No state after the last instruction"); | ||
return abstract_state_before(std::next(l)); | ||
} | ||
|
||
/// Resets the domain | ||
virtual void clear() | ||
{ | ||
} | ||
|
@@ -237,6 +249,9 @@ class ai_baset | |
const goto_functionst &goto_functions, | ||
const namespacet &ns); | ||
|
||
// Visit performs one step of abstract interpretation from location l | ||
// Depending on the instruction type it may compute a number of "edges" | ||
// or applications of the abstract transformer | ||
// true = found something new | ||
bool visit( | ||
locationt l, | ||
|
@@ -304,10 +319,17 @@ class ait:public ai_baset | |
return it->second; | ||
} | ||
|
||
const ai_domain_baset & abstract_state_before( | ||
goto_programt::const_targett t) const override | ||
std::unique_ptr<statet> abstract_state_before(locationt t) const override | ||
{ | ||
return (*this)[t]; | ||
typename state_mapt::const_iterator it = state_map.find(t); | ||
if(it == state_map.end()) | ||
{ | ||
std::unique_ptr<statet> d = util_make_unique<domainT>(); | ||
CHECK_RETURN(d->is_bottom()); | ||
return d; | ||
} | ||
|
||
return util_make_unique<domainT>(it->second); | ||
} | ||
|
||
void clear() override | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering whether this shouldn't use
optional
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would be the use-case for it not returning anything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why can't it just return a const ref? I'm unsure where the return value comes from.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case it could and used to return a constant ref. However if we are going to break the 1-to-1 link between location and domain (which is the whole point of this patch set) then we have to be able to support cases where the abstract interpreter may not have the domain and may have to compute it in this function. For example : merging all of the different contexts that reach this point or dynamically recomputing the domain from the last merge point. That's why the API needs to change.