Skip to content

Commit f580917

Browse files
committed
Support overflow plus/minus expressions over pointers
We did not specify that using overflow+/overflow- for pointer +/- integer was unsupported. The translation via regular arithmetic overflow, however, would produce results that did not take into account the peculiarities of pointer arithmetic. Fixing this now also makes it possible to take into account the implementation details of the object/offset encoding, which would result in overflows even when full-width addition/subtraction would not have overflowed. Fixes: #6842
1 parent dd10704 commit f580917

File tree

6 files changed

+139
-36
lines changed

6 files changed

+139
-36
lines changed

regression/cbmc/pointer-overflow3/no-simplify.desc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ main.c
55
^\[main.pointer_arithmetic.\d+\] line 7 pointer arithmetic: pointer outside object bounds in p - \(signed (long (long )?)?int\)10: FAILURE
66
^\[main.pointer_arithmetic.\d+\] line 10 pointer arithmetic: pointer outside object bounds in arr \+ \(signed (long (long )?)?int\)10: FAILURE
77
^\[main.pointer_arithmetic.\d+\] line 11 pointer arithmetic: pointer outside object bounds in arr - \(signed (long (long )?)?int\)10: FAILURE
8-
^\*\* 4 of \d+ failed
8+
^\[main.overflow.\d+\] line 7 arithmetic overflow on pointer \- in p \- \(signed (long (long )?)?int\)10: FAILURE
9+
^\[main.overflow.\d+\] line 11 arithmetic overflow on pointer \- in arr \- \(signed (long (long )?)?int\)10: FAILURE
10+
^\*\* 6 of \d+ failed
911
^VERIFICATION FAILED$
1012
^EXIT=10$
1113
^SIGNAL=0$

regression/cbmc/pointer-overflow3/test.desc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ main.c
55
^\[main.pointer_arithmetic.\d+\] line 7 pointer arithmetic: pointer outside object bounds in p - \(signed (long (long )?)?int\)10: FAILURE
66
^\[main.pointer_arithmetic.\d+\] line 10 pointer arithmetic: pointer outside object bounds in arr \+ \(signed (long (long )?)?int\)10: FAILURE
77
^\[main.pointer_arithmetic.\d+\] line 11 pointer arithmetic: pointer outside object bounds in arr - \(signed (long (long )?)?int\)10: FAILURE
8-
^\*\* 4 of \d+ failed
8+
^\[main.overflow.\d+\] line 7 arithmetic overflow on pointer \- in p \- \(signed (long (long )?)?int\)10: FAILURE
9+
^\[main.overflow.\d+\] line 11 arithmetic overflow on pointer \- in arr \- \(signed (long (long )?)?int\)10: FAILURE
10+
^\*\* 6 of \d+ failed
911
^VERIFICATION FAILED$
1012
^EXIT=10$
1113
^SIGNAL=0$

regression/cbmc/pointer-overflow4/test.desc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
CORE
22
main.c
33
--pointer-overflow-check
4-
^\[main.overflow.1\] line 10 arithmetic overflow on signed \* in (0x)?[0-9a-fA-F]+l* \* \(signed (long (long )?)?int\)4ul*: FAILURE$
4+
^\[main.overflow.1\] line 10 arithmetic overflow on pointer \+ in p \+ (0x)?[0-9a-fA-F]+l*: FAILURE$
55
^\*\* 1 of \d+ failed
66
^VERIFICATION FAILED$
77
^EXIT=10$

src/ansi-c/goto_check_c.cpp

Lines changed: 11 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -819,9 +819,6 @@ void goto_check_ct::integer_overflow_check(
819819
const exprt &expr,
820820
const guardt &guard)
821821
{
822-
if(!enable_signed_overflow_check && !enable_unsigned_overflow_check)
823-
return;
824-
825822
// First, check type.
826823
const typet &type = expr.type();
827824

@@ -831,6 +828,9 @@ void goto_check_ct::integer_overflow_check(
831828
if(type.id() == ID_unsignedbv && !enable_unsigned_overflow_check)
832829
return;
833830

831+
if(type.id() == ID_pointer && !enable_pointer_overflow_check)
832+
return;
833+
834834
// add overflow subgoal
835835

836836
if(expr.id() == ID_div)
@@ -1021,7 +1021,10 @@ void goto_check_ct::integer_overflow_check(
10211021
tmp.operands().resize(i);
10221022
}
10231023

1024-
std::string kind = type.id() == ID_unsignedbv ? "unsigned" : "signed";
1024+
std::string kind =
1025+
type.id() == ID_pointer
1026+
? "pointer"
1027+
: (type.id() == ID_unsignedbv ? "unsigned" : "signed");
10251028

10261029
add_guarded_property(
10271030
not_exprt{binary_overflow_exprt{tmp, expr.id(), expr.operands()[i]}},
@@ -1034,7 +1037,9 @@ void goto_check_ct::integer_overflow_check(
10341037
}
10351038
else if(expr.operands().size() == 2)
10361039
{
1037-
std::string kind = type.id() == ID_unsignedbv ? "unsigned" : "signed";
1040+
std::string kind = type.id() == ID_pointer
1041+
? "pointer"
1042+
: (type.id() == ID_unsignedbv ? "unsigned" : "signed");
10381043

10391044
const binary_exprt &bexpr = to_binary_expr(expr);
10401045
add_guarded_property(
@@ -1335,31 +1340,7 @@ void goto_check_ct::pointer_overflow_check(
13351340
expr.operands().size() == 2,
13361341
"pointer arithmetic expected to have exactly 2 operands");
13371342

1338-
// multiplying the offset by the object size must not result in arithmetic
1339-
// overflow
1340-
const typet &object_type = to_pointer_type(expr.type()).base_type();
1341-
if(object_type.id() != ID_empty)
1342-
{
1343-
auto size_of_expr_opt = size_of_expr(object_type, ns);
1344-
CHECK_RETURN(size_of_expr_opt.has_value());
1345-
exprt object_size = size_of_expr_opt.value();
1346-
1347-
const binary_exprt &binary_expr = to_binary_expr(expr);
1348-
exprt offset_operand = binary_expr.lhs().type().id() == ID_pointer
1349-
? binary_expr.rhs()
1350-
: binary_expr.lhs();
1351-
mult_exprt mul{
1352-
offset_operand,
1353-
typecast_exprt::conditional_cast(object_size, offset_operand.type())};
1354-
mul.add_source_location() = offset_operand.source_location();
1355-
1356-
flag_overridet override_overflow(offset_operand.source_location());
1357-
override_overflow.set_flag(
1358-
enable_signed_overflow_check, true, "signed_overflow_check");
1359-
override_overflow.set_flag(
1360-
enable_unsigned_overflow_check, true, "unsigned_overflow_check");
1361-
integer_overflow_check(mul, guard);
1362-
}
1343+
integer_overflow_check(expr, guard);
13631344

13641345
// the result must be within object bounds or one past the end
13651346
const auto size = from_integer(0, size_type());

src/solvers/flattening/bv_pointers.cpp

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ Author: Daniel Kroening, [email protected]
99
#include "bv_pointers.h"
1010

1111
#include <util/arith_tools.h>
12+
#include <util/bitvector_expr.h>
1213
#include <util/byte_operators.h>
1314
#include <util/c_types.h>
1415
#include <util/config.h>
@@ -204,6 +205,79 @@ literalt bv_pointerst::convert_rest(const exprt &expr)
204205
bv_utilst::representationt::SIGNED));
205206
}
206207
}
208+
else if(
209+
(expr.id() == ID_overflow_plus || expr.id() == ID_overflow_minus) &&
210+
operands.size() == 2 &&
211+
(operands[0].type().id() == ID_pointer) !=
212+
(operands[1].type().id() == ID_pointer))
213+
{
214+
// this has to be pointer plus/minus integer
215+
const exprt &pointer_op =
216+
operands[0].type().id() == ID_pointer ? operands[0] : operands[1];
217+
const pointer_typet &pointer_type = to_pointer_type(pointer_op.type());
218+
const typet &object_type = pointer_type.base_type();
219+
220+
const exprt &offset_op =
221+
operands[0].type().id() == ID_pointer ? operands[1] : operands[0];
222+
if(
223+
offset_op.type().id() != ID_unsignedbv &&
224+
offset_op.type().id() != ID_signedbv)
225+
{
226+
return SUB::convert_rest(expr);
227+
}
228+
exprt offset_bytes = offset_op;
229+
literalt offset_overflow = const_literal(false);
230+
231+
// Arithmetic over void* is a gcc extension.
232+
// https://gcc.gnu.org/onlinedocs/gcc-4.8.0/gcc/Pointer-Arith.html
233+
if(object_type.id() != ID_empty)
234+
{
235+
auto object_size_opt = size_of_expr(object_type, ns);
236+
CHECK_RETURN(object_size_opt.has_value());
237+
238+
offset_bytes = mult_exprt{
239+
offset_op,
240+
typecast_exprt::conditional_cast(*object_size_opt, offset_op.type())};
241+
242+
// multiplying the offset by the object size may result in arithmetic
243+
// overflow
244+
mult_overflow_exprt of{
245+
to_mult_expr(offset_bytes).op0(), to_mult_expr(offset_bytes).op1()};
246+
offset_overflow = convert(of);
247+
}
248+
249+
const std::size_t full_width = pointer_type.get_width();
250+
251+
bv_utilst::representationt rep = offset_bytes.type().id() == ID_signedbv
252+
? bv_utilst::representationt::SIGNED
253+
: bv_utilst::representationt::UNSIGNED;
254+
bvt offset_bv = convert_bv(offset_bytes);
255+
CHECK_RETURN(!offset_bv.empty());
256+
offset_bv = bv_utils.extension(offset_bv, full_width, rep);
257+
if(expr.id() == ID_overflow_minus)
258+
offset_bv = bv_utils.negate(offset_bv);
259+
260+
bvt pointer_offset_bv = bv_utils.sign_extension(
261+
offset_literals(convert_bv(pointer_op), pointer_type), full_width);
262+
263+
// add and determine (signed) overflow of this addition
264+
bvt offset_sum = bv_utils.add(pointer_offset_bv, offset_bv);
265+
literalt addition_overflow = prop.land(
266+
prop.lequal(
267+
bv_utils.sign_bit(pointer_offset_bv), bv_utils.sign_bit(offset_bv)),
268+
prop.lxor(
269+
bv_utils.sign_bit(offset_sum), bv_utils.sign_bit(pointer_offset_bv)));
270+
271+
// there is an overflow iff any of [offset width - 1 .. full_width - 1] bits
272+
// are non-zero or there was an overflow in the addition or the earlier
273+
// multiplication
274+
const std::size_t offset_bits = get_offset_width(pointer_type);
275+
CHECK_RETURN(offset_bits > 0);
276+
offset_sum.erase(offset_sum.begin(), offset_sum.begin() + offset_bits - 1);
277+
offset_sum.push_back(addition_overflow);
278+
offset_sum.push_back(offset_overflow);
279+
return prop.lor(offset_sum);
280+
}
207281

208282
return SUB::convert_rest(expr);
209283
}

src/solvers/smt2/smt2_conv.cpp

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1878,7 +1878,52 @@ void smt2_convt::convert_expr(const exprt &expr)
18781878
const typet &op_type = op0.type();
18791879
std::size_t width=boolbv_width(op_type);
18801880

1881-
if(op_type.id()==ID_signedbv)
1881+
if(op0.type().id() == ID_pointer || op1.type().id() == ID_pointer)
1882+
{
1883+
exprt p = op0, i = op1;
1884+
1885+
if(p.type().id() != ID_pointer)
1886+
p.swap(i);
1887+
1888+
const auto &object_type = to_pointer_type(p.type()).base_type();
1889+
exprt offset_bytes = i;
1890+
exprt::operandst disjuncts;
1891+
1892+
// Arithmetic over void* is a gcc extension.
1893+
// https://gcc.gnu.org/onlinedocs/gcc-4.8.0/gcc/Pointer-Arith.html
1894+
if(object_type.id() != ID_empty)
1895+
{
1896+
auto object_size_opt = size_of_expr(object_type, ns);
1897+
CHECK_RETURN(object_size_opt.has_value());
1898+
1899+
offset_bytes = mult_exprt{
1900+
i, typecast_exprt::conditional_cast(*object_size_opt, i.type())};
1901+
1902+
// multiplying the offset by the object size may result in arithmetic
1903+
// overflow
1904+
disjuncts.push_back(mult_overflow_exprt{
1905+
to_mult_expr(offset_bytes).op0(), to_mult_expr(offset_bytes).op1()});
1906+
}
1907+
1908+
exprt pointer_offset_op = pointer_offset(p);
1909+
offset_bytes = typecast_exprt::conditional_cast(
1910+
offset_bytes, pointer_offset_op.type());
1911+
1912+
if(can_cast_expr<minus_overflow_exprt>(expr))
1913+
offset_bytes = unary_minus_exprt{offset_bytes};
1914+
1915+
disjuncts.push_back(plus_overflow_exprt{pointer_offset_op, offset_bytes});
1916+
1917+
plus_exprt sum{pointer_offset_op, offset_bytes};
1918+
exprt pointer_offset_sum = pointer_offset(sum);
1919+
1920+
disjuncts.push_back(binary_relation_exprt{
1921+
pointer_offset_sum, ID_lt, from_integer(0, pointer_offset_sum.type())});
1922+
disjuncts.push_back(notequal_exprt{pointer_offset_sum, sum});
1923+
1924+
convert_expr(disjunction(disjuncts));
1925+
}
1926+
else if(op_type.id() == ID_signedbv)
18821927
{
18831928
// an overflow occurs if the top two bits of the extended sum differ
18841929
out << "(let ((?sum (";
@@ -1894,8 +1939,7 @@ void smt2_convt::convert_expr(const exprt &expr)
18941939
"((_ extract " << (width-1) << " " << (width-1) << ") ?sum)";
18951940
out << ")))"; // =, not, let
18961941
}
1897-
else if(op_type.id()==ID_unsignedbv ||
1898-
op_type.id()==ID_pointer)
1942+
else if(op_type.id() == ID_unsignedbv)
18991943
{
19001944
// overflow is simply carry-out
19011945
out << "(= ";

0 commit comments

Comments
 (0)