[PATCH] stdlib: Make strtod/strtof set ERANGE consistently for underflow.

[PATCH] stdlib: Make strtod/strtof set ERANGE consistently for underflow.

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 834 bytes --]


(I suspect this patch is incomplete, but it covers all of the test cases
I've got. Help identifying additional places where this bug might exist
would be greatly appreciated)

The C standard says that errno may acquire the value ERANGE if the
result from strtod underflows. According to IEEE 754, underflow occurs
whenever the value cannot be represented in normalized form.

Newlib is inconsistent in this, setting errno to ERANGE only if the
value underflows to zero, but not for denorm values, and never for hex
format floats.

This patch attempts to consistently set errno to ERANGE for all
'underflow' conditions, which is to say all values which are not
exactly zero and which cannot be represented in normalized form.

This matches glibc behavior, as well as the Linux, Mac OS X, OpenBSD,
FreeBSD and SunOS strtod man pages.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-stdlib-Make-strtod-strtof-set-ERANGE-consistently-fo.patch --]
[-- Type: text/x-diff, Size: 3121 bytes --]

From 3464e947eb12dd27c11fd88ef6a75ad0e560542d Mon Sep 17 00:00:00 2001
From: Keith Packard <keithp@keithp.com>
Date: Tue, 22 Jun 2021 10:26:26 -0700
Subject: [PATCH] stdlib: Make strtod/strtof set ERANGE consistently for
 underflow.

The C standard says that errno may acquire the value ERANGE if the
result from strtod underflows. According to IEEE 754, underflow occurs
whenever the value cannot be represented in normalized form.

Newlib is inconsistent in this, setting errno to ERANGE only if the
value underflows to zero, but not for denorm values, and never for hex
format floats.

This patch attempts to consistently set errno to ERANGE for all
'underflow' conditions, which is to say all values which are not
exactly zero and which cannot be represented in normalized form.

This matches glibc behavior, as well as the Linux, Mac OS X, OpenBSD,
FreeBSD and SunOS strtod man pages.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 newlib/libc/stdlib/strtod.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/newlib/libc/stdlib/strtod.c b/newlib/libc/stdlib/strtod.c
index 8bb75ef0a..019416ca7 100644
--- a/newlib/libc/stdlib/strtod.c
+++ b/newlib/libc/stdlib/strtod.c
@@ -326,6 +326,11 @@ _strtod_l (struct _reent *ptr, const char *__restrict s00, char **__restrict se,
 					Bfree(ptr,bb);
 					}
 				ULtod(rv.i, bits, exp, i);
+#ifndef NO_ERRNO
+                                /* try to avoid the bug of testing an 8087 register value */
+                                if ((dword0(rv)&Exp_mask) == 0)
+                                    errno = ERANGE;
+#endif
 			  }}
 			goto ret;
 		  }
@@ -1238,7 +1243,7 @@ _strtod_l (struct _reent *ptr, const char *__restrict s00, char **__restrict se,
 		dval(rv) *= dval(rv0);
 #ifndef NO_ERRNO
 		/* try to avoid the bug of testing an 8087 register value */
-		if (dword0(rv) == 0 && dword1(rv) == 0)
+		if ((dword0(rv) & Exp_mask) == 0)
 			ptr->_errno = ERANGE;
 #endif
 		}
@@ -1298,6 +1303,28 @@ strtof_l (const char *__restrict s00, char **__restrict se, locale_t loc)
   return retval;
 }
 
+/*
+ * These two functions are not quite correct as they return true for
+ * zero, however they are 'good enough' for the test in strtof below
+ * as we only need to know whether the double test is false when
+ * the float test is true.
+ */
+static inline int
+isdenorm(double d)
+{
+    U u;
+    dval(u) = d;
+    return (dword0(u) & Exp_mask) == 0;
+}
+
+static inline int
+isdenormf(float f)
+{
+    union { float f; __uint32_t i; } u;
+    u.f = f;
+    return (u.i & 0x7f800000) == 0;
+}
+
 float
 strtof (const char *__restrict s00,
 	char **__restrict se)
@@ -1307,7 +1334,7 @@ strtof (const char *__restrict s00,
     return signbit (val) ? -nanf ("") : nanf ("");
   float retval = (float) val;
 #ifndef NO_ERRNO
-  if (isinf (retval) && !isinf (val))
+  if ((isinf (retval) && !isinf (val)) || (isdenormf(retval) && !isdenorm(val)))
     _REENT->_errno = ERANGE;
 #endif
   return retval;
-- 
2.32.0


[-- Attachment #1.3: Type: text/plain, Size: 15 bytes --]


-- 
-keith

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

Re: [PATCH] stdlib: Make strtod/strtof set ERANGE consistently for underflow.

[Joseph Myers]

On Tue, 22 Jun 2021, Keith Packard wrote:

> This patch attempts to consistently set errno to ERANGE for all
> 'underflow' conditions, which is to say all values which are not
> exactly zero and which cannot be represented in normalized form.
> 
> This matches glibc behavior, as well as the Linux, Mac OS X, OpenBSD,
> FreeBSD and SunOS strtod man pages.

What glibc does is set it when the IEEE underflow exception flag is 
raised.  That is, when the result is tiny and inexact, where "tiny" 
follows each architecture's choice of before-rounding or after-rounding 
tininess detection (neither of which means "the rounded result is 
subnormal"; for both definitions of tiny results, there are values that 
round to the least-magnitude normal value of their sign but are still 
considered tiny, because after-rounding actually means "the result would 
have a subnormal exponent if rounded to the same precision as for normal 
values but with a wider exponent range").

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] stdlib: Make strtod/strtof set ERANGE consistently for underflow.

[Keith Packard]

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

Joseph Myers <joseph@codesourcery.com> writes:

>> This matches glibc behavior, as well as the Linux, Mac OS X, OpenBSD,
>> FreeBSD and SunOS strtod man pages.

(I should have said 'matches glibc in all of my tests, which are not
exhaustive' :-)

> What glibc does is set it when the IEEE underflow exception flag is 
> raised.  That is, when the result is tiny and inexact, where "tiny" 
> follows each architecture's choice of before-rounding or after-rounding 
> tininess detection (neither of which means "the rounded result is 
> subnormal"; for both definitions of tiny results, there are values that 
> round to the least-magnitude normal value of their sign but are still 
> considered tiny, because after-rounding actually means "the result would 
> have a subnormal exponent if rounded to the same precision as for normal 
> values but with a wider exponent range").

Thanks for this clarification; I didn't realize that underflow had this
additional subtly. It sounds like this means that there are additional
cases where errno should be set to ERANGE in the code. I'm afraid I
don't understand the workings of this function well enough to know how
to detect this particular case. Could we somehow detect this using
exceptions? Or are there places in the code which could add some direct
checks?

-- 
-keith

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

Re: [PATCH] stdlib: Make strtod/strtof set ERANGE consistently for underflow.

[Joseph Myers]

On Tue, 22 Jun 2021, Keith Packard wrote:

> Thanks for this clarification; I didn't realize that underflow had this
> additional subtly. It sounds like this means that there are additional
> cases where errno should be set to ERANGE in the code. I'm afraid I
> don't understand the workings of this function well enough to know how
> to detect this particular case. Could we somehow detect this using
> exceptions? Or are there places in the code which could add some direct
> checks?

Detecting with exceptions is tricky, since (a) you'd need to save and 
clear any underflow exceptions raised before strtod is called, then merge 
them with the exceptions raised in strtod before returning and (b) the 
strtod code would also need to ensure that the correct exceptions are 
raised in all cases, which itself is tricky (and once you have logic to 
ensure the correct exceptions are raised in all cases, it's probably 
simpler to use that logic to drive setting errno as well, rather than 
deducing the errno setting from what exceptions were raised).  I'm not 
familiar enough with the newlib strtod code to know where you might need 
to add direct checks (including dealing with architecture-specific 
configuration of whether tininess is detected before and after rounding).

(The sort of case where before-rounding versus after-rounding tininess 
detection matters and a result rounding to the least normal value is still 
considered to underflow is: for round-to-nearest, if the mathematical 
result is between 1/2 (inclusive) and 3/4 (exclusive) of the way from the 
greatest subnormal value to the least normal value, then it rounds to the 
least normal value but still is considered to underflow for both 
before-rounding and after-rounding tininess detection; if it's at least 
3/4 of the way from the greatest subnormal value to the least normal 
value, it only underflows for before-rounding tininess detection but not 
for after-rounding tininess detection.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] stdlib: Make strtod/strtof set ERANGE consistently for underflow.

[Jeff Johnston]

Hi Joseph,

While I realize that glibc does things differently and there are cases
still not handled, do you object to merging Keith's patch as-is?  If not,
I'll push it.

-- Jeff J.

On Tue, Jun 22, 2021 at 5:26 PM Keith Packard <keithp@keithp.com> wrote:

> Joseph Myers <joseph@codesourcery.com> writes:
>
> >> This matches glibc behavior, as well as the Linux, Mac OS X, OpenBSD,
> >> FreeBSD and SunOS strtod man pages.
>
> (I should have said 'matches glibc in all of my tests, which are not
> exhaustive' :-)
>
> > What glibc does is set it when the IEEE underflow exception flag is
> > raised.  That is, when the result is tiny and inexact, where "tiny"
> > follows each architecture's choice of before-rounding or after-rounding
> > tininess detection (neither of which means "the rounded result is
> > subnormal"; for both definitions of tiny results, there are values that
> > round to the least-magnitude normal value of their sign but are still
> > considered tiny, because after-rounding actually means "the result would
> > have a subnormal exponent if rounded to the same precision as for normal
> > values but with a wider exponent range").
>
> Thanks for this clarification; I didn't realize that underflow had this
> additional subtly. It sounds like this means that there are additional
> cases where errno should be set to ERANGE in the code. I'm afraid I
> don't understand the workings of this function well enough to know how
> to detect this particular case. Could we somehow detect this using
> exceptions? Or are there places in the code which could add some direct
> checks?
>
> --
> -keith
>

Re: [PATCH] stdlib: Make strtod/strtof set ERANGE consistently for underflow.

[Joseph Myers]

On Mon, 28 Jun 2021, Jeff Johnston wrote:

> Hi Joseph,
> 
> While I realize that glibc does things differently and there are cases
> still not handled, do you object to merging Keith's patch as-is?  If not,
> I'll push it.

I have no objections to merging this patch.  (I have not reviewed it.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] stdlib: Make strtod/strtof set ERANGE consistently for underflow.

[Jeff Johnston]

Hi Keith,

Thanks for the patch.  I have pushed it to master.

-- Jeff J.

On Tue, Jun 22, 2021 at 1:49 PM Keith Packard <keithp@keithp.com> wrote:

>
> (I suspect this patch is incomplete, but it covers all of the test cases
> I've got. Help identifying additional places where this bug might exist
> would be greatly appreciated)
>
> The C standard says that errno may acquire the value ERANGE if the
> result from strtod underflows. According to IEEE 754, underflow occurs
> whenever the value cannot be represented in normalized form.
>
> Newlib is inconsistent in this, setting errno to ERANGE only if the
> value underflows to zero, but not for denorm values, and never for hex
> format floats.
>
> This patch attempts to consistently set errno to ERANGE for all
> 'underflow' conditions, which is to say all values which are not
> exactly zero and which cannot be represented in normalized form.
>
> This matches glibc behavior, as well as the Linux, Mac OS X, OpenBSD,
> FreeBSD and SunOS strtod man pages.
>
>
> --
> -keith
>