Re: [PATCH 2/2] libm: Set math_errhandling to match library and hardware

["Keith Packard" <keithp@keithp.com>]

[-- Attachment #1: Type: text/plain, Size: 4611 bytes --]

Corinna Vinschen <vinschen@redhat.com> writes:

> Prior to this patch, math_errhandling on Cygwin was set to MATH_ERRNO.
> With this patch, math_errhandling on Cygwin is set to MATH_ERREXCEPT.

Oh, my test is *backwards*. _IEEE_LIBM means that the original math
functions never set errno. However, even inverted, my test is not
complete. In the non-_IEEE_LIBM case, it needs to check _LIB_VERSION !=
_IEEE_ as much of the old math library uses that to decide whether to
set errno or not.

Thanks for catching this! I didn't create a test case for this, which I
have done now.

> That may break backward compatibility.  From your patch I take it
> that not setting MATH_ERREXCEPT before was just missing, but actually
> changing the MATH_ERRNO bit looks wrong to me.

Yes, MATH_ERREXCEPT should be set on any platform that supports
exceptions. What that means for hardware that only supports exceptions
on a subset of floating point formats is unclear.

> Does that mean Cygwin just has to define _IEEE_LIBM somehwere?  I don't
> see that any target is setting _IEEE_LIBM anywhere...

I don't know -- right now, Cygwin is not setting _POSIX_MODE and so
applications are not getting errno set unless they set
_LIB_VERSION=_POSIX_, which I suspect no applications are even aware of.

The test I wrote uncovered some additional information about errno
handling in libm. There are three different mechanisms for controlling
errno in various parts of the library:

 1) _IEEE_LIBM. Used by some routines the old math library to only
    support IEEE (754 I assume) standards for math functions, which means
    not setting errno and (sometimes) applying slightly different
    semantics to the functions (!). Not universally supported (i.e.,
    some functions still set errno even when this flag is set).

 2) _LIB_VERSION. This global variable (named __fdlib_version) selects
    whether the library supports IEEE semantics or posix semantics at
    run time. The default value for this is _IEEE_ unless the
    configuration sets _POSIX_MODE, which is only set on the 'spu'
    host. As a result, the default math library configuration is to run
    in _IEEE_ mode, which doesn't set errno in lots of functions.

 3) WANT_ERRNO. This is used in most of the libm/common code to select
    whether math routines should set errno or not. All of the relevant
    code is found in math_err.c and math_errf.c. This talks about using
    math_errhandling but it doesn't actually do it. This is enabled by
    default, and no configurations appear to disable it.

I think we need to fix things so that the library is internally
consistent -- right now, there's no way to configure the library to
disable errno entirely, nor is there a way to control errno at runtime
in all cases.

It seems that there are three possible ways we might want to configure
newlib:

 1) No errno ever. Use exceptions to report errors back to
    applications.

 2) Always set errno. All exceptions always set errno.

 3) Allow applications to enable/disable errno setting at runtime.

Right now, you can get 1) by defining _IEEE_LIBM, setting WANT_ERRNO=0
and never touching _LIB_VERSION. Applications may still end up
referencing errno if they use functions which only check _LIB_VERSION.

You can get 2) by by defining _POSIX_MODE, not defining _IEEE_LIBM and
never setting _LIB_VERSION.

You can't get 3) because math/common doesn't use _LIB_VERSION.

Given that only SPU ever sets _POSIX_MODE, I'm tempted to suggest that
newlib should never set errno and report errors only through the
exception mechanism. Applications could detect this by checking the
value of math_errhandling, which would not include MATH_ERRNO.

I don't think there's any reasonable standard way to enable 3). The
_LIB_VERSION mechanism isn't great -- it's a global setting (not
per-thread), and it's not supported in either musl or glibc. Plus,
newlib doesn't support the 'matherr' function anyways, which is tightly
associated with _LIB_VERSION. The math_errhandling 'replacement' doesn't
offer any defined way to change the current settings as math_errhandling
may well be constant.

If we want to support 2) in addition to 1), we'll want a single global
definition that covers that mode. I'd suggest using the existing
_IEEE_LIBM value as that is used in a lot more places than the somewhat
ambiguously named WANT_ERRNO, and has a nice '_' prefix so we can expose
it to applications (as is required if we want to use it when defining
math_errhandling).

-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]