-
Notifications
You must be signed in to change notification settings - Fork 274
Add separate overflow expr classes #6744
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
19db9a0
0f6ae1c
5d7c21c
ce025e1
defc6d4
0d36c43
636ffa8
d4b17bd
533a902
fdc0fc8
12a4451
4e3e8b9
24c9da7
7936b57
98c80dc
631aeb4
c0f179c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,51 +6,59 @@ Author: Daniel Kroening, [email protected] | |
|
||
\*******************************************************************/ | ||
|
||
#include "boolbv.h" | ||
|
||
#include <util/bitvector_expr.h> | ||
#include <util/invariant.h> | ||
|
||
#include "boolbv.h" | ||
|
||
literalt boolbvt::convert_overflow(const exprt &expr) | ||
{ | ||
if(expr.id()==ID_overflow_plus || | ||
expr.id()==ID_overflow_minus) | ||
const auto plus_or_minus_conversion = | ||
[&]( | ||
const binary_overflow_exprt &overflow_expr, | ||
const std::function<literalt( | ||
bv_utilst *, const bvt &, const bvt &, bv_utilst::representationt)> | ||
&bv_util_overflow) { | ||
const bvt &bv0 = convert_bv(overflow_expr.lhs()); | ||
const bvt &bv1 = convert_bv(overflow_expr.rhs()); | ||
|
||
if(bv0.size() != bv1.size()) | ||
return SUB::convert_rest(expr); | ||
|
||
bv_utilst::representationt rep = | ||
overflow_expr.lhs().type().id() == ID_signedbv | ||
? bv_utilst::representationt::SIGNED | ||
: bv_utilst::representationt::UNSIGNED; | ||
|
||
return bv_util_overflow(&bv_utils, bv0, bv1, rep); | ||
}; | ||
if( | ||
const auto plus_overflow = expr_try_dynamic_cast<plus_overflow_exprt>(expr)) | ||
{ | ||
const auto &overflow_expr = to_binary_expr(expr); | ||
|
||
const bvt &bv0 = convert_bv(overflow_expr.lhs()); | ||
const bvt &bv1 = convert_bv(overflow_expr.rhs()); | ||
|
||
if(bv0.size()!=bv1.size()) | ||
return SUB::convert_rest(expr); | ||
|
||
bv_utilst::representationt rep = | ||
overflow_expr.lhs().type().id() == ID_signedbv | ||
? bv_utilst::representationt::SIGNED | ||
: bv_utilst::representationt::UNSIGNED; | ||
|
||
return expr.id()==ID_overflow_minus? | ||
bv_utils.overflow_sub(bv0, bv1, rep): | ||
bv_utils.overflow_add(bv0, bv1, rep); | ||
return plus_or_minus_conversion(*plus_overflow, &bv_utilst::overflow_add); | ||
} | ||
else if(expr.id()==ID_overflow_mult) | ||
if(const auto minus = expr_try_dynamic_cast<minus_overflow_exprt>(expr)) | ||
{ | ||
return plus_or_minus_conversion(*minus, &bv_utilst::overflow_sub); | ||
} | ||
else if( | ||
const auto mult_overflow = expr_try_dynamic_cast<mult_overflow_exprt>(expr)) | ||
{ | ||
const auto &overflow_expr = to_binary_expr(expr); | ||
|
||
if( | ||
overflow_expr.lhs().type().id() != ID_unsignedbv && | ||
overflow_expr.lhs().type().id() != ID_signedbv) | ||
mult_overflow->lhs().type().id() != ID_unsignedbv && | ||
mult_overflow->lhs().type().id() != ID_signedbv) | ||
return SUB::convert_rest(expr); | ||
|
||
bvt bv0 = convert_bv(overflow_expr.lhs()); | ||
bvt bv1 = convert_bv(overflow_expr.rhs(), bv0.size()); | ||
bvt bv0 = convert_bv(mult_overflow->lhs()); | ||
bvt bv1 = convert_bv(mult_overflow->rhs(), bv0.size()); | ||
|
||
bv_utilst::representationt rep = | ||
overflow_expr.lhs().type().id() == ID_signedbv | ||
mult_overflow->lhs().type().id() == ID_signedbv | ||
? bv_utilst::representationt::SIGNED | ||
: bv_utilst::representationt::UNSIGNED; | ||
|
||
DATA_INVARIANT( | ||
overflow_expr.lhs().type() == overflow_expr.rhs().type(), | ||
mult_overflow->lhs().type() == mult_overflow->rhs().type(), | ||
"operands of overflow_mult expression shall have same type"); | ||
|
||
std::size_t old_size=bv0.size(); | ||
|
@@ -89,18 +97,17 @@ literalt boolbvt::convert_overflow(const exprt &expr) | |
return !prop.lor(all_one, all_zero); | ||
} | ||
} | ||
else if(expr.id() == ID_overflow_shl) | ||
else if( | ||
const auto shl_overflow = expr_try_dynamic_cast<shl_overflow_exprt>(expr)) | ||
{ | ||
const auto &overflow_expr = to_binary_expr(expr); | ||
|
||
const bvt &bv0 = convert_bv(overflow_expr.lhs()); | ||
const bvt &bv1 = convert_bv(overflow_expr.rhs()); | ||
const bvt &bv0 = convert_bv(shl_overflow->lhs()); | ||
const bvt &bv1 = convert_bv(shl_overflow->rhs()); | ||
|
||
std::size_t old_size = bv0.size(); | ||
std::size_t new_size = old_size * 2; | ||
|
||
bv_utilst::representationt rep = | ||
overflow_expr.lhs().type().id() == ID_signedbv | ||
shl_overflow->lhs().type().id() == ID_signedbv | ||
? bv_utilst::representationt::SIGNED | ||
: bv_utilst::representationt::UNSIGNED; | ||
|
||
|
@@ -109,7 +116,7 @@ literalt boolbvt::convert_overflow(const exprt &expr) | |
bvt result=bv_utils.shift(bv_ext, bv_utilst::shiftt::SHIFT_LEFT, bv1); | ||
|
||
// a negative shift is undefined; yet this isn't an overflow | ||
literalt neg_shift = overflow_expr.lhs().type().id() == ID_unsignedbv | ||
literalt neg_shift = shl_overflow->lhs().type().id() == ID_unsignedbv | ||
? const_literal(false) | ||
: bv1.back(); // sign bit | ||
|
||
|
@@ -154,11 +161,11 @@ literalt boolbvt::convert_overflow(const exprt &expr) | |
return | ||
prop.land(!neg_shift, prop.lselect(undef, prop.lor(bv0), overflow)); | ||
} | ||
else if(expr.id()==ID_overflow_unary_minus) | ||
else if( | ||
const auto unary_minus_overflow = | ||
expr_try_dynamic_cast<unary_minus_overflow_exprt>(expr)) | ||
{ | ||
const auto &overflow_expr = to_unary_expr(expr); | ||
|
||
const bvt &bv = convert_bv(overflow_expr.op()); | ||
const bvt &bv = convert_bv(unary_minus_overflow->op()); | ||
|
||
return bv_utils.overflow_negate(bv); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -705,11 +705,12 @@ class binary_overflow_exprt : public binary_predicate_exprt | |
{ | ||
public: | ||
binary_overflow_exprt(exprt _lhs, const irep_idt &kind, exprt _rhs) | ||
: binary_predicate_exprt( | ||
std::move(_lhs), | ||
"overflow-" + id2string(kind), | ||
std::move(_rhs)) | ||
: binary_predicate_exprt(std::move(_lhs), make_id(kind), std::move(_rhs)) | ||
{ | ||
INVARIANT( | ||
valid_id(id()), | ||
"The kind used to construct binary_overflow_exprt should be in the set " | ||
"of expected valid kinds."); | ||
} | ||
|
||
static void check( | ||
|
@@ -735,13 +736,28 @@ class binary_overflow_exprt : public binary_predicate_exprt | |
{ | ||
check(expr, vm); | ||
} | ||
|
||
/// Returns true iff \p id is a valid identifier of a `binary_overflow_exprt`. | ||
static bool valid_id(const irep_idt &id) | ||
{ | ||
return id == ID_overflow_plus || id == ID_overflow_mult || | ||
id == ID_overflow_minus || id == ID_overflow_shl; | ||
} | ||
|
||
private: | ||
static irep_idt make_id(const irep_idt &kind) | ||
{ | ||
if(valid_id(kind)) | ||
return kind; | ||
else | ||
return "overflow-" + id2string(kind); | ||
} | ||
}; | ||
|
||
template <> | ||
inline bool can_cast_expr<binary_overflow_exprt>(const exprt &base) | ||
{ | ||
return base.id() == ID_overflow_plus || base.id() == ID_overflow_mult || | ||
base.id() == ID_overflow_minus || base.id() == ID_overflow_shl; | ||
return binary_overflow_exprt::valid_id(base.id()); | ||
} | ||
|
||
inline void validate_expr(const binary_overflow_exprt &value) | ||
|
@@ -778,6 +794,66 @@ inline binary_overflow_exprt &to_binary_overflow_expr(exprt &expr) | |
return ret; | ||
} | ||
|
||
class plus_overflow_exprt : public binary_overflow_exprt | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't these classes be used somewhere in the existing code? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had previously taken a look and it seems we consistently take shortcuts by creating There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have now pushed additional commits which use the new classes throughout the existing code where the IDs were previously being checked. |
||
{ | ||
public: | ||
plus_overflow_exprt(exprt _lhs, exprt _rhs) | ||
: binary_overflow_exprt(std::move(_lhs), ID_overflow_plus, std::move(_rhs)) | ||
{ | ||
} | ||
}; | ||
|
||
template <> | ||
inline bool can_cast_expr<plus_overflow_exprt>(const exprt &base) | ||
{ | ||
return base.id() == ID_overflow_plus; | ||
} | ||
|
||
class minus_overflow_exprt : public binary_overflow_exprt | ||
{ | ||
public: | ||
minus_overflow_exprt(exprt _lhs, exprt _rhs) | ||
: binary_overflow_exprt(std::move(_lhs), ID_overflow_minus, std::move(_rhs)) | ||
{ | ||
} | ||
}; | ||
|
||
template <> | ||
inline bool can_cast_expr<minus_overflow_exprt>(const exprt &base) | ||
{ | ||
return base.id() == ID_overflow_minus; | ||
} | ||
|
||
class mult_overflow_exprt : public binary_overflow_exprt | ||
{ | ||
public: | ||
mult_overflow_exprt(exprt _lhs, exprt _rhs) | ||
: binary_overflow_exprt(std::move(_lhs), ID_overflow_mult, std::move(_rhs)) | ||
{ | ||
} | ||
}; | ||
|
||
template <> | ||
inline bool can_cast_expr<mult_overflow_exprt>(const exprt &base) | ||
{ | ||
return base.id() == ID_overflow_mult; | ||
} | ||
|
||
class shl_overflow_exprt : public binary_overflow_exprt | ||
{ | ||
public: | ||
shl_overflow_exprt(exprt _lhs, exprt _rhs) | ||
: binary_overflow_exprt(std::move(_lhs), ID_overflow_shl, std::move(_rhs)) | ||
{ | ||
} | ||
}; | ||
|
||
template <> | ||
inline bool can_cast_expr<shl_overflow_exprt>(const exprt &base) | ||
{ | ||
return base.id() == ID_overflow_shl; | ||
} | ||
|
||
/// \brief A Boolean expression returning true, iff operation \c kind would | ||
/// result in an overflow when applied to the (single) operand. | ||
class unary_overflow_exprt : public unary_predicate_exprt | ||
|
@@ -816,6 +892,44 @@ inline void validate_expr(const unary_overflow_exprt &value) | |
value, 1, "unary overflow expression must have one operand"); | ||
} | ||
|
||
/// \brief A Boolean expression returning true, iff negation would result in an | ||
/// overflow when applied to the (single) operand. | ||
class unary_minus_overflow_exprt : public unary_overflow_exprt | ||
{ | ||
public: | ||
explicit unary_minus_overflow_exprt(exprt _op) | ||
: unary_overflow_exprt(ID_unary_minus, std::move(_op)) | ||
{ | ||
} | ||
|
||
static void check( | ||
const exprt &expr, | ||
const validation_modet vm = validation_modet::INVARIANT) | ||
{ | ||
unary_exprt::check(expr, vm); | ||
} | ||
|
||
static void validate( | ||
const exprt &expr, | ||
const namespacet &, | ||
const validation_modet vm = validation_modet::INVARIANT) | ||
{ | ||
check(expr, vm); | ||
} | ||
}; | ||
|
||
template <> | ||
inline bool can_cast_expr<unary_minus_overflow_exprt>(const exprt &base) | ||
{ | ||
return base.id() == ID_overflow_unary_minus; | ||
} | ||
|
||
inline void validate_expr(const unary_minus_overflow_exprt &value) | ||
{ | ||
validate_operands( | ||
value, 1, "unary minus overflow expression must have one operand"); | ||
} | ||
|
||
/// \brief Cast an exprt to a \ref unary_overflow_exprt | ||
/// | ||
/// \a expr must be known to be \ref unary_overflow_exprt. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I quite like this.
I favour things being correct by construction, and subsequently, for the invariants to be set as early as possible.