Skip to content

Commit 61ea8ed

Browse files
committed
Avoid crashing when applying trivial contracts
Requires and ensures are currently treated as conjunctions, thus empty clauses are mapped to true. This commit also prevents that trivials `assert(true)` are injected into functions. Signed-off-by: Felipe R. Monteiro <felisous@amazon.com>
1 parent 552beee commit 61ea8ed

File tree

5 files changed

+56
-15
lines changed

5 files changed

+56
-15
lines changed
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
#include <assert.h>
2+
#include <stdlib.h>
3+
4+
int foo(int *x) __CPROVER_requires(x != NULL)
5+
{
6+
return *x + 5;
7+
}
8+
9+
int main()
10+
{
11+
int n = 10;
12+
assert(foo(&n) != 15);
13+
return 0;
14+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
CORE
2+
main.c
3+
--enforce-all-contracts
4+
^EXIT=10$
5+
^SIGNAL=0$
6+
^\[main.assertion.\d+\] line \d+ assertion foo\(\&n\) != 15\: FAILURE$
7+
^VERIFICATION FAILED$
8+
--
9+
--
10+
Check whether CBMC doesn't crash when enforcing trivial contracts, i.e.,
11+
the postcondition is true (default when missing) and therefore there is
12+
nothing to check/assert.
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
#include <assert.h>
2+
3+
int foo(int *x) __CPROVER_ensures(__CPROVER_return_value == *x + 5)
4+
{
5+
return *x + 5;
6+
}
7+
8+
int main()
9+
{
10+
int n = 10;
11+
assert(foo(&n) != 15);
12+
return 0;
13+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
CORE
2+
main.c
3+
--enforce-all-contracts
4+
^EXIT=10$
5+
^SIGNAL=0$
6+
^\[main.assertion.\d+\] line \d+ assertion foo\(\&n\) != 15\: FAILURE$
7+
^VERIFICATION FAILED$
8+
--
9+
--
10+
Check whether CBMC doesn't crash when replacing trivial contracts, i.e.,
11+
the precondition is true (default when missing) and therefore there is
12+
nothing to check/assert.

src/goto-instrument/contracts/contracts.cpp

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -491,11 +491,6 @@ bool code_contractst::apply_function_contract(
491491
auto requires = conjunction(type.requires());
492492
auto ensures = conjunction(type.ensures());
493493

494-
// Check to see if the function contract actually constrains its effect on
495-
// the program state; if not, return.
496-
if(ensures.is_true() && assigns.is_nil())
497-
return false;
498-
499494
// Create a replace_symbolt object, for replacing expressions in the callee
500495
// with expressions from the call site (e.g. the return value).
501496
// This object tracks replacements that are common to ENSURES and REQUIRES.
@@ -559,7 +554,7 @@ bool code_contractst::apply_function_contract(
559554
is_fresh.create_declarations();
560555

561556
// Insert assertion of the precondition immediately before the call site.
562-
if(requires.is_not_nil())
557+
if(!requires.is_true())
563558
{
564559
replace_symbolt replace(common_replace);
565560
code_contractst::add_quantified_variable(requires, replace, mode);
@@ -584,7 +579,7 @@ bool code_contractst::apply_function_contract(
584579
// Gather all the instructions required to handle history variables
585580
// as well as the ensures clause
586581
std::pair<goto_programt, goto_programt> ensures_pair;
587-
if(ensures.is_not_nil())
582+
if(!ensures.is_false())
588583
{
589584
replace_symbolt replace(common_replace);
590585
code_contractst::add_quantified_variable(ensures, replace, mode);
@@ -619,7 +614,7 @@ bool code_contractst::apply_function_contract(
619614

620615
// To remove the function call, insert statements related to the assumption.
621616
// Then, replace the function call with a SKIP statement.
622-
if(ensures.is_not_nil())
617+
if(!ensures.is_false())
623618
{
624619
is_fresh.update_ensures(ensures_pair.first);
625620
auto lines_to_iterate = ensures_pair.first.instructions.size();
@@ -1072,11 +1067,6 @@ void code_contractst::add_contract_check(
10721067
exprt requires = conjunction(code_type.requires());
10731068
exprt ensures = conjunction(code_type.ensures());
10741069

1075-
INVARIANT(
1076-
!ensures.is_true() || assigns.is_not_nil(),
1077-
"Code contract enforcement is trivial without an ensures or assigns "
1078-
"clause.");
1079-
10801070
// build:
10811071
// if(nondet)
10821072
// decl ret
@@ -1148,7 +1138,7 @@ void code_contractst::add_contract_check(
11481138
visitor.create_declarations();
11491139

11501140
// Generate: assume(requires)
1151-
if(requires.is_not_nil())
1141+
if(!requires.is_false())
11521142
{
11531143
// extend common_replace with quantified variables in REQUIRES,
11541144
// and then do the replacement
@@ -1168,7 +1158,7 @@ void code_contractst::add_contract_check(
11681158
std::pair<goto_programt, goto_programt> ensures_pair;
11691159

11701160
// Generate: copies for history variables
1171-
if(ensures.is_not_nil())
1161+
if(!ensures.is_true())
11721162
{
11731163
// extend common_replace with quantified variables in ENSURES,
11741164
// and then do the replacement

0 commit comments

Comments
 (0)