Skip to content

Commit 26760d3

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. Moreover, for loops, declarations of some temporaries (those involved in the "initialization" of loop counter, for instance) are "outside" of the loop identified by CBMC, so we no longer raise an exception on not finding a corresponding DECL for each DEAD. These variables are writable because they appear as assigns clause targets, not because they were local DECLs.
1 parent e6b3118 commit 26760d3

File tree

3 files changed

+93
-7
lines changed

3 files changed

+93
-7
lines changed
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
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_1(int i)
21+
{
22+
j = i;
23+
}
24+
25+
void body_2(int *i)
26+
{
27+
(*i)++;
28+
(*i)--;
29+
}
30+
31+
int body_3(int *i)
32+
{
33+
(*i)++;
34+
if(*i == 4)
35+
return 1;
36+
37+
(*i)--;
38+
return 0;
39+
}
40+
41+
int main()
42+
{
43+
for(int i = lowerbound(); i < upperbound(); incr(&i))
44+
// clang-format off
45+
__CPROVER_assigns(i, j)
46+
__CPROVER_loop_invariant(0 <= i && i <= 10)
47+
__CPROVER_loop_invariant(i != 0 ==> j + 1 == i)
48+
// clang-format on
49+
{
50+
body_1(i);
51+
52+
if(body_3(&i))
53+
return 1;
54+
55+
body_2(&i);
56+
}
57+
58+
assert(j == 9);
59+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
CORE
2+
main.c
3+
--apply-loop-contracts
4+
^EXIT=0$
5+
^SIGNAL=0$
6+
^\[body_1.\d+\] .* Check that j is assignable: SUCCESS$
7+
^\[body_2.\d+\] .* Check that \*i is assignable: SUCCESS$
8+
^\[body_3.\d+\] .* Check that \*i is assignable: SUCCESS$
9+
^\[incr.\d+\] .* Check that \*i is assignable: SUCCESS$
10+
^\[main.\d+\] .* Check that i is assignable: SUCCESS$
11+
^\[main.\d+\] .* Check loop invariant before entry: SUCCESS$
12+
^\[main.\d+\] .* Check that loop invariant is preserved: SUCCESS$
13+
^\[main.assertion.\d+\] .* assertion j == 9: SUCCESS$
14+
^VERIFICATION SUCCESSFUL$
15+
--
16+
--
17+
This test checks write set inclusion checks in presence of function calls,
18+
which are inlined, and also checks that DEAD instructions introduced during
19+
inlining is correctly handled.

src/goto-instrument/contracts/contracts.cpp

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1054,20 +1054,28 @@ 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
{
1068-
throw incorrect_goto_program_exceptiont(
1069-
"Found a `DEAD` variable without corresponding `DECL`!",
1070-
instruction_it->source_location());
1068+
// For loops, the loop_head appears after the DECL of counters,
1069+
// and any other temporaries introduced during "initialization".
1070+
// However, they go `DEAD` (possible conditionally) inside the loop,
1071+
// in presence of return statements.
1072+
// Notice that for them those variables be writable,
1073+
// they must appear as assigns targets anyway,
1074+
// but their DECL statements are outside of the loop.
1075+
log.warning() << "Found a `DEAD` variable "
1076+
<< name2string(symbol.get_identifier())
1077+
<< " without corresponding `DECL`, at: "
1078+
<< instruction_it->source_location() << messaget::eom;
10711079
}
10721080
}
10731081
else if(

0 commit comments

Comments
 (0)