Re: strtold does not set errno when it should

Re: strtold does not set errno when it should

[Corinna Vinschen]

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

Hi Bruno,

[moving this discussion to the newlib list since that's newlib code]

On Dec 12 07:38, Bruno Haible wrote:
> POSIX [1] makes it clear that when the value to be returned would cause
> underflow, it should set errno to ERANGE.
> 
> [1] <https://pubs.opengroup.org/onlinepubs/9699919799/functions/strtod.html>
> 
> This test case fails with error code 4 on Cygwin 2.9.
> 
> ========================== foo.c ==========================
> #include <stdlib.h>
> #include <errno.h>
> #include <float.h>
> #include <math.h>
> 
> int main ()
> {
>   const char input[] = "1E-100000";
>   char *ptr;
>   long double result;
>   errno = 0;
>   result = strtold (input, &ptr);
>   if (!(ptr == input + 9))
>     return 1;
>   if (!(0.0L <= result && result <= LDBL_MIN))
>     return 2;
>   if (signbit (result))
>     return 3;
>   if (result == 0.0L && errno != ERANGE)
>     return 4;
>   return 0;
> }
> ============================================================
> $ gcc -Wall foo.c
> $ ./a.exe
> $ echo $?
> 4
> 
> The corresponding test case for strtod() / 'double' succeeds.

The code for strtold is almost verbatim taken from

  https://github.com/jwiegley/gdtoa.git

There haven't been any patches upstream.  I, for one, am not overly
fluent with FP math, so I'm glad for any help.

Looking into _strtodg_l (newlib/libc/stdlib/strtodg.c) I tried this
patch:

diff --git a/newlib/libc/stdlib/strtodg.c b/newlib/libc/stdlib/strtodg.c
index 013315946c1b..d6fb26ad3b45 100644
--- a/newlib/libc/stdlib/strtodg.c
+++ b/newlib/libc/stdlib/strtodg.c
@@ -1091,6 +1091,10 @@ _strtodg_l (struct _reent *p, const char *s00, char **se, FPI *fpi, Long *exp,
 				irv |= STRTOG_Underflow;
 			}
 		}
+#ifndef NO_ERRNO
+	if (irv & STRTOG_Underflow)
+		errno = ERANGE;
+#endif
 	if (se)
 		*se = (char *)s;
 	if (sign)

which seems to do the trick.  But, does it make sense?  If not, I'd
really appreciate a patch.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer

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

Re: strtold does not set errno when it should

[Bruno Haible]

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

Hi Corinna,

> The code for strtold is almost verbatim taken from
> 
>   https://github.com/jwiegley/gdtoa.git
> 
> There haven't been any patches upstream.

Probably that's because the code is good enough for ISO C compliance;
however, POSIX [1] adds the "ERANGE upon underflow" requirement,
whereas ISO C only has an "ERANGE upon overflow" requirement.

[1] <https://pubs.opengroup.org/onlinepubs/9699919799/functions/strtod.html>

> diff --git a/newlib/libc/stdlib/strtodg.c b/newlib/libc/stdlib/strtodg.c
> index 013315946c1b..d6fb26ad3b45 100644
> --- a/newlib/libc/stdlib/strtodg.c
> +++ b/newlib/libc/stdlib/strtodg.c
> @@ -1091,6 +1091,10 @@ _strtodg_l (struct _reent *p, const char *s00, char **se, FPI *fpi, Long *exp,
>  				irv |= STRTOG_Underflow;
>  			}
>  		}
> +#ifndef NO_ERRNO
> +	if (irv & STRTOG_Underflow)
> +		errno = ERANGE;
> +#endif
>  	if (se)
>  		*se = (char *)s;
>  	if (sign)
> 
> which seems to do the trick.  But, does it make sense?  If not, I'd
> really appreciate a patch.

I think it goes in the right direction.

It can be improved a bit, though: The existing code for overflow is careful
to do the errno stuff only when STRTOG_Overflow is being set/added. I.e.
keep this overflow/underflow handling out of the main path, in order not
to slow down the 99.99% of the cases that don't need it.

My attempt to do this would look like the attached patch. Untested.

Bruno

[-- Attachment #2: strtodg.c.diff --]
[-- Type: text/x-patch, Size: 1911 bytes --]

diff --git a/newlib/libc/stdlib/strtodg.c b/newlib/libc/stdlib/strtodg.c
index 0133159..743f60b 100644
--- a/newlib/libc/stdlib/strtodg.c
+++ b/newlib/libc/stdlib/strtodg.c
@@ -348,6 +348,9 @@ rvOK (struct _reent *p, double d, FPI *fpi, Long *exp, __ULong *bits, int exact,
 		if (k > nb || fpi->sudden_underflow) {
 			b->_wds = inex = 0;
 			*irv = STRTOG_Underflow | STRTOG_Inexlo;
+#ifndef NO_ERRNO
+			errno = ERANGE;
+#endif
 			}
 		else {
 			k1 = k - 1;
@@ -362,9 +365,15 @@ rvOK (struct _reent *p, double d, FPI *fpi, Long *exp, __ULong *bits, int exact,
 			if (carry) {
 				b = increment(p, b);
 				inex = STRTOG_Inexhi | STRTOG_Underflow;
+#ifndef NO_ERRNO
+				errno = ERANGE;
+#endif
 				}
 			else if (lostbits)
 				inex = STRTOG_Inexlo | STRTOG_Underflow;
+#ifndef NO_ERRNO
+				errno = ERANGE;
+#endif
 			}
 		}
 	else if (e > fpi->emax) {
@@ -761,6 +770,9 @@ _strtodg_l (struct _reent *p, const char *s00, char **se, FPI *fpi, Long *exp,
 					rvb->_x[0] = 0;
 					*exp = emin;
 					irv = STRTOG_Underflow | STRTOG_Inexlo;
+#ifndef NO_ERRNO
+					errno = ERANGE;
+#endif
 					goto ret;
 					}
 				rvb->_x[0] = rvb->_wds = rvbits = 1;
@@ -940,6 +952,9 @@ _strtodg_l (struct _reent *p, const char *s00, char **se, FPI *fpi, Long *exp,
 				rvb->_wds = 0;
 				rve = emin;
 				irv = STRTOG_Underflow | STRTOG_Inexlo;
+#ifndef NO_ERRNO
+				errno = ERANGE;
+#endif
 				break;
 				}
 			adj0 = dval(adj) = 1.;
@@ -1083,12 +1098,18 @@ _strtodg_l (struct _reent *p, const char *s00, char **se, FPI *fpi, Long *exp,
 		if (sudden_underflow) {
 			rvb->_wds = 0;
 			irv = STRTOG_Underflow | STRTOG_Inexlo;
+#ifndef NO_ERRNO
+			errno = ERANGE;
+#endif
 			}
 		else  {
 			irv = (irv & ~STRTOG_Retmask) |
 				(rvb->_wds > 0 ? STRTOG_Denormal : STRTOG_Zero);
 			if (irv & STRTOG_Inexact)
 				irv |= STRTOG_Underflow;
+#ifndef NO_ERRNO
+				errno = ERANGE;
+#endif
 			}
 		}
 	if (se)

Re: strtold does not set errno when it should

[Corinna Vinschen]

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

Hi Bruno,

On Dec 16 11:05, Bruno Haible wrote:
> Hi Corinna,
> 
> > The code for strtold is almost verbatim taken from
> > 
> >   https://github.com/jwiegley/gdtoa.git
> > 
> > There haven't been any patches upstream.
> 
> Probably that's because the code is good enough for ISO C compliance;
> however, POSIX [1] adds the "ERANGE upon underflow" requirement,
> whereas ISO C only has an "ERANGE upon overflow" requirement.
> 
> [1] <https://pubs.opengroup.org/onlinepubs/9699919799/functions/strtod.html>
> 
> > diff --git a/newlib/libc/stdlib/strtodg.c b/newlib/libc/stdlib/strtodg.c
> > index 013315946c1b..d6fb26ad3b45 100644
> > --- a/newlib/libc/stdlib/strtodg.c
> > +++ b/newlib/libc/stdlib/strtodg.c
> > @@ -1091,6 +1091,10 @@ _strtodg_l (struct _reent *p, const char *s00, char **se, FPI *fpi, Long *exp,
> >  				irv |= STRTOG_Underflow;
> >  			}
> >  		}
> > +#ifndef NO_ERRNO
> > +	if (irv & STRTOG_Underflow)
> > +		errno = ERANGE;
> > +#endif
> >  	if (se)
> >  		*se = (char *)s;
> >  	if (sign)
> > 
> > which seems to do the trick.  But, does it make sense?  If not, I'd
> > really appreciate a patch.
> 
> I think it goes in the right direction.
> 
> It can be improved a bit, though: The existing code for overflow is careful
> to do the errno stuff only when STRTOG_Overflow is being set/added. I.e.
> keep this overflow/underflow handling out of the main path, in order not
> to slow down the 99.99% of the cases that don't need it.
> 
> My attempt to do this would look like the attached patch. Untested.

That looks good to me and the result is the expected one.

If nobody complains in the next few hours, I'll check this in.
I *think* as of yet Cygwin is the only user of this code anyway.

Thanks!


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

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