Skip to content

Floating-point normalization must not overflow #7617

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions regression/cbmc/Float-rounding2/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#include <assert.h>

int main()
{
#if 0
// examples of constants that previously exhibited wrong behaviour
union U
{
double d;
uint64_t b;
};
union U u = {
.b = 0b0000000000000011111111111111111111111110000001000000000000000000};
union U u2 = {
.b = 0b0000000000000000000000000000001111111111111111111111111000000000};
double d = u.d;
#else
double d;
#endif
float f;
if(d > 0.0 && d < 1.0)
{
f = d;
assert(f <= 1.0f);
}
return 0;
}
8 changes: 8 additions & 0 deletions regression/cbmc/Float-rounding2/test.desc
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
CORE
main.c

^EXIT=0$
^SIGNAL=0$
^VERIFICATION SUCCESSFUL$
--
^warning: ignoring
11 changes: 9 additions & 2 deletions src/solvers/floatbv/float_bv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,14 @@ exprt float_bvt::conversion(
// if the number was denormal and is normal in the new format,
// normalise it!
if(dest_spec.e > src_spec.e)
{
normalization_shift(result.fraction, result.exponent);
// normalization_shift unconditionally extends the exponent size to avoid
// arithmetic overflow, but this cannot have happened here as the exponent
// had already been extended to dest_spec's size
result.exponent =
typecast_exprt(result.exponent, signedbv_typet(dest_spec.e));
}

// the flags get copied
result.sign=unpacked_src.sign;
Expand Down Expand Up @@ -954,8 +961,8 @@ void float_bvt::normalization_shift(

std::size_t depth = address_bits(fraction_bits - 1);

if(exponent_bits<depth)
exponent=typecast_exprt(exponent, signedbv_typet(depth));
exponent = typecast_exprt(
exponent, signedbv_typet(std::max(depth, exponent_bits + 1)));

exprt exponent_delta=from_integer(0, exponent.type());

Expand Down
9 changes: 7 additions & 2 deletions src/solvers/floatbv/float_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,10 @@ bvt float_utilst::conversion(
if(dest_spec.e > spec.e)
{
normalization_shift(result.fraction, result.exponent);
// normalization_shift unconditionally extends the exponent size to avoid
// arithmetic overflow, but this cannot have happened here as the exponent
// had already been extended to dest_spec's size
result.exponent.resize(dest_spec.e);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain that one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment added.

}

// the flags get copied
Expand Down Expand Up @@ -792,8 +796,9 @@ void float_utilst::normalization_shift(bvt &fraction, bvt &exponent)
PRECONDITION(!fraction.empty());
std::size_t depth = address_bits(fraction.size() - 1);

if(exponent.size()<depth)
exponent=bv_utils.sign_extension(exponent, depth);
// sign-extend to ensure the arithmetic below cannot result in overflow/underflow
exponent =
bv_utils.sign_extension(exponent, std::max(depth, exponent.size() + 1));

bvt exponent_delta=bv_utils.zeros(exponent.size());

Expand Down