-
Notifications
You must be signed in to change notification settings - Fork 274
shift left overflow check now done in goto_check #3662
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
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 |
---|---|---|
@@ -1,11 +1,24 @@ | ||
int main() | ||
{ | ||
unsigned char x; | ||
|
||
// signed, owing to promotion, and may overflow | ||
unsigned r=x << ((sizeof(unsigned)-1)*8); | ||
|
||
// signed, owing to promotion, and cannot overflow | ||
r=x << ((sizeof(unsigned)-1)*8-1); | ||
|
||
// unsigned | ||
r=(unsigned)x << ((sizeof(unsigned)-1)*8); | ||
|
||
int s=-1 << ((sizeof(int)-1)*8); | ||
s=-256 << ((sizeof(int)-1)*8); | ||
// negative value or zero, not an overflow | ||
int s = -x << ((sizeof(int) - 1) * 8); | ||
|
||
// overflow | ||
s = 1 << x; | ||
|
||
// distance too far, not an overflow | ||
s = s << 100; | ||
|
||
return 0; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -589,6 +589,81 @@ void goto_checkt::integer_overflow_check( | |
|
||
return; | ||
} | ||
else if(expr.id() == ID_shl) | ||
{ | ||
if(type.id() == ID_signedbv) | ||
{ | ||
const auto &shl_expr = to_shl_expr(expr); | ||
const auto &op = shl_expr.op(); | ||
const auto &op_type = to_signedbv_type(type); | ||
const auto op_width = op_type.get_width(); | ||
const auto &distance = shl_expr.distance(); | ||
const auto &distance_type = distance.type(); | ||
|
||
// a left shift of a negative value is undefined; | ||
// yet this isn't an overflow | ||
exprt neg_value_shift; | ||
|
||
if(op_type.id() == ID_unsignedbv) | ||
neg_value_shift = false_exprt(); | ||
else | ||
neg_value_shift = | ||
binary_relation_exprt(op, ID_lt, from_integer(0, op_type)); | ||
|
||
// a shift with negative distance is undefined; | ||
// yet this isn't an overflow | ||
exprt neg_dist_shift; | ||
|
||
if(distance_type.id() == ID_unsignedbv) | ||
neg_dist_shift = false_exprt(); | ||
else | ||
neg_dist_shift = | ||
binary_relation_exprt(op, ID_lt, from_integer(0, distance_type)); | ||
|
||
// shifting a non-zero value by more than its width is undefined; | ||
// yet this isn't an overflow | ||
const exprt dist_too_large = binary_predicate_exprt( | ||
distance, ID_gt, from_integer(op_width, distance_type)); | ||
|
||
const exprt op_zero = equal_exprt(op, from_integer(0, op_type)); | ||
|
||
auto new_type = to_bitvector_type(op_type); | ||
new_type.set_width(op_width * 2); | ||
|
||
const exprt op_ext = typecast_exprt(op, new_type); | ||
|
||
const exprt op_ext_shifted = shl_exprt(op_ext, distance); | ||
|
||
// get top bits of the shifted operand | ||
const exprt top_bits = extractbits_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. ? Why shift and then extract? All the bits we're interested in testing are present in the original value. 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. Our extactbits matches the one in the SMT-LIB, and thus, only does constants. |
||
op_ext_shifted, | ||
new_type.get_width() - 1, | ||
op_width - 1, | ||
unsignedbv_typet(op_width + 1)); | ||
|
||
const exprt top_bits_zero = | ||
equal_exprt(top_bits, from_integer(0, top_bits.type())); | ||
|
||
// a negative distance shift isn't an overflow; | ||
// a negative value shift isn't an overflow; | ||
// a shift that's too far isn't an overflow; | ||
// a shift of zero isn't overflow; | ||
// else check the top bits | ||
add_guarded_claim( | ||
disjunction({neg_value_shift, | ||
neg_dist_shift, | ||
dist_too_large, | ||
op_zero, | ||
top_bits_zero}), | ||
"arithmetic overflow on signed shl", | ||
"overflow", | ||
expr.find_source_location(), | ||
expr, | ||
guard); | ||
} | ||
|
||
return; | ||
} | ||
|
||
exprt overflow("overflow-"+expr.id_string(), bool_typet()); | ||
overflow.operands()=expr.operands(); | ||
|
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.
Perhaps add a must-not-match for "line 12 arithmetic overflow" if that's intentionally not tested?
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.
ok