Skip to content

Commit 5ac6ab4

Browse files
author
Remi Delmas
committed
Function contracts: new method for havocking assigns clause targets that works when there are dependencies between targets.
We now take a snapshot of the target addresses and havoc them in a second step if they are valid. Previously we were havocking targets directly in sequence, and havocking *s before *(s->ptr) or __CPROVER_POINTER_OBJECT(s->ptr) would cause errors. Added a regression test with dependent assigns targets.
1 parent c143a27 commit 5ac6ab4

File tree

11 files changed

+418
-14
lines changed

11 files changed

+418
-14
lines changed

regression/contracts/assigns_enforce_21/test.desc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ main.c
55
^SIGNAL=0$
66
main.c function bar
77
^\[bar.\d+\] line \d+ Check that \*y is assignable: SUCCESS$
8-
^\[bar.\d+\] line \d+ Check that x is assignable: FAILURE$
8+
^\[bar.\d+\] line \d+ Check that x assigned by replaced quz is assignable: FAILURE$
99
^VERIFICATION FAILED$
1010
--
1111
--

regression/contracts/assigns_replace_08/test.desc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ main.c
33
--enforce-contract foo --replace-call-with-contract bar _ --pointer-primitive-check
44
^EXIT=10$
55
^SIGNAL=0$
6-
\[foo.\d+\] line \d+ Check that \*z is assignable: FAILURE$
6+
\[foo.\d+\] line \d+ Check that \*z assigned by replaced bar is assignable: FAILURE$
77
^.* 1 of \d+ failed \(\d+ iteration.*\)$
88
^VERIFICATION FAILED$
99
--

regression/contracts/assigns_replace_09/test.desc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ main.c
33
--replace-call-with-contract bar --enforce-contract foo
44
^EXIT=0$
55
^SIGNAL=0$
6-
\[foo.\d+\] line \d+ Check that \*z is assignable: SUCCESS$
6+
\[foo.\d+\] line \d+ Check that \*z assigned by replaced bar is assignable: SUCCESS$
77
^VERIFICATION SUCCESSFUL$
88
--
99
^Condition: \!not\_found$
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
CORE
2+
main_enforce.c
3+
--enforce-contract resize_vec _ --signed-overflow-check --unsigned-overflow-check --pointer-overflow-check
4+
^VERIFICATION SUCCESSFUL$
5+
^EXIT=0$
6+
^SIGNAL=0$
7+
--
8+
--
9+
Verifies the contract being replaced in `replace_resize_vec.desc`.
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
#include "vect.h"
2+
3+
int main()
4+
{
5+
vect *v;
6+
size_t incr;
7+
resize_vec(v, incr);
8+
return 0;
9+
}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
#include "vect.h"
2+
3+
int main()
4+
{
5+
vect *v;
6+
resize_vec_incr10(v);
7+
return 0;
8+
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
CORE
2+
main_replace.c
3+
--replace-call-with-contract resize_vec --enforce-contract resize_vec_incr10 _ --signed-overflow-check --unsigned-overflow-check --pointer-overflow-check
4+
^VERIFICATION SUCCESSFUL$
5+
^\[.*\] .* Check that \*v assigned by replaced resize_vec is assignable: SUCCESS
6+
^\[.*\] .* Check that POINTER_OBJECT\(\(const void \*\)v->arr\) assigned by replaced resize_vec is assignable: SUCCESS
7+
^EXIT=0$
8+
^SIGNAL=0$
9+
--
10+
--
11+
Shows that when an assigns clause contains targets that are dependent,
12+
in this case, a struct `*v` and an array `__CPROVER_POINTER_OBJECT(v->arr)`
13+
pointed to by a member of that struct, we can correctly havoc them when
14+
replacing the call by the contract.
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
#include <assert.h>
2+
#include <stdlib.h>
3+
4+
typedef struct vect
5+
{
6+
char *arr;
7+
size_t size;
8+
} vect;
9+
10+
void resize_vec(vect *v, size_t incr)
11+
// clang-format off
12+
__CPROVER_requires(
13+
__CPROVER_is_fresh(v, sizeof(vect)) &&
14+
0 < v->size && v->size <= __CPROVER_max_malloc_size &&
15+
0 < incr && incr < __CPROVER_max_malloc_size - v->size &&
16+
__CPROVER_is_fresh(v->arr, v->size)
17+
)
18+
__CPROVER_assigns(*v, __CPROVER_POINTER_OBJECT(v->arr))
19+
__CPROVER_ensures(
20+
v->size == __CPROVER_old(v->size) + __CPROVER_old(incr) &&
21+
__CPROVER_is_fresh(v->arr, v->size)
22+
)
23+
// clang-format on
24+
{
25+
free(v->arr);
26+
v->size += incr;
27+
v->arr = malloc(v->size);
28+
return;
29+
}
30+
31+
void resize_vec_incr10(vect *v)
32+
// clang-format off
33+
__CPROVER_requires(
34+
__CPROVER_is_fresh(v, sizeof(vect)) &&
35+
0 < v->size && v->size <= __CPROVER_max_malloc_size &&
36+
v->size + 10 < __CPROVER_max_malloc_size &&
37+
__CPROVER_is_fresh(v->arr, v->size)
38+
)
39+
__CPROVER_assigns(*v, __CPROVER_POINTER_OBJECT(v->arr))
40+
__CPROVER_ensures(
41+
v->size == __CPROVER_old(v->size) + 10 &&
42+
__CPROVER_is_fresh(v->arr, v->size)
43+
)
44+
// clang-format on
45+
{
46+
resize_vec(v, 10);
47+
return;
48+
}

src/goto-instrument/contracts/contracts.cpp

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ Date: February 2016
4343
#include <util/replace_symbol.h>
4444
#include <util/std_code.h>
4545

46+
#include "havoc_multiple_targets.h"
4647
#include "memory_predicates.h"
4748
#include "utils.h"
4849

@@ -786,15 +787,17 @@ bool code_contractst::apply_function_contract(
786787
// ...for assigns clause targets
787788
if(!assigns_clause.empty())
788789
{
789-
assigns_clauset assigns_clause(
790-
targets.operands(), log, ns, target_function, symbol_table);
791-
792-
// Havoc all targets in the write set
793-
assignst assigns;
794-
assigns.insert(targets.operands().cbegin(), targets.operands().cend());
795-
796-
havoc_assigns_targetst havoc_gen(assigns, ns);
797-
havoc_gen.append_full_havoc_code(location, havoc_instructions);
790+
// Havoc all targets in the assigns clause
791+
// TODO handle local statics possibly touched by this function
792+
havoc_multiple_targets(
793+
target_function,
794+
targets.operands(),
795+
havoc_instructions,
796+
// context parameters
797+
location,
798+
mode,
799+
ns,
800+
symbol_table);
798801
}
799802

800803
// ...for the return value
@@ -1401,8 +1404,20 @@ code_contractst::add_inclusion_check(
14011404
source_locationt location_no_checks =
14021405
instruction_it->source_location_nonconst();
14031406
disable_pointer_checks(location_no_checks);
1404-
location_no_checks.set_comment(
1405-
"Check that " + from_expr(ns, lhs.id(), lhs) + " is assignable");
1407+
1408+
// does this assignment come from some contract replacement ?
1409+
const auto &comment = location_no_checks.get_comment();
1410+
if(is_assigns_clause_replacement_tracking_comment(comment))
1411+
{
1412+
location_no_checks.set_comment(
1413+
"Check that " + id2string(comment) + " is assignable");
1414+
}
1415+
else
1416+
{
1417+
location_no_checks.set_comment(
1418+
"Check that " + from_expr(ns, lhs.id(), lhs) + " is assignable");
1419+
}
1420+
14061421
assertion.add(goto_programt::make_assertion(
14071422
assigns.generate_inclusion_check(car, cfg_info_opt), location_no_checks));
14081423
insert_before_swap_and_advance(program, instruction_it, assertion);

0 commit comments

Comments
 (0)