Skip to content

Commit 9e164e3

Browse files
committed
Properly checks struct members in alias expression
Signed-off-by: Felipe R. Monteiro <felisous@amazon.com>
1 parent e60d94b commit 9e164e3

File tree

8 files changed

+217
-28
lines changed

8 files changed

+217
-28
lines changed
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
#include <assert.h>
2+
#include <stdlib.h>
3+
4+
struct pair
5+
{
6+
int x;
7+
int y;
8+
};
9+
10+
void f1(struct pair *p) __CPROVER_assigns(p->x)
11+
{
12+
p->y = 2;
13+
}
14+
15+
void f2(struct pair *p) __CPROVER_assigns(p->y)
16+
{
17+
p->x = 2;
18+
}
19+
20+
void f3(struct pair *p) __CPROVER_assigns(p->y)
21+
{
22+
p->y = 0;
23+
}
24+
25+
void f4(struct pair *p) __CPROVER_assigns(*p)
26+
{
27+
p = NULL;
28+
}
29+
30+
int main()
31+
{
32+
struct pair p = {0};
33+
f1(&p);
34+
f2(&p);
35+
assert(p.y == 2);
36+
assert(p.x == 2);
37+
f3(&p);
38+
assert(p.y == 0);
39+
f4(&p);
40+
return 0;
41+
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
CORE
2+
main.c
3+
--enforce-all-contracts
4+
^EXIT=10$
5+
^SIGNAL=0$
6+
^\[f1.\d+\] line \d+ Check that p\-\>y is assignable\: FAILURE$
7+
^\[f2.\d+\] line \d+ Check that p\-\>x is assignable\: FAILURE$
8+
^\[f3.\d+\] line \d+ Check that p\-\>y is assignable\: SUCCESS$
9+
^\[f4.\d+\] line \d+ Check that p is assignable\: FAILURE$
10+
^VERIFICATION FAILED$
11+
--
12+
--
13+
Checks whether CBMC properly evaluates write set of members
14+
from the same object.
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
#include <assert.h>
2+
3+
struct pair
4+
{
5+
int x[3];
6+
int y;
7+
};
8+
9+
int f1(struct pair *p) __CPROVER_assigns(p->x)
10+
{
11+
p->y = 2;
12+
p->x[0] = 0;
13+
p->x[1] = 1;
14+
p->x[2] = 2;
15+
return 0;
16+
}
17+
18+
int main()
19+
{
20+
struct pair p = {0};
21+
f1(&p);
22+
assert(p.y == 2);
23+
assert(p.x[0] == 0);
24+
assert(p.x[1] == 1);
25+
assert(p.x[2] == 2);
26+
return 0;
27+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
CORE
2+
main.c
3+
--enforce-all-contracts
4+
^EXIT=10$
5+
^SIGNAL=0$
6+
^\[f1.\d+\] line \d+ Check that p\-\>y is assignable\: FAILURE$
7+
^\[f1.\d+\] line \d+ Check that p\-\>x\[\(.*\)0\] is assignable\: SUCCESS$
8+
^\[f1.\d+\] line \d+ Check that p\-\>x\[\(.*\)1\] is assignable\: SUCCESS$
9+
^\[f1.\d+\] line \d+ Check that p\-\>x\[\(.*\)2\] is assignable\: SUCCESS$
10+
^VERIFICATION FAILED$
11+
--
12+
--
13+
Checks whether CBMC properly evaluates write set of members
14+
from the same object. In this case, we have an assigns clause
15+
with a struct member `x[3]` and an assignment to the struct member `y`.
16+
CBMC must considers only the region of `x[3]` is assignable.
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
#include <assert.h>
2+
#include <stdint.h>
3+
#include <stdlib.h>
4+
5+
struct pair
6+
{
7+
uint8_t *buf;
8+
size_t size;
9+
};
10+
11+
void f1(struct pair *p) __CPROVER_assigns(*(p->buf))
12+
{
13+
p->buf[0] = 0;
14+
p->buf[1] = 1;
15+
p->buf[2] = 2;
16+
p->size = 4;
17+
}
18+
19+
void f2(struct pair *p) __CPROVER_assigns(p->size)
20+
{
21+
p->buf[0] = 0;
22+
p->size = 0;
23+
}
24+
25+
void f3(struct pair *p) __CPROVER_assigns(*p)
26+
{
27+
p->buf = NULL;
28+
p->size = 0;
29+
}
30+
31+
int main()
32+
{
33+
struct pair *p = malloc(sizeof(*p));
34+
p->size = 3;
35+
p->buf = malloc(p->size);
36+
f1(p);
37+
assert(p->buf[0] == 0);
38+
assert(p->buf[1] == 1);
39+
assert(p->buf[2] == 2);
40+
f2(p);
41+
assert(p->size == 0);
42+
f3(p);
43+
assert(p->buf == NULL);
44+
assert(p->size == 0);
45+
return 0;
46+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
CORE
2+
main.c
3+
--enforce-all-contracts
4+
^EXIT=10$
5+
^SIGNAL=0$
6+
^\[f1.\d+\] line \d+ Check that p\-\>buf\[\(.*\)0\] is assignable\: SUCCESS$
7+
^\[f1.\d+\] line \d+ Check that p\-\>buf\[\(.*\)1\] is assignable\: SUCCESS$
8+
^\[f1.\d+\] line \d+ Check that p\-\>buf\[\(.*\)2\] is assignable\: SUCCESS$
9+
^\[f1.\d+\] line \d+ Check that p\-\>size is assignable\: FAILURE$
10+
^\[f2.\d+\] line \d+ Check that p\-\>buf\[\(.*\)0\] is assignable\: FAILURE$
11+
^\[f2.\d+\] line \d+ Check that p\-\>size is assignable\: SUCCESS$
12+
^\[f3.\d+\] line \d+ Check that p\-\>buf is assignable\: SUCCESS$
13+
^\[f3.\d+\] line \d+ Check that p\-\>size is assignable\: SUCCESS$
14+
^VERIFICATION FAILED$
15+
--
16+
--
17+
Checks whether CBMC properly evaluates write set of members
18+
from the same object. In this case, we have a dynamic object
19+
as one of the struct members.

src/goto-instrument/contracts/assigns.cpp

Lines changed: 50 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ assigns_clause_targett::assigns_clause_targett(
3131
INVARIANT(
3232
pointer_object.type().id() == ID_pointer,
3333
"Assigns clause targets should be stored as pointer expressions.");
34-
3534
const symbolt &function_symbol = contract.ns.lookup(function_id);
3635

3736
// Declare a new symbol to stand in for the reference
@@ -60,15 +59,49 @@ std::vector<symbol_exprt> assigns_clause_targett::temporary_declarations() const
6059
return result;
6160
}
6261

63-
exprt assigns_clause_targett::alias_expression(const exprt &ptr)
62+
exprt assigns_clause_targett::alias_expression(const exprt &lhs)
6463
{
65-
return same_object(ptr, local_target);
64+
exprt::operandst condition;
65+
exprt lhs_ptr = (lhs.id() == ID_address_of) ? to_address_of_expr(lhs).object()
66+
: pointer_for(lhs);
67+
68+
// __CPROVER_same_object(lhs, target)
69+
condition.push_back(same_object(lhs_ptr, target));
70+
71+
// If assigns target was a dereference, comparing objects is enough
72+
if(target_id == ID_dereference)
73+
{
74+
return conjunction(condition);
75+
}
76+
77+
const exprt lhs_offset = pointer_offset(lhs_ptr);
78+
const exprt target_offset = pointer_offset(target);
79+
80+
// __CPROVER_offset(lhs) >= __CPROVER_offset(target)
81+
condition.push_back(binary_relation_exprt(lhs_offset, ID_ge, target_offset));
82+
83+
const exprt region_lhs = plus_exprt(
84+
typecast_exprt::conditional_cast(
85+
size_of_expr(lhs.type(), contract.ns).value(), lhs_offset.type()),
86+
lhs_offset);
87+
88+
const exprt region_target = plus_exprt(
89+
typecast_exprt::conditional_cast(
90+
size_of_expr(dereference_exprt(local_target).type(), contract.ns).value(),
91+
target_offset.type()),
92+
target_offset);
93+
94+
// (sizeof(lhs) + __CPROVER_offset(lhs)) <=
95+
// (sizeof(target) + __CPROVER_offset(target))
96+
condition.push_back(binary_relation_exprt(region_lhs, ID_le, region_target));
97+
98+
return conjunction(condition);
6699
}
67100

68101
exprt assigns_clause_targett::compatible_expression(
69102
const assigns_clause_targett &called_target)
70103
{
71-
return alias_expression(called_target.get_direct_pointer());
104+
return same_object(called_target.get_direct_pointer(), local_target);
72105
}
73106

74107
goto_programt
@@ -107,9 +140,9 @@ assigns_clauset::assigns_clauset(
107140
function_id(function_id),
108141
log(log_parameter)
109142
{
110-
for(exprt current_operation : assigns_expr.operands())
143+
for(exprt target : assigns_expr.operands())
111144
{
112-
add_target(current_operation);
145+
add_target(target);
113146
}
114147
}
115148

@@ -121,17 +154,17 @@ assigns_clauset::~assigns_clauset()
121154
}
122155
}
123156

124-
assigns_clause_targett *assigns_clauset::add_target(exprt current_operation)
157+
assigns_clause_targett *assigns_clauset::add_target(exprt target)
125158
{
126-
assigns_clause_targett *target = new assigns_clause_targett(
127-
(current_operation.id() == ID_address_of)
128-
? to_index_expr(to_address_of_expr(current_operation).object()).array()
129-
: current_operation,
159+
assigns_clause_targett *new_target = new assigns_clause_targett(
160+
(target.id() == ID_address_of)
161+
? to_index_expr(to_address_of_expr(target).object()).array()
162+
: target,
130163
parent,
131164
log,
132165
function_id);
133-
targets.push_back(target);
134-
return target;
166+
targets.push_back(new_target);
167+
return new_target;
135168
}
136169

137170
assigns_clause_targett *
@@ -239,28 +272,18 @@ goto_programt assigns_clauset::havoc_code(
239272

240273
exprt assigns_clauset::alias_expression(const exprt &lhs)
241274
{
275+
// If write set is empty, no assignment is allowed.
242276
if(targets.empty())
243277
{
244278
return false_exprt();
245279
}
246280

247-
exprt left_ptr = assigns_clause_targett::pointer_for(lhs);
248-
249-
bool first_iter = true;
250-
exprt result = false_exprt();
281+
exprt::operandst condition;
251282
for(assigns_clause_targett *target : targets)
252283
{
253-
if(first_iter)
254-
{
255-
result = target->alias_expression(left_ptr);
256-
first_iter = false;
257-
}
258-
else
259-
{
260-
result = or_exprt(result, target->alias_expression(left_ptr));
261-
}
284+
condition.push_back(target->alias_expression(lhs));
262285
}
263-
return result;
286+
return disjunction(condition);
264287
}
265288

266289
exprt assigns_clauset::compatible_expression(

src/goto-instrument/contracts/assigns.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ Date: July 2021
1616

1717
#include "contracts.h"
1818

19+
#include <ansi-c/expr2c.h>
20+
#include <util/pointer_offset_size.h>
21+
1922
/// \brief A base class for assigns clause targets
2023
class assigns_clause_targett
2124
{
@@ -57,7 +60,7 @@ class assigns_clauset
5760
messaget log_parameter);
5861
~assigns_clauset();
5962

60-
assigns_clause_targett *add_target(exprt current_operation);
63+
assigns_clause_targett *add_target(exprt target);
6164
assigns_clause_targett *add_pointer_target(exprt current_operation);
6265
goto_programt init_block(source_locationt location);
6366
goto_programt &temporary_declarations(

0 commit comments

Comments
 (0)