Skip to content

bpo-31373: fix undefined floating-point demotions #3396

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.

Sign up for GitHub

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
benjaminp merged 1 commit into from
Sep 7, 2017
Merged

bpo-31373: fix undefined floating-point demotions #3396

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 23 additions & 11 deletions Include/pymath.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,14 +187,14 @@ PyAPI_FUNC(void) _Py_set_387controlword(unsigned short);
* result.
* Caution:
* This isn't reliable. C99 no longer requires libm to set errno under
* any exceptional condition, but does require +- HUGE_VAL return
* values on overflow. A 754 box *probably* maps HUGE_VAL to a
* double infinity, and we're cool if that's so, unless the input
* was an infinity and an infinity is the expected result. A C89
* system sets errno to ERANGE, so we check for that too. We're
* out of luck if a C99 754 box doesn't map HUGE_VAL to +Inf, or
* if the returned result is a NaN, or if a C89 box returns HUGE_VAL
* in non-overflow cases.
* any exceptional condition, but does require +- HUGE_VAL return
* values on overflow. A 754 box *probably* maps HUGE_VAL to a
* double infinity, and we're cool if that's so, unless the input
* was an infinity and an infinity is the expected result. A C89
* system sets errno to ERANGE, so we check for that too. We're
* out of luck if a C99 754 box doesn't map HUGE_VAL to +Inf, or
* if the returned result is a NaN, or if a C89 box returns HUGE_VAL
* in non-overflow cases.
* X is evaluated more than once.
* Some platforms have better way to spell this, so expect some #ifdef'ery.
*
Expand All @@ -211,8 +211,20 @@ PyAPI_FUNC(void) _Py_set_387controlword(unsigned short);
#define Py_OVERFLOWED(X) isinf(X)
#else
#define Py_OVERFLOWED(X) ((X) != 0.0 && (errno == ERANGE || \
(X) == Py_HUGE_VAL || \
(X) == -Py_HUGE_VAL))
#endif
(X) == Py_HUGE_VAL || \
(X) == -Py_HUGE_VAL))
#endif

/* Return whether integral type *type* is signed or not. */
#define _Py_IntegralTypeSigned(type) ((type)(-1) < 0)
/* Return the maximum value of integral type *type*. */
#define _Py_IntegralTypeMax(type) ((_Py_IntegralTypeSigned(type)) ? (((((type)1 << (sizeof(type)*CHAR_BIT - 2)) - 1) << 1) + 1) : ~(type)0)
/* Return the minimum value of integral type *type*. */
#define _Py_IntegralTypeMin(type) ((_Py_IntegralTypeSigned(type)) ? -_Py_IntegralTypeMax(type) - 1 : 0)
/* Check whether *v* is in the range of integral type *type*. This is most
* useful if *v* is floating-point, since demoting a floating-point *v* to an
* integral type that cannot represent *v*'s integral part is undefined
* behavior. */
#define _Py_InIntegralTypeRange(type, v) (_Py_IntegralTypeMin(type) <= v && v <= _Py_IntegralTypeMax(type))

#endif /* Py_PYMATH_H */
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Core and Builtins/2017-09-06-15-25-59.bpo-31373.dC4jd4.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix several possible instances of undefined behavior due to floating-point
demotions.
10 changes: 6 additions & 4 deletions Objects/floatobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2233,21 +2233,23 @@ _PyFloat_Pack4(double x, unsigned char *p, int le)

}
else {
float y = (float)x;
const unsigned char *s = (unsigned char*)&y;
int i, incr = 1;

if (Py_IS_INFINITY(y) && !Py_IS_INFINITY(x))

@vstinner vstinner Sep 6, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't understand why you remove Py_IS_INFINITY(y).

@benjaminp benjaminp Sep 6, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

y is no longer defined at this point.

if (fabs(x) > FLT_MAX && !Py_IS_INFINITY(x))
goto Overflow;

unsigned char s[sizeof(float)];

@vstinner vstinner Sep 6, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would be more confident with an assert(sizeof(float) == 4); just in case.

@benjaminp benjaminp Sep 6, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This code only executes if floats are in IEE754 format and are therefore 4 bytes.

float y = (float)x;
memcpy(s, &y, sizeof(float));

if ((float_format == ieee_little_endian_format && !le)
|| (float_format == ieee_big_endian_format && le)) {
p += 3;
incr = -1;
}

for (i = 0; i < 4; i++) {
*p = *s++;
*p = s[i];
p += incr;
}
return 0;
Expand Down
5 changes: 5 additions & 0 deletions Python/getargs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "Python.h"

#include <ctype.h>
#include <float.h>


#ifdef __cplusplus
Expand Down Expand Up @@ -858,6 +859,10 @@ convertsimple(PyObject *arg, const char **p_format, va_list *p_va, int flags,
double dval = PyFloat_AsDouble(arg);
if (PyErr_Occurred())
RETURN_ERR_OCCURRED;
else if (dval > FLT_MAX)
*p = (float)INFINITY;
else if (dval < -FLT_MAX)

@vstinner vstinner Sep 6, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would prefer to use FLT_MIN.

@benjaminp benjaminp Sep 6, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

FLT_MIN is the smallest nonnegative float not what we want here.

*p = (float)-INFINITY;
else
*p = (float) dval;
break;
Expand Down
32 changes: 16 additions & 16 deletions Python/pytime.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ static int
_PyTime_DoubleToDenominator(double d, time_t *sec, long *numerator,
double denominator, _PyTime_round_t round)
{
double intpart, err;
double intpart;
/* volatile avoids optimization changing how numbers are rounded */
volatile double floatpart;

Expand All @@ -115,14 +115,13 @@ _PyTime_DoubleToDenominator(double d, time_t *sec, long *numerator,
}
assert(0.0 <= floatpart && floatpart < denominator);

*sec = (time_t)intpart;
*numerator = (long)floatpart;

err = intpart - (double)*sec;
if (err <= -1.0 || err >= 1.0) {
if (!_Py_InIntegralTypeRange(time_t, intpart)) {
error_time_t_overflow();
return -1;
}
*sec = (time_t)intpart;
*numerator = (long)floatpart;

return 0;
}

Expand Down Expand Up @@ -150,20 +149,19 @@ int
_PyTime_ObjectToTime_t(PyObject *obj, time_t *sec, _PyTime_round_t round)
{
if (PyFloat_Check(obj)) {
double intpart, err;
double intpart;
/* volatile avoids optimization changing how numbers are rounded */
volatile double d;

d = PyFloat_AsDouble(obj);
d = _PyTime_Round(d, round);
(void)modf(d, &intpart);

*sec = (time_t)intpart;
err = intpart - (double)*sec;
if (err <= -1.0 || err >= 1.0) {
if (!_Py_InIntegralTypeRange(time_t, intpart)) {
error_time_t_overflow();
return -1;
}
*sec = (time_t)intpart;
return 0;
}
else {
Expand All @@ -180,7 +178,9 @@ _PyTime_ObjectToTimespec(PyObject *obj, time_t *sec, long *nsec,
{
int res;
res = _PyTime_ObjectToDenominator(obj, sec, nsec, 1e9, round);
assert(0 <= *nsec && *nsec < SEC_TO_NS);
if (res == 0) {
assert(0 <= *nsec && *nsec < SEC_TO_NS);
}
return res;
}

Expand All @@ -190,7 +190,9 @@ _PyTime_ObjectToTimeval(PyObject *obj, time_t *sec, long *usec,
{
int res;
res = _PyTime_ObjectToDenominator(obj, sec, usec, 1e6, round);
assert(0 <= *usec && *usec < SEC_TO_US);
if (res == 0) {
assert(0 <= *usec && *usec < SEC_TO_US);
}
return res;
}

Expand Down Expand Up @@ -276,7 +278,6 @@ static int
_PyTime_FromFloatObject(_PyTime_t *t, double value, _PyTime_round_t round,
long unit_to_ns)
{
double err;
/* volatile avoids optimization changing how numbers are rounded */
volatile double d;

Expand All @@ -285,12 +286,11 @@ _PyTime_FromFloatObject(_PyTime_t *t, double value, _PyTime_round_t round,
d *= (double)unit_to_ns;
d = _PyTime_Round(d, round);

*t = (_PyTime_t)d;
err = d - (double)*t;
if (fabs(err) >= 1.0) {
if (!_Py_InIntegralTypeRange(_PyTime_t, d)) {
_PyTime_overflow();
return -1;
}
*t = (_PyTime_t)d;
return 0;
}

Expand Down