-
Notifications
You must be signed in to change notification settings - Fork 274
Enforce purity on missing assigns clause #6074
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,11 @@ | ||
CORE | ||
main.c | ||
--enforce-all-contracts | ||
^EXIT=10$ | ||
^EXIT=0$ | ||
^SIGNAL=0$ | ||
^VERIFICATION FAILED$ | ||
^\[f1.1\] line \d+ .*: SUCCESS$ | ||
^VERIFICATION SUCCESSFUL$ | ||
-- | ||
-- | ||
This test checks that verification fails when a function with an assigns clause calls a function without an assigns clause. | ||
This test checks that verification succeeds when enforcing a contract | ||
for functions, without assigns clauses, that don't modify anything. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
int global = 0; | ||
|
||
int foo(int *x) __CPROVER_assigns(*x) | ||
__CPROVER_ensures(__CPROVER_return_value == *x + 5) | ||
{ | ||
int z = 5; | ||
int y = bar(&z); | ||
return *x + 5; | ||
} | ||
|
||
int bar(int *a) __CPROVER_ensures(__CPROVER_return_value >= 5) | ||
{ | ||
*a = *a + 2; | ||
return *a; | ||
} | ||
|
||
int baz() __CPROVER_ensures(__CPROVER_return_value == global) | ||
{ | ||
global = global + 1; | ||
return global; | ||
} | ||
|
||
int main() | ||
feliperodri marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
int n; | ||
n = foo(&n); | ||
n = baz(); | ||
return 0; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
CORE | ||
main.c | ||
--enforce-all-contracts | ||
^EXIT=10$ | ||
^SIGNAL=0$ | ||
^\[bar.1\] line \d+ .*: FAILURE$ | ||
^\[baz.1\] line \d+ .*: FAILURE$ | ||
^VERIFICATION FAILED$ | ||
feliperodri marked this conversation as resolved.
Show resolved
Hide resolved
|
||
-- | ||
-- | ||
Checks whether verification fails when enforcing a contract | ||
for functions, without assigns clauses, that modify an input. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -513,50 +513,38 @@ void code_contractst::instrument_call_statement( | |
|
||
exprt called_assigns = | ||
to_code_with_contract_type(called_symbol.type).assigns(); | ||
if(called_assigns.is_nil()) // Called function has no assigns clause | ||
{ | ||
// Create a false assertion, so the analysis | ||
// will fail if this function is called. | ||
goto_programt failing_assertion; | ||
failing_assertion.add(goto_programt::make_assertion( | ||
false_exprt(), instruction_iterator->source_location)); | ||
program.insert_before_swap(instruction_iterator, failing_assertion); | ||
++instruction_iterator; | ||
|
||
return; | ||
} | ||
else // Called function has assigns clause | ||
if(!called_assigns.is_nil()) // Called function has assigns clause | ||
{ | ||
feliperodri marked this conversation as resolved.
Show resolved
Hide resolved
|
||
replace_symbolt replace; | ||
// Replace formal parameters | ||
code_function_callt::argumentst::const_iterator a_it = | ||
call.arguments().begin(); | ||
for(code_typet::parameterst::const_iterator p_it = | ||
called_type.parameters().begin(); | ||
p_it != called_type.parameters().end() && | ||
a_it != call.arguments().end(); | ||
++p_it, ++a_it) | ||
{ | ||
replace_symbolt replace; | ||
// Replace formal parameters | ||
code_function_callt::argumentst::const_iterator a_it = | ||
call.arguments().begin(); | ||
for(code_typet::parameterst::const_iterator p_it = | ||
called_type.parameters().begin(); | ||
p_it != called_type.parameters().end() && | ||
a_it != call.arguments().end(); | ||
++p_it, ++a_it) | ||
if(!p_it->get_identifier().empty()) | ||
{ | ||
if(!p_it->get_identifier().empty()) | ||
{ | ||
symbol_exprt p(p_it->get_identifier(), p_it->type()); | ||
replace.insert(p, *a_it); | ||
} | ||
symbol_exprt p(p_it->get_identifier(), p_it->type()); | ||
replace.insert(p, *a_it); | ||
} | ||
|
||
replace(called_assigns); | ||
|
||
// check compatibility of assigns clause with the called function | ||
assigns_clauset called_assigns_clause( | ||
called_assigns, *this, function_id, log); | ||
exprt compatible = | ||
assigns_clause.compatible_expression(called_assigns_clause); | ||
goto_programt alias_assertion; | ||
alias_assertion.add(goto_programt::make_assertion( | ||
compatible, instruction_iterator->source_location)); | ||
program.insert_before_swap(instruction_iterator, alias_assertion); | ||
++instruction_iterator; | ||
} | ||
|
||
replace(called_assigns); | ||
|
||
// check compatibility of assigns clause with the called function | ||
assigns_clauset called_assigns_clause( | ||
called_assigns, *this, function_id, log); | ||
exprt compatible = | ||
assigns_clause.compatible_expression(called_assigns_clause); | ||
goto_programt alias_assertion; | ||
alias_assertion.add(goto_programt::make_assertion( | ||
compatible, instruction_iterator->source_location)); | ||
program.insert_before_swap(instruction_iterator, alias_assertion); | ||
++instruction_iterator; | ||
} | ||
} | ||
|
||
bool code_contractst::check_for_looped_mallocs(const goto_programt &program) | ||
|
@@ -634,9 +622,6 @@ bool code_contractst::add_pointer_checks(const std::string &function_name) | |
const auto &type = to_code_with_contract_type(function_symbol.type); | ||
|
||
exprt assigns_expr = type.assigns(); | ||
// Return if there are no reference checks to perform. | ||
if(assigns_expr.is_nil()) | ||
return false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a check - is there a test case that will fail if this code ever gets inserted again? :-) It superficially looks like an "obvious" optimisation, so I can imaging someone in the future re-adding it with good intent. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If somebody add this back, |
||
|
||
assigns_clauset assigns(assigns_expr, *this, function_id, log); | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.