diff options
author | Fangyi Zhou <fangyi.zhou15@imperial.ac.uk> | 2018-10-28 12:28:53 -0400 |
---|---|---|
committer | Ben Gamari <ben@smart-cactus.org> | 2018-10-28 13:40:14 -0400 |
commit | 8e1a5593f940b9d2882a5e81abf028e7b99aff3a (patch) | |
tree | bf3a4b4c5f1855e8ed8e8aa27b80293164eaa480 | |
parent | c4a876d5f99554e87400946dd26ca11819d11673 (diff) | |
download | haskell-8e1a5593f940b9d2882a5e81abf028e7b99aff3a.tar.gz |
Fix integer overflow when encoding doubles (Trac #15271)
Summary:
Ticket #15271 reports a case where 1e1000000000 is incorrectly
converted to 0.0. After some investigation, I discovered the number is
converted to rational correctly, but converting the ratio into a double
introduced an error.
Tracking down to how the conversion is done, I found the rts float
implementation uses `ldexp`, whose signature is
`double ldexp (double x, int exp);`
The callsite passes an `I_` to the second argument, which is
platform-dependent. On machines where `I_` is 64 bits and `int` is 32 bits, we
observe integer overflow behaviour.
Here is a mapping from rational to exponent with observations
1e646457008 -> 2147483645 (result = infinity, positive in int32)
1e646457009 -> 2147483648 (result = 0.0, overflow to negative in int32)
1e1000000000 -> 3321928042 (result = infinity, overflow to positive in int32)
1e1555550000 -> 5167425196 (result = 0.0, overflow to negative in int32)
We fix this issue by comparing STG_INT_MIN/MAX and INT_MIN/MAX and bound the
value appropriately.
Test Plan: New test cases
Reviewers: bgamari, erikd, simonmar
Reviewed By: bgamari
Subscribers: rwbarton, carter
GHC Trac Issues: #15271
Differential Revision: https://phabricator.haskell.org/D5271
-rw-r--r-- | rts/StgPrimFloat.c | 26 | ||||
-rw-r--r-- | testsuite/tests/numeric/should_run/T15271.hs | 5 | ||||
-rw-r--r-- | testsuite/tests/numeric/should_run/T15271.stdout | 4 |
3 files changed, 31 insertions, 4 deletions
diff --git a/rts/StgPrimFloat.c b/rts/StgPrimFloat.c index 277ae66af5..f1f6736e01 100644 --- a/rts/StgPrimFloat.c +++ b/rts/StgPrimFloat.c @@ -14,6 +14,7 @@ #include <math.h> #include <float.h> +#include <limits.h> #define IEEE_FLOATING_POINT 1 @@ -47,6 +48,23 @@ #define __abs(a) (( (a) >= 0 ) ? (a) : (-(a))) +/** Trac #15271: Some large ratios are converted into double incorrectly. + * This occurs when StgInt has 64 bits and C int has 32 bits, where wrapping + * occurs and an incorrect signed value is passed into ldexp */ +STATIC_INLINE int +truncExponent(I_ e) +{ +#if INT_MAX < STG_INT_MAX + if (RTS_UNLIKELY(e > INT_MAX)) + e = INT_MAX; +#endif +#if INT_MIN > STG_INT_MIN + if (RTS_UNLIKELY(e < INT_MIN)) + e = INT_MIN; +#endif + return e; +} + /* Special version for words */ StgDouble __word_encodeDouble (W_ j, I_ e) @@ -57,7 +75,7 @@ __word_encodeDouble (W_ j, I_ e) /* Now raise to the exponent */ if ( r != 0.0 ) /* Lennart suggests this avoids a bug in MIPS's ldexp */ - r = ldexp(r, e); + r = ldexp(r, truncExponent(e)); return r; } @@ -72,7 +90,7 @@ __int_encodeDouble (I_ j, I_ e) /* Now raise to the exponent */ if ( r != 0.0 ) /* Lennart suggests this avoids a bug in MIPS's ldexp */ - r = ldexp(r, e); + r = ldexp(r, truncExponent(e)); /* sign is encoded in the size */ if (j < 0) @@ -91,7 +109,7 @@ __int_encodeFloat (I_ j, I_ e) /* Now raise to the exponent */ if ( r != 0.0 ) /* Lennart suggests this avoids a bug in MIPS's ldexp */ - r = ldexp(r, e); + r = ldexp(r, truncExponent(e)); /* sign is encoded in the size */ if (j < 0) @@ -110,7 +128,7 @@ __word_encodeFloat (W_ j, I_ e) /* Now raise to the exponent */ if ( r != 0.0 ) /* Lennart suggests this avoids a bug in MIPS's ldexp */ - r = ldexp(r, e); + r = ldexp(r, truncExponent(e)); return r; } diff --git a/testsuite/tests/numeric/should_run/T15271.hs b/testsuite/tests/numeric/should_run/T15271.hs new file mode 100644 index 0000000000..c8f2c95de5 --- /dev/null +++ b/testsuite/tests/numeric/should_run/T15271.hs @@ -0,0 +1,5 @@ +main = do + print 1e646457008 + print 1e646457009 -- T15271: This incorrectly printed 0.0 + print 1e1555550000 -- This is still infinity + print 1e1000000000 -- T15271: This incorrectly printed 0.0 diff --git a/testsuite/tests/numeric/should_run/T15271.stdout b/testsuite/tests/numeric/should_run/T15271.stdout new file mode 100644 index 0000000000..6e89f246c1 --- /dev/null +++ b/testsuite/tests/numeric/should_run/T15271.stdout @@ -0,0 +1,4 @@ +Infinity +Infinity +Infinity +Infinity |