Skip to content

CONTRACTS: Allow void function calls in assigns clauses #7214

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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion regression/contracts/assigns_enforce_address_of/test.desc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ main.c
--enforce-contract foo
^EXIT=(1|64)$
^SIGNAL=0$
^.*error: assigns clause target must be a non-void lvalue or a call to one of __CPROVER_POINTER_OBJECT, __CPROVER_assignable, __CPROVER_object_whole, __CPROVER_object_upto, __CPROVER_object_from$
^.*error: assigns clause target must be a non-void lvalue, a call to __CPROVER_POINTER_OBJECT or a call to a function returning void$
^CONVERSION ERROR$
--
--
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
CORE
main.c
--enforce-contract foo
^.*error: assigns clause target must be a non-void lvalue or a call to one of __CPROVER_POINTER_OBJECT, __CPROVER_assignable, __CPROVER_object_whole, __CPROVER_object_upto, __CPROVER_object_from$
^.*error: assigns clause target must be a non-void lvalue, a call to __CPROVER_POINTER_OBJECT or a call to a function returning void$
^CONVERSION ERROR
^EXIT=(1|64)$
^SIGNAL=0$
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
CORE
main.c
--enforce-contract foo
^.*error: assigns clause target must be a non-void lvalue or a call to one of __CPROVER_POINTER_OBJECT, __CPROVER_assignable, __CPROVER_object_whole, __CPROVER_object_upto, __CPROVER_object_from$
^.*error: assigns clause target must be a non-void lvalue, a call to __CPROVER_POINTER_OBJECT or a call to a function returning void$
^CONVERSION ERROR
^EXIT=(1|64)$
^SIGNAL=0$
Expand Down
1 change: 0 additions & 1 deletion regression/contracts/assigns_enforce_function_calls/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,4 @@ int main()
{
int x;
foo(&x);
baz(&x);
}
4 changes: 2 additions & 2 deletions regression/contracts/assigns_enforce_function_calls/test.desc
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ main.c
--enforce-contract foo
^EXIT=(1|64)$
^SIGNAL=0$
^.*error: assigns clause target must be a non-void lvalue or a call to one of __CPROVER_POINTER_OBJECT, __CPROVER_assignable, __CPROVER_object_whole, __CPROVER_object_upto, __CPROVER_object_from$
^.*error: expecting void return type for function 'bar' called in assigns clause$
^CONVERSION ERROR$
--
--
Check that function call expressions are rejected in assigns clauses.
Check that non-void function call expressions are rejected in assigns clauses.
17 changes: 17 additions & 0 deletions regression/contracts/assigns_enforce_function_calls_ignored/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
void bar(int *x)
{
if(x)
__CPROVER_typed_target(x);
}

int foo(int *x) __CPROVER_assigns(bar(x))
{
*x = 0;
return 0;
}

int main()
{
int x;
foo(&x);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
CORE
main.c
--enforce-contract foo
^call to function 'bar' in assigns clause not supported yet$
^EXIT=(127|134)$
^SIGNAL=0$
--
--
Check that void function call expressions in assigns clauses make
instrumentation fail.
2 changes: 1 addition & 1 deletion regression/contracts/assigns_enforce_literal/test.desc
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ main.c
--enforce-contract foo
^EXIT=(1|64)$
^SIGNAL=0$
^.*error: assigns clause target must be a non-void lvalue or a call to one of __CPROVER_POINTER_OBJECT, __CPROVER_assignable, __CPROVER_object_whole, __CPROVER_object_upto, __CPROVER_object_from$
^.*error: assigns clause target must be a non-void lvalue, a call to __CPROVER_POINTER_OBJECT or a call to a function returning void$
^CONVERSION ERROR$
--
--
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ main.c
--enforce-contract foo
^EXIT=(1|64)$
^SIGNAL=0$
^.*error: assigns clause target must be a non-void lvalue or a call to one of __CPROVER_POINTER_OBJECT, __CPROVER_assignable, __CPROVER_object_whole, __CPROVER_object_upto, __CPROVER_object_from$
^.*error: assigns clause target must be a non-void lvalue, a call to __CPROVER_POINTER_OBJECT or a call to a function returning void$
^CONVERSION ERROR$
--
--
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ main.c
--enforce-contract foo
^EXIT=(1|64)$
^SIGNAL=0$
^.*error: assigns clause target must be a non-void lvalue or a call to one of __CPROVER_POINTER_OBJECT, __CPROVER_assignable, __CPROVER_object_whole, __CPROVER_object_upto, __CPROVER_object_from$
^.*error: assigns clause target must be a non-void lvalue, a call to __CPROVER_POINTER_OBJECT or a call to a function returning void$
^CONVERSION ERROR$
--
--
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ main.c
^EXIT=(1|64)$
^SIGNAL=0$
^CONVERSION ERROR$
^.*error: assigns clause target must be a non-void lvalue or a call to one of __CPROVER_POINTER_OBJECT, __CPROVER_assignable, __CPROVER_object_whole, __CPROVER_object_upto, __CPROVER_object_from$
^.*error: assigns clause target must be a non-void lvalue, a call to __CPROVER_POINTER_OBJECT or a call to a function returning void$
--
Checks whether type checking rejects literal constants in assigns clause.
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
CORE
main.c
--enforce-contract foo
^main.c.*error: assigns clause target must be a non-void lvalue or a call to one of __CPROVER_POINTER_OBJECT, __CPROVER_assignable, __CPROVER_object_whole, __CPROVER_object_upto, __CPROVER_object_from$
^main.c.*error: assigns clause target must be a non-void lvalue, a call to __CPROVER_POINTER_OBJECT or a call to a function returning void$
^CONVERSION ERROR$
^EXIT=(1|64)$
^SIGNAL=0$
Expand Down
42 changes: 21 additions & 21 deletions src/ansi-c/c_typecheck_code.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -995,34 +995,34 @@ void c_typecheck_baset::typecheck_spec_assigns_target(exprt &target)
else if(can_cast_expr<side_effect_expr_function_callt>(target))
{
const auto &funcall = to_side_effect_expr_function_call(target);
if(can_cast_expr<symbol_exprt>(funcall.function()))
if(!can_cast_expr<symbol_exprt>(funcall.function()))
{
const auto &ident = to_symbol_expr(funcall.function()).get_identifier();
if(
ident == CPROVER_PREFIX "assignable" ||
ident == CPROVER_PREFIX "object_whole" ||
ident == CPROVER_PREFIX "object_upto" ||
ident == CPROVER_PREFIX "object_from")
{
for(const auto &argument : funcall.arguments())
throw_on_side_effects(argument);
return;
}
throw invalid_source_file_exceptiont(
"function pointer calls not allowed in assigns clauses",
target.source_location());
Comment on lines +1000 to +1002
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we add a regression test to cover this case?

}
else

if(target.type().id() != ID_empty)
{
throw invalid_source_file_exceptiont(
"function pointer calls not allowed in assigns clauses",
"expecting void return type for function '" +
id2string(to_symbol_expr(funcall.function()).get_identifier()) +
"' called in assigns clause",
target.source_location());
}

for(const auto &argument : funcall.arguments())
throw_on_side_effects(argument);
}
else
{
// if we reach this point the target did not pass the checks
throw invalid_source_file_exceptiont(
"assigns clause target must be a non-void lvalue, a call "
"to " CPROVER_PREFIX
"POINTER_OBJECT or a call to a function returning void",
Comment on lines +1022 to +1023
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need to remove here all CPROVER_PREFIX_object_* cases? Make the error message shorter? For me, it was quite informative before.

target.source_location());
}
// if we reach this point the target did not pass the checks
throw invalid_source_file_exceptiont(
"assigns clause target must be a non-void lvalue or a call to one "
"of " CPROVER_PREFIX "POINTER_OBJECT, " CPROVER_PREFIX
"assignable, " CPROVER_PREFIX "object_whole, " CPROVER_PREFIX
"object_upto, " CPROVER_PREFIX "object_from",
target.source_location());
}

void c_typecheck_baset::typecheck_spec_function_pointer_obeys_contract(
Expand Down
15 changes: 9 additions & 6 deletions src/goto-instrument/contracts/instrument_spec_assigns.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,15 @@ car_exprt instrument_spec_assignst::create_car_expr(
if(can_cast_expr<symbol_exprt>(funcall.function()))
{
const auto &ident = to_symbol_expr(funcall.function()).get_identifier();

PRECONDITION_WITH_DIAGNOSTICS(
ident == CPROVER_PREFIX "object_from" ||
ident == CPROVER_PREFIX "object_upto" ||
ident == CPROVER_PREFIX "object_whole" ||
ident == CPROVER_PREFIX "assignable",
"call to function '" + id2string(ident) +
"' in assigns clause not supported yet");

if(ident == CPROVER_PREFIX "object_from")
{
const auto &ptr = funcall.arguments().at(0);
Expand Down Expand Up @@ -558,12 +567,6 @@ car_exprt instrument_spec_assignst::create_car_expr(
is_ptr_to_ptr.is_true() ? car_havoc_methodt::NONDET_ASSIGN
: car_havoc_methodt::HAVOC_SLICE};
}
else
{
log.error().source_location = target.source_location();
log.error() << "call to " << ident
<< " in assigns clauses not supported in this version";
}
}
}
else if(is_assignable(target))
Expand Down