Re: strtod ("nan") returns negative NaN
Masamichi Hosoda <[email protected]>
| Newsgroups | gmane.comp.lib.newlib |
|---|---|
| Message-ID | <[email protected]> |
> On Aug 15 11:51, Craig Howland wrote:
>> On 08/15/2018 11:40 AM, Joseph Myers wrote:
>> > On Wed, 15 Aug 2018, Joseph Myers wrote:
>> >
>> > > On Tue, 14 Aug 2018, Craig Howland wrote:
>> > >
>> > > > The f_QNAN value should be 0x7fc00000 regardless of byte ordering. In
>> > > It would be better to use __builtin_nan ("") (and __builtin_nanf,
>> > > __builtin_nanl for other types) rather than using an integer
>> > > representation at all (of course that requires changes to other code to
>> > > avoid requiring an integer representation there).
>> I totally agree. To add it to the record, in conjunction with this, the
>> strtod implementation really should be upgraded to David Gay's more recent
>> version, which is 128-bit friendly. (I almost had this done some time ago,
>> but didn't quite finish.)
>> > (This is not an objection to any of the present patch proposals, just an
>> > observation that a different approach would avoid a series of problems
>> > that result from trying to hardcode information about such choices of
>> > bit-patterns for NaNs.)
>> >
>> Also agreed. If I had had time yesterday I might have tried it as I had
>> briefly thought of it, but didn't think to get the idea out to the list, so
>> I'm glad you did.
>
> Sounds like a nice followup patch...?
Here's patch v4.
It uses {nan|nanl} ("") instead of the integer representations of NaN.
It also removes the unused definitions of them.
v4-0001-Fix-strtod-nan-and-strtold-nan-returns-wrong-nega.patch
(text/x-patch, 1.7 KB)
From 7ed3011f720975c6e00968361e737a71fea3640c Mon Sep 17 00:00:00 2001 From: Masamichi Hosoda <[email protected]> Date: Thu, 16 Aug 2018 09:18:50 +0900 Subject: [PATCH v4 1/3] Fix strtod ("nan") and strtold ("nan") returns wrong negative NaN The definition of qNaN for x86_64 and i386 was wrong. strto{d|ld} ("nan") returned wrong negative NaN instead of correct positive NaN since it used the wrong definition. On the other hand, strtof ("nan") returns correct positive NaN since it uses nanf ("") instead of the wrong definition. This commit makes strto{d|ld} ("nan") uses {nan|nanl} ("") like strtof ("nan") using. So strto{d|ld} ("nan") returns positive NaN. --- newlib/libc/stdlib/strtod.c | 5 +---- newlib/libc/stdlib/strtorx.c | 6 +----- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/newlib/libc/stdlib/strtod.c b/newlib/libc/stdlib/strtod.c index 0cfa9e6..d70d2c2 100644 --- a/newlib/libc/stdlib/strtod.c +++ b/newlib/libc/stdlib/strtod.c @@ -444,10 +444,7 @@ _strtod_l (struct _reent *ptr, const char *__restrict s00, char **__restrict se, } else { #endif - dword0(rv) = NAN_WORD0; -#ifndef _DOUBLE_IS_32BITS - dword1(rv) = NAN_WORD1; -#endif /*!_DOUBLE_IS_32BITS*/ + dval(rv) = nan (""); #ifndef No_Hex_NaN } #endif diff --git a/newlib/libc/stdlib/strtorx.c b/newlib/libc/stdlib/strtorx.c index aeeb250..f923fdf 100644 --- a/newlib/libc/stdlib/strtorx.c +++ b/newlib/libc/stdlib/strtorx.c @@ -93,11 +93,7 @@ ULtox(__UShort *L, __ULong *bits, Long exp, int k) break; case STRTOG_NaN: - L[0] = ldus_QNAN0; - L[1] = ldus_QNAN1; - L[2] = ldus_QNAN2; - L[3] = ldus_QNAN3; - L[4] = ldus_QNAN4; + *((long double*)L) = nanl (""); } if (k & STRTOG_Neg) L[_0] |= 0x8000; -- 2.17.0
v4-0002-Remove-unused-NaN-s-integer-representation-defini.patch
(text/x-patch, 5.1 KB)
From b94a348991122290aabb647a500ada21494b9d36 Mon Sep 17 00:00:00 2001 From: Masamichi Hosoda <[email protected]> Date: Thu, 16 Aug 2018 09:46:43 +0900 Subject: [PATCH v4 2/3] Remove unused NaN's integer representation definitions By previous commit, strto{d|ld} ("nan") does not use the definition of NaN. There is no other function that uses the definitions. This commit remove the definitions. --- newlib/libc/stdlib/gd_qnan.h | 53 ------------------------------- newlib/libc/stdlib/gdtoa-gethex.c | 1 - newlib/libc/stdlib/mprec.h | 33 ------------------- newlib/libc/stdlib/strtod.c | 9 +++++- newlib/libc/stdlib/strtodg.c | 1 - newlib/libc/stdlib/strtorx.c | 4 --- 6 files changed, 8 insertions(+), 93 deletions(-) delete mode 100644 newlib/libc/stdlib/gd_qnan.h diff --git a/newlib/libc/stdlib/gd_qnan.h b/newlib/libc/stdlib/gd_qnan.h deleted file mode 100644 index b775f82..0000000 --- a/newlib/libc/stdlib/gd_qnan.h +++ /dev/null @@ -1,53 +0,0 @@ -#ifdef __IEEE_BIG_ENDIAN - -#if !defined(__mips) -#define f_QNAN 0x7fc00000 -#define d_QNAN0 0x7ff80000 -#define d_QNAN1 0x0 -#define ld_QNAN0 0x7ff80000 -#define ld_QNAN1 0x0 -#define ld_QNAN2 0x0 -#define ld_QNAN3 0x0 -#define ldus_QNAN0 0x7ff8 -#define ldus_QNAN1 0x0 -#define ldus_QNAN2 0x0 -#define ldus_QNAN3 0x0 -#define ldus_QNAN4 0x0 -#elif defined(__mips_nan2008) -#define f_QNAN 0x7fc00000 -#define d_QNAN0 0x7ff80000 -#define d_QNAN1 0x0 -#else -#define f_QNAN 0x7fbfffff -#define d_QNAN0 0x7ff7ffff -#define d_QNAN1 0xffffffff -#endif - -#elif defined(__IEEE_LITTLE_ENDIAN) - -#if !defined(__mips) -#define f_QNAN 0xffc00000 -#define d_QNAN0 0x0 -#define d_QNAN1 0xfff80000 -#define ld_QNAN0 0x0 -#define ld_QNAN1 0xc0000000 -#define ld_QNAN2 0xffff -#define ld_QNAN3 0x0 -#define ldus_QNAN0 0x0 -#define ldus_QNAN1 0x0 -#define ldus_QNAN2 0x0 -#define ldus_QNAN3 0xc000 -#define ldus_QNAN4 0xffff -#elif defined(__mips_nan2008) -#define f_QNAN 0x7fc00000 -#define d_QNAN0 0x0 -#define d_QNAN1 0x7ff80000 -#else -#define f_QNAN 0x7fbfffff -#define d_QNAN0 0xffffffff -#define d_QNAN1 0x7ff7ffff -#endif - -#else -#error IEEE endian not defined -#endif diff --git a/newlib/libc/stdlib/gdtoa-gethex.c b/newlib/libc/stdlib/gdtoa-gethex.c index 939e0dd..d160015 100644 --- a/newlib/libc/stdlib/gdtoa-gethex.c +++ b/newlib/libc/stdlib/gdtoa-gethex.c @@ -35,7 +35,6 @@ THIS SOFTWARE. #include <locale.h> #include "mprec.h" #include "gdtoa.h" -#include "gd_qnan.h" #if !defined(PREFER_SIZE_OVER_SPEED) && !defined(__OPTIMIZE_SIZE__) && !defined(_SMALL_HEXDIG) const unsigned char __hexdig[256]= diff --git a/newlib/libc/stdlib/mprec.h b/newlib/libc/stdlib/mprec.h index 7baec83..7e9a88b 100644 --- a/newlib/libc/stdlib/mprec.h +++ b/newlib/libc/stdlib/mprec.h @@ -265,39 +265,6 @@ typedef union { double d; __ULong i[2]; } U; #define INFNAN_CHECK #endif -/* - * NAN_WORD0 and NAN_WORD1 are only referenced in strtod.c. Prior to - * 20050115, they used to be hard-wired here (to 0x7ff80000 and 0, - * respectively), but now are determined by compiling and running - * qnan.c to generate gd_qnan.h, which specifies d_QNAN0 and d_QNAN1. - * Formerly gdtoaimp.h recommended supplying suitable -DNAN_WORD0=... - * and -DNAN_WORD1=... values if necessary. This should still work. - * (On HP Series 700/800 machines, -DNAN_WORD0=0x7ff40000 works.) - */ -#ifdef IEEE_Arith -#ifdef IEEE_MC68k -#define _0 0 -#define _1 1 -#ifndef NAN_WORD0 -#define NAN_WORD0 d_QNAN0 -#endif -#ifndef NAN_WORD1 -#define NAN_WORD1 d_QNAN1 -#endif -#else -#define _0 1 -#define _1 0 -#ifndef NAN_WORD0 -#define NAN_WORD0 d_QNAN1 -#endif -#ifndef NAN_WORD1 -#define NAN_WORD1 d_QNAN0 -#endif -#endif -#else -#undef INFNAN_CHECK -#endif - #ifdef RND_PRODQUOT #define rounded_product(a,b) a = rnd_prod(a, b) #define rounded_quotient(a,b) a = rnd_quot(a, b) diff --git a/newlib/libc/stdlib/strtod.c b/newlib/libc/stdlib/strtod.c index d70d2c2..3164e30 100644 --- a/newlib/libc/stdlib/strtod.c +++ b/newlib/libc/stdlib/strtod.c @@ -137,7 +137,6 @@ THIS SOFTWARE. #include <string.h> #include "mprec.h" #include "gdtoa.h" -#include "gd_qnan.h" #include "../locale/setlocale.h" /* #ifndef NO_FENV_H */ @@ -172,6 +171,14 @@ static const double tinytens[] = { 1e-16, 1e-32, #define Rounding Flt_Rounds #endif +#ifdef IEEE_MC68k +#define _0 0 +#define _1 1 +#else +#define _0 1 +#define _1 0 +#endif + #ifdef Avoid_Underflow /*{*/ static double sulp (U x, diff --git a/newlib/libc/stdlib/strtodg.c b/newlib/libc/stdlib/strtodg.c index 4ac1f8e..c8e581c 100644 --- a/newlib/libc/stdlib/strtodg.c +++ b/newlib/libc/stdlib/strtodg.c @@ -35,7 +35,6 @@ THIS SOFTWARE. #include <string.h> #include "mprec.h" #include "gdtoa.h" -#include "gd_qnan.h" #include "locale.h" diff --git a/newlib/libc/stdlib/strtorx.c b/newlib/libc/stdlib/strtorx.c index f923fdf..a35dabe 100644 --- a/newlib/libc/stdlib/strtorx.c +++ b/newlib/libc/stdlib/strtorx.c @@ -35,13 +35,9 @@ THIS SOFTWARE. #include <string.h> #include "mprec.h" #include "gdtoa.h" -#include "gd_qnan.h" #if defined (_HAVE_LONG_DOUBLE) && !defined (_LDBL_EQ_DBL) -#undef _0 -#undef _1 - /* one or the other of IEEE_MC68k or IEEE_8087 should be #defined */ #ifdef IEEE_MC68k -- 2.17.0
v4-0003-Fix-strtof-nan-returns-positive-NaN.patch
(text/x-patch, 1.5 KB)
From 521bd9379f70d15c2f9848ed7ee3b7e4e3e6da72 Mon Sep 17 00:00:00 2001 From: Masamichi Hosoda <[email protected]> Date: Wed, 15 Aug 2018 08:39:22 +0900 Subject: [PATCH v4 3/3] Fix strtof ("-nan") returns positive NaN strtof ("-nan") returned positive NaN instead of negative NaN. strtod ("-nan") and strtold ("-nan") return negative NaN. Linux glibc has been fixed that strto{f|d|ld} ("-nan") returns negative NaN. https://sourceware.org/bugzilla/show_bug.cgi?id=23007 This commit makes strtof preserves the negative sign bit when parsing "-nan" like glibc. --- newlib/libc/stdlib/strtod.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/newlib/libc/stdlib/strtod.c b/newlib/libc/stdlib/strtod.c index 3164e30..431d3ab 100644 --- a/newlib/libc/stdlib/strtod.c +++ b/newlib/libc/stdlib/strtod.c @@ -1289,7 +1289,7 @@ strtof_l (const char *__restrict s00, char **__restrict se, locale_t loc) { double val = _strtod_l (_REENT, s00, se, loc); if (isnan (val)) - return nanf (NULL); + return signbit (val) ? -nanf (NULL) : nanf (NULL); float retval = (float) val; #ifndef NO_ERRNO if (isinf (retval) && !isinf (val)) @@ -1304,7 +1304,7 @@ strtof (const char *__restrict s00, { double val = _strtod_l (_REENT, s00, se, __get_current_locale ()); if (isnan (val)) - return nanf (NULL); + return signbit (val) ? -nanf (NULL) : nanf (NULL); float retval = (float) val; #ifndef NO_ERRNO if (isinf (retval) && !isinf (val)) -- 2.17.0