Skip to content

Commit 087dabd

Browse files
committed
Fix pointer subtraction over integer constants
633875b replaced the simplification of `p - p` to zero, which previously was the only code to correctly evaluate to zero expressions like `(char *)n - (char *)n` for some integer `n` when using the SAT back-end: 5b8028a removed such support from the propositional back-end, as bounds-checking over the null object would have deemed each of the pointers out-of-bounds.
1 parent d60295d commit 087dabd

File tree

4 files changed

+72
-23
lines changed

4 files changed

+72
-23
lines changed

regression/cbmc/Pointer_difference1/main.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,11 @@ int main()
88
unsigned int diff2 = &arrayOfInt [1] - arrayOfInt;
99
__CPROVER_assert(diff2==1, "pointer diff2");
1010

11+
int zero = (char **)8 - (char **)8;
12+
__CPROVER_assert(zero == 0, "should be zero");
13+
14+
int ten = (char *)20 - (char *)10;
15+
__CPROVER_assert(ten == 10, "should be ten");
16+
1117
return 0;
1218
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
CORE
2+
main.c
3+
--no-simplify
4+
VERIFICATION SUCCESSFUL
5+
^EXIT=0$
6+
^SIGNAL=0$
7+
--
8+
^warning: ignoring
9+
^CONVERSION ERROR$

src/solvers/flattening/bv_pointers.cpp

Lines changed: 32 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -624,11 +624,6 @@ bvt bv_pointerst::convert_bitvector(const exprt &expr)
624624
const exprt same_object = ::same_object(minus_expr.lhs(), minus_expr.rhs());
625625
const literalt same_object_lit = convert(same_object);
626626

627-
// compute the object size (again, possibly using cached results)
628-
const exprt object_size = ::object_size(minus_expr.lhs());
629-
const bvt object_size_bv =
630-
bv_utils.zero_extension(convert_bv(object_size), width);
631-
632627
bvt bv = prop.new_variables(width);
633628

634629
if(!same_object_lit.is_false())
@@ -638,27 +633,11 @@ bvt bv_pointerst::convert_bitvector(const exprt &expr)
638633
const bvt lhs_offset =
639634
bv_utils.sign_extension(offset_literals(lhs, lhs_pt), width);
640635

641-
const literalt lhs_in_bounds = prop.land(
642-
!bv_utils.sign_bit(lhs_offset),
643-
bv_utils.rel(
644-
lhs_offset,
645-
ID_le,
646-
object_size_bv,
647-
bv_utilst::representationt::UNSIGNED));
648-
649636
const pointer_typet &rhs_pt = to_pointer_type(minus_expr.rhs().type());
650637
const bvt &rhs = convert_bv(minus_expr.rhs());
651638
const bvt rhs_offset =
652639
bv_utils.sign_extension(offset_literals(rhs, rhs_pt), width);
653640

654-
const literalt rhs_in_bounds = prop.land(
655-
!bv_utils.sign_bit(rhs_offset),
656-
bv_utils.rel(
657-
rhs_offset,
658-
ID_le,
659-
object_size_bv,
660-
bv_utilst::representationt::UNSIGNED));
661-
662641
bvt difference = bv_utils.sub(lhs_offset, rhs_offset);
663642

664643
// Support for void* is a gcc extension, with the size treated as 1 byte
@@ -678,9 +657,39 @@ bvt bv_pointerst::convert_bitvector(const exprt &expr)
678657
}
679658
}
680659

660+
// test for null object (integer constants)
661+
const exprt null_object = ::null_object(minus_expr.lhs());
662+
literalt in_bounds = convert(null_object);
663+
664+
if(!in_bounds.is_true())
665+
{
666+
// compute the object size (again, possibly using cached results)
667+
const exprt object_size = ::object_size(minus_expr.lhs());
668+
const bvt object_size_bv =
669+
bv_utils.zero_extension(convert_bv(object_size), width);
670+
671+
const literalt lhs_in_bounds = prop.land(
672+
!bv_utils.sign_bit(lhs_offset),
673+
bv_utils.rel(
674+
lhs_offset,
675+
ID_le,
676+
object_size_bv,
677+
bv_utilst::representationt::UNSIGNED));
678+
679+
const literalt rhs_in_bounds = prop.land(
680+
!bv_utils.sign_bit(rhs_offset),
681+
bv_utils.rel(
682+
rhs_offset,
683+
ID_le,
684+
object_size_bv,
685+
bv_utilst::representationt::UNSIGNED));
686+
687+
in_bounds =
688+
prop.lor(in_bounds, prop.land(lhs_in_bounds, rhs_in_bounds));
689+
}
690+
681691
prop.l_set_to_true(prop.limplies(
682-
prop.land(same_object_lit, prop.land(lhs_in_bounds, rhs_in_bounds)),
683-
bv_utils.equal(difference, bv)));
692+
prop.land(same_object_lit, in_bounds), bv_utils.equal(difference, bv)));
684693
}
685694

686695
return bv;

src/util/simplify_expr_int.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -614,6 +614,7 @@ simplify_exprt::simplify_minus(const minus_exprt &expr)
614614
if(
615615
offset_op0.is_constant() && offset_op1.is_constant() &&
616616
object_size.has_value() && element_size.has_value() &&
617+
element_size->is_constant() && !element_size->is_zero() &&
617618
numeric_cast_v<mp_integer>(to_constant_expr(offset_op0)) <=
618619
*object_size &&
619620
numeric_cast_v<mp_integer>(to_constant_expr(offset_op1)) <=
@@ -624,6 +625,30 @@ simplify_exprt::simplify_minus(const minus_exprt &expr)
624625
typecast_exprt{*element_size, minus_expr.type()}}));
625626
}
626627
}
628+
629+
const exprt &ptr_op0_skipped_tc = skip_typecast(ptr_op0);
630+
const exprt &ptr_op1_skipped_tc = skip_typecast(ptr_op1);
631+
if(
632+
is_number(ptr_op0_skipped_tc.type()) &&
633+
is_number(ptr_op1_skipped_tc.type()))
634+
{
635+
exprt offset_op0 = simplify_pointer_offset(
636+
pointer_offset_exprt{operands[0], minus_expr.type()});
637+
exprt offset_op1 = simplify_pointer_offset(
638+
pointer_offset_exprt{operands[1], minus_expr.type()});
639+
640+
auto element_size =
641+
size_of_expr(to_pointer_type(operands[0].type()).base_type(), ns);
642+
643+
if(
644+
element_size.has_value() && element_size->is_constant() &&
645+
!element_size->is_zero())
646+
{
647+
return changed(simplify_rec(div_exprt{
648+
minus_exprt{offset_op0, offset_op1},
649+
typecast_exprt{*element_size, minus_expr.type()}}));
650+
}
651+
}
627652
}
628653

629654
return unchanged(expr);

0 commit comments

Comments
 (0)