Re: [PATCH] Remove matherr, and SVID and X/Open math library configurations

Corinna Vinschen <[email protected]> Wed, 16 Jan 2019 09:50:00 +0100
Newsgroups	gmane.comp.lib.newlib
Message-ID	<[email protected]>
On Jan 15 21:37, Jozef Lawrynowicz wrote:
> Hi Corinna,
> 
> Did you have any thoughts on my proposals detailed below?
> 
> Thanks,
> Jozef
> 
> On Fri, 14 Dec 2018 12:31:47 +0000
> Jozef Lawrynowicz <[email protected]> wrote:
> 
> > Hi Corinna,
> > 
> > Thanks for the feedback.
> > 
> > On Thu, 13 Dec 2018 13:56:29 +0100
> > Corinna Vinschen <[email protected]> wrote:
> > 
> > > We have to keep matherr as exported symbol from Cygwin to maintain
> > > backward compatibility.  In contrast to embedded, we can't afford
> > > applications using this function to stop working with a new Cygwin
> > > release just because there's no matherr symbol anymore.  
> > 
> > I believe matherr is designed to only be called from the
> > floating-point arithmetic functions in the library, so if the
> > definition of matherr is removed along with all the uses of matherr,
> > users would still be able to link their applications.
> > 
> > > So, would it be feasible to convert the matherr functions to a stub
> > > which just returns 0 and otherwise just go ahead?  I'm pretty fuzzy
> > > on the implications this change has on old apps actually using
> > > matherr...  
> > 
> > The default matherr is already a stub and will always return 0, the
> > problem would be if the users' application code defines a more
> > sophisticated matherr which might fix up or report errors at runtime.
> > If they expect their customized matherr to be called after a
> > floating-point arithmetic error, then that will not happen anymore.

Here's the problem:  This would never work on Cygwin anyway, given the
way symbols are linked on PE/COFF targets.  The code inside newlib
would always use the newlib version.  Looks like nobody ever thought
about fixing that.

> > Since the default matherr is a stub, and the default libm
> > configuration is X/Open, the default behaviour is actually identical
> > to POSIX libm.
> > 
> > If we want application code with a customized matherr implementation
> > to fail to build, we can add a bad prototype for matherr to math.h.
> > So when users define their own matherr which uses the (old) correct
> > declaration, they will see an error:
> > 
> > >   sqrt.c:4:5: error: conflicting types for 'matherr'
> > >       4 | int matherr (struct exception *e)
> > >       |     ^~~~~~~
> > > In file included from sqrt.c:1:
> > > /usr/local/msp430-elf/include/math.h:571:12: note: previous
> > > declaration of 'matherr' was here 571 | extern int matherr (void *
> > > MATHERR_ERROR_HANDLING_IS_OBSOLETE, int x); |            ^~~~~~~  
> > 
> > So I think we have only two choices:
> > 1. Remove matherr completely (this is what the submitted patch does)
> >      Code which uses a customized matherr will build successfully
> >      But the customized matherr will never be called at runtime
> > 2. Remove all usage of matherr, but give matherr a different
> > prototype to what is expected
> >      Application code with a customized matherr will fail to build
> > 
> > It's also worth noting that since the previous default libm
> > configuration was X/Open, errno was set correctly. The new default is
> > IEEE, so errno does not get set. The only other configuration now
> > available is POSIX, which sets errno correctly. If we are concerned
> > about backwards compatibility, then perhaps for Cygwin the default
> > libm configuration should be POSIX, so errno still gets set.

We should use the same default as glibc.

> > What do you think is the best way forward?
> > 
> > Jozef

Your patch was fine, just the problem of stopping to export a symbol
from Cygwin worried me.

We don't want to break an older application to run just because we
removed a function from the DLL.  On reading your mail and further
reading up on matherr it seems this isn't really a problem.

If we *really* get a complaint on the Cygwin mailing list, we can
resurrect matherr just as an exportable symbol for Cygwin.

Can you please resend your patch?


Thanks,
Corinna
signature.asc (application/pgp-signature, 833 B)