This is the mail archive of the newlib@sourceware.org mailing list for the newlib project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: strtod ("nan") returns negative NaN


> On 08/14/2018 08:02 PM, Masamichi Hosoda wrote:
>>> On Wed, 15 Aug 2018, Masamichi Hosoda wrote:
>>>
>>>> On Linux with glibc, both strtod ("nan")
>>>> and strtod ("-nan") return positive NaN.
>>>>
>>>> So I've created the patch that behaves like glibc.
>>>> Both strtod ("nan") and strtod ("-nan") return positive NaN.
>>> I would suggest that you should not consider fixed bugs in glibc (bug
>>> 23007 in this case) to be appropriate to emulate in other libraries.
>> I've create the patch that behaves to preserve the sign bit.
>>
>> The result on Cygwin 64 bit (newlib, x86_64) with the patch:
>> ```
>> strtof ("nan", NULL) = nan
>> strtof ("-nan", NULL) = -nan
>> strtod ("nan", NULL) = nan
>> strtod ("-nan", NULL) = -nan
>> strtold ("nan", NULL) = nan
>> strtold ("-nan", NULL) = -nan
>> ```
>>
>> Thank you for your suggestion.
>      The f_QNAN value should be 0x7fc00000 regardless of byte
> ordering.  In addition, the d_QNAN* values should be 0x0 and
> 0x7FF80000, with only the index changing based on byte ordering.  So
> instead of them being inside of a x86/x86_64 define, they should just
> have their values corrected in the LITTLE_ENDIAN clause.
>      x86 does use a different coding for their 80-bit long double, so
> the ldus_QNAN* defines could belong within an x86 define.  On the
> other hand, the ldus_QNAN* defines only apply for Intel 80-bit, so in
> that respect don't need to be within a guard.
>      The ld_QNAN defines are not actually used anywhere.  If they
> were, however, Intel 80-bit would require different values than
> 128-bit.  However, the long double support in Newlib really only works
> when long double is the same size as double.  Some functions can work
> with Intel 80-bit, but almost none of them work with 128-bit.
>      Given the prior considerations, I suggest that only the values
> get fixed so that Cygwin works, but the #if x86 is not added.  For
> this to work on other platforms (128-bit long double), more work will
> be needed.  Again, sorry that I can't provide a git diff patch, but
> here's a suggested one with diff -pu.  It contains the value
> corrections from Masamichi, but skips the #if x86 and adds a comments
> about some of the deficiencies.
>      The changes to strtod.c look OK.

Thank you for your suggestion.
Here's patch v3.
>From 4127a5473f6aaf546f09880826529c372c25643d Mon Sep 17 00:00:00 2001
From: Masamichi Hosoda <trueroad@trueroad.jp>
Date: Tue, 14 Aug 2018 22:29:34 +0900
Subject: [PATCH v3 1/2] Fix strtod ("nan") returns negative NaN

The definition of qNaN for x86_64 and i386 was wrong.
So strtod ("nan") and strtold ("nan") returned negative NaN
instead of positive NaN.

strtof ("nan") returns positive NaN so it does not have this issue.

This commit fixes definition of qNaN for x86_64 and i386.
So strtod ("nan") and strtold ("nan") return positive NaN.

Craig Howland <howland@LGSInnovations.com> suggested to skip
`#if defined (__x86_x64__) || defined (__i386__)`
block and to add a comments
for non-Intel (or 128-bit long double) platforms.
https://sourceware.org/ml/newlib/2018/msg00753.html
---
 newlib/libc/stdlib/gd_qnan.h | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/newlib/libc/stdlib/gd_qnan.h b/newlib/libc/stdlib/gd_qnan.h
index b775f82..0bfa7f3 100644
--- a/newlib/libc/stdlib/gd_qnan.h
+++ b/newlib/libc/stdlib/gd_qnan.h
@@ -26,18 +26,22 @@
 #elif defined(__IEEE_LITTLE_ENDIAN)
 
 #if !defined(__mips)
-#define f_QNAN 0xffc00000
+#define f_QNAN 0x7fc00000
 #define d_QNAN0 0x0
-#define d_QNAN1 0xfff80000
+#define d_QNAN1 0x7ff80000
+/* NOTE (FIXME):  Newlib only can do long double in limited situations.  The
+ * ld_QNAN* and ldus_QNAN* defines are for Intel 80-bit.  Additional C function
+ * work (both here and in multiple *.c files) is needed to support standard
+ * 128-bit long double.  */
 #define ld_QNAN0 0x0
 #define ld_QNAN1 0xc0000000
-#define ld_QNAN2 0xffff
+#define ld_QNAN2 0x7fff
 #define ld_QNAN3 0x0
 #define ldus_QNAN0 0x0
 #define ldus_QNAN1 0x0
 #define ldus_QNAN2 0x0
 #define ldus_QNAN3 0xc000
-#define ldus_QNAN4 0xffff
+#define ldus_QNAN4 0x7fff
 #elif defined(__mips_nan2008)
 #define f_QNAN 0x7fc00000
 #define d_QNAN0 0x0
-- 
2.17.0

>From 6ae0ab8a7a27fc828ee66d8705f166f26c8f772c Mon Sep 17 00:00:00 2001
From: Masamichi Hosoda <trueroad@trueroad.jp>
Date: Wed, 15 Aug 2018 08:39:22 +0900
Subject: [PATCH v3 2/2] 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 0cfa9e6..9c5ed56 100644
--- a/newlib/libc/stdlib/strtod.c
+++ b/newlib/libc/stdlib/strtod.c
@@ -1285,7 +1285,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))
@@ -1300,7 +1300,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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]