Skip to content

Commit 7617ba1

Browse files
committed
fix writeset inclusion checks on DEAD instructions
When handling DEAD instructions in function/loop assigns clause instrumentation, the instruction iterator was not being incremented correctly. This led to instrumentation outside of the function/loop scope, and spurious write set inclusion violations.
1 parent e6b3118 commit 7617ba1

File tree

3 files changed

+61
-5
lines changed

3 files changed

+61
-5
lines changed
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
#include <assert.h>
2+
3+
int j;
4+
5+
int lowerbound()
6+
{
7+
return 0;
8+
}
9+
10+
int upperbound()
11+
{
12+
return 10;
13+
}
14+
15+
void incr(int *i)
16+
{
17+
(*i)++;
18+
}
19+
20+
void body(int i)
21+
{
22+
j = i;
23+
}
24+
25+
void main()
26+
{
27+
for(int i = lowerbound(); i < upperbound(); incr(&i))
28+
// clang-format off
29+
__CPROVER_assigns(i, j)
30+
__CPROVER_loop_invariant(0 <= i && i <= 10)
31+
__CPROVER_loop_invariant(i != 0 ==> j + 1 == i)
32+
// clang-format on
33+
{
34+
body(i);
35+
}
36+
37+
assert(j == 9);
38+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
CORE
2+
main.c
3+
--apply-loop-contracts
4+
^EXIT=0$
5+
^SIGNAL=0$
6+
^\[body.\d+\] .* Check that j is assignable: SUCCESS$
7+
^\[incr.\d+\] .* Check that \*i is assignable: SUCCESS$
8+
^\[main.\d+\] .* Check that i is assignable: SUCCESS$
9+
^\[main.\d+\] .* Check loop invariant before entry: SUCCESS$
10+
^\[main.\d+\] .* Check that loop invariant is preserved: SUCCESS$
11+
^\[main.assertion.\d+\] .* assertion j == 9: SUCCESS$
12+
^VERIFICATION SUCCESSFUL$
13+
--
14+
--
15+
This test checks write set inclusion checks in presence of function calls,
16+
which are inlined, and also checks that DEAD instructions introduced during
17+
inlining is correctly handled.

src/goto-instrument/contracts/contracts.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1054,19 +1054,20 @@ void code_contractst::check_frame_conditions(
10541054
assigns_clauset::conditional_address_ranget{assigns, symbol});
10551055
if(symbol_car != assigns.get_write_set().end())
10561056
{
1057-
instruction_it++;
10581057
auto invalidation_assignment = goto_programt::make_assignment(
10591058
symbol_car->validity_condition_var,
10601059
false_exprt{},
10611060
instruction_it->source_location());
1062-
// note that instruction_it is not advanced by this call,
1063-
// so no need to move it backwards
1064-
body.insert_before_swap(instruction_it, invalidation_assignment);
1061+
// note that the CAR must be invalidated _after_ the DEAD instruction
1062+
body.insert_after(
1063+
instruction_it,
1064+
add_pragma_disable_assigns_check(invalidation_assignment));
10651065
}
10661066
else
10671067
{
10681068
throw incorrect_goto_program_exceptiont(
1069-
"Found a `DEAD` variable without corresponding `DECL`!",
1069+
"Found a `DEAD` variable " + name2string(symbol.get_identifier()) +
1070+
" without corresponding `DECL`!",
10701071
instruction_it->source_location());
10711072
}
10721073
}

0 commit comments

Comments
 (0)