Skip to content

Don't rely on errno values, coming from platform's math functions #156145

Description

@skirpichev

Feature or enhancement

Proposal:

In the cmath module most functions don't check errno values. See cosh() example below, which set nonzero errno in some cases, but don't read errno (e.g. to detect overflow in libm's cosh/sinh()). Here we essentially assume, that platform implements Annex F of the C standard:

static Py_complex
cmath_cosh_impl(PyObject *module, Py_complex z)
/*[clinic end generated code: output=2e969047da601bdb input=d6b66339e9cc332b]*/
{
Py_complex r;
double x_minus_one;
/* special treatment for cosh(+/-inf + iy) if y is not a NaN */
if (!isfinite(z.real) || !isfinite(z.imag)) {
if (isinf(z.real) && isfinite(z.imag) &&
(z.imag != 0.)) {
if (z.real > 0) {
r.real = copysign(INF, cos(z.imag));
r.imag = copysign(INF, sin(z.imag));
}
else {
r.real = copysign(INF, cos(z.imag));
r.imag = -copysign(INF, sin(z.imag));
}
}
else {
r = cosh_special_values[special_type(z.real)]
[special_type(z.imag)];
}
/* need to set errno = EDOM if y is +/- infinity and x is not
a NaN */
if (isinf(z.imag) && !isnan(z.real))
errno = EDOM;
else
errno = 0;
return r;
}
if (fabs(z.real) > CM_LOG_LARGE_DOUBLE) {
/* deal correctly with cases where cosh(z.real) overflows but
cosh(z) does not. */
x_minus_one = z.real - copysign(1., z.real);
r.real = cos(z.imag) * cosh(x_minus_one) * Py_MATH_E;
r.imag = sin(z.imag) * sinh(x_minus_one) * Py_MATH_E;
} else {
r.real = cos(z.imag) * cosh(z.real);
r.imag = sin(z.imag) * sinh(z.real);
}
/* detect overflow, and set errno accordingly */
if (isinf(r.real) || isinf(r.imag))
errno = ERANGE;
else
errno = 0;
return r;
}

What if we extend this approach to the rest of CPython's floating-point arithmetics? Now this seems natural, as we require that platforms C double has IEEE 754 binary64 format. On practice, that means much more, i.e. that platform supports Annex F (modulo bugs). And we can utilize that, assuming that for invalid input - NaN's produced, for overflows - infinities.

For example, instead of current code of float_pow() that handle finite input

errno = 0;
ix = pow(iv, iw);
_Py_ADJUST_ERANGE1(ix);
if (negate_result)
ix = -ix;
if (errno != 0) {
/* We don't expect any errno value other than ERANGE, but
* the range of libm bugs appears unbounded.
*/
PyErr_SetFromErrno(errno == ERANGE ? PyExc_OverflowError :
PyExc_ValueError);
return NULL;
}

we could use something much more simple:

    ix = pow(iv, iw);
    if (negate_result)
        ix = -ix;
    if (isinf(ix)) {
        PyErr_SetString(PyExc_OverflowError,
                        "float exponentiation result out of range");
        return NULL;
    }

(_Py_ADJUST_ERANGE1 and _Py_ADJUST_ERANGE2 helpers will be removed.)

See recent d.p.o thread for illustration of subtle issues, that can be introduced trying to fix errno, coming from platforms functions. We also have occurring bugreports, when platform libm set errno wrongly, e.g. #153144.

Disclaimer: this issue filled, based on my humble understanding of the proposal by @mdickinson. It seems, there are volunteers to work on this (CC @hpkfft), hence issue was opened.

Has this already been discussed elsewhere?

I have already discussed this feature proposal on Discourse

Links to previous discussion of this feature:

https://discuss.python.org/t/108539/

Metadata

Metadata

Assignees

No one assigned

    Labels

    extension-modulesC modules in the Modules dirinterpreter-core(Objects, Python, Grammar, and Parser dirs)type-featureA feature request or enhancement

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions