[PATCH v3] msvcrt: Make the cosh/sinh/tanh functions NAN preserving

26 Jul 2021

When a NAN is fed as input, return it as-is without mangling
it through the calculation (which e.g. loses the sign of the
input value).

This fixes a regression in a testcase of mine, after switching
to the internal implementation of these functions.

Signed-off-by: Martin Storsjo <martin(a)martin.st>
---
v3: Added initial tests for the modified functions. (Similar tests
could probably added for lots of other functions that handle it
properly right now already.) The tests are a bit loose (they ignore
the quiet/signaling bit instead of require it to be set though) to make
the comparison function usable for other functions that haven't been
modified to explicitly set the output NAN as quiet).

Modified the tanh fix to actually preserve the sign bit too.

Didn't integrate the nan handling further into the original algorithm,
pending discussion (for how to handle the sign bit that is masked out),
but reposting with tests for further discussion.

Also, note that tanhf() returns via math_error(), which returns a double.
So for those cases, the original input float nan will be cast to a double
and back to a float.
---
 dlls/msvcrt/math.c       | 40 +++++++++++++++++++++++++++++-----------
 dlls/msvcrt/tests/misc.c | 27 +++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 11 deletions(-)

diff --git a/dlls/msvcrt/math.c b/dlls/msvcrt/math.c
index d21883757a2..2f0ee6f0918 100644
--- a/dlls/msvcrt/math.c
+++ b/dlls/msvcrt/math.c
@@ -1139,6 +1139,9 @@ float CDECL coshf( float x )
     UINT32 ui = *(UINT32*)&x;
     float t;
 
+    if (isnan(x))
+        return x;
+
     /* |x| */
     ui &= 0x7fffffff;
     x = *(float*)&ui;
@@ -1159,7 +1162,7 @@ float CDECL coshf( float x )
         return 0.5f * (t + 1 / t);
     }
 
-    /* |x| > log(FLT_MAX) or nan */
+    /* |x| > log(FLT_MAX) */
     t = __expo2f(x, 1.0f);
     return t;
 }
@@ -1676,6 +1679,9 @@ float CDECL sinhf( float x )
     UINT32 ui = *(UINT32*)&x;
     float t, h, absx;
 
+    if (isnan(x))
+        return x;
+
     h = 0.5;
     if (ui >> 31)
         h = -h;
@@ -1694,7 +1700,7 @@ float CDECL sinhf( float x )
         return h * (t + t / (t + 1));
     }
 
-    /* |x| > logf(FLT_MAX) or nan */
+    /* |x| > logf(FLT_MAX) */
     t = __expo2f(absx, 2 * h);
     return t;
 }
@@ -1875,7 +1881,7 @@ float CDECL tanhf( float x )
 {
     UINT32 ui = *(UINT32*)&x;
     int sign;
-    float t;
+    float t, orig_x = x;
 
     /* x = |x| */
     sign = ui >> 31;
@@ -1885,10 +1891,13 @@ float CDECL tanhf( float x )
     if (ui > 0x3f0c9f54) {
         /* |x| > log(3)/2 ~= 0.5493 or nan */
         if (ui > 0x41200000) {
+            if (isnan(x)) {
 #if _MSVCR_VER < 140
-            if (isnan(x))
-                return math_error(_DOMAIN, "tanhf", x, 0, x);
+                return math_error(_DOMAIN, "tanhf", orig_x, 0, orig_x);
+#else
+                return orig_x;
 #endif
+            }
             /* |x| > 10 */
             fp_barrierf(x + 0x1p120f);
             t = 1 + 0 / x;
@@ -2769,6 +2778,9 @@ double CDECL cosh( double x )
     UINT32 w;
     double t;
 
+    if (isnan(x))
+        return x;
+
     /* |x| */
     ux &= (uint64_t)-1 / 2;
     x = *(double*)&ux;
@@ -2791,7 +2803,7 @@ double CDECL cosh( double x )
         return 0.5 * (t + 1 / t);
     }
 
-    /* |x| > log(DBL_MAX) or nan */
+    /* |x| > log(DBL_MAX) */
     /* note: the result is stored to handle overflow */
     t = __expo2(x, 1.0);
     return t;
@@ -4035,6 +4047,9 @@ double CDECL sinh( double x )
     UINT32 w;
     double t, h, absx;
 
+    if (isnan(x))
+        return x;
+
     h = 0.5;
     if (ux >> 63)
         h = -h;
@@ -4054,7 +4069,7 @@ double CDECL sinh( double x )
         return h * (t + t / (t + 1));
     }
 
-    /* |x| > log(DBL_MAX) or nan */
+    /* |x| > log(DBL_MAX) */
     /* note: the result is stored to handle overflow */
     t = __expo2(absx, 2 * h);
     return t;
@@ -4320,7 +4335,7 @@ double CDECL tanh( double x )
     UINT64 ui = *(UINT64*)&x;
     UINT32 w;
     int sign;
-    double t;
+    double t, orig_x = x;
 
     /* x = |x| */
     sign = ui >> 63;
@@ -4331,11 +4346,14 @@ double CDECL tanh( double x )
     if (w > 0x3fe193ea) {
         /* |x| > log(3)/2 ~= 0.5493 or nan */
         if (w > 0x40340000) {
+            if (isnan(x)) {
 #if _MSVCR_VER < 140
-            if (isnan(x))
-                return math_error(_DOMAIN, "tanh", x, 0, x);
+                return math_error(_DOMAIN, "tanh", orig_x, 0, orig_x);
+#else
+                return orig_x;
 #endif
-            /* |x| > 20 or nan */
+            }
+            /* |x| > 20 */
             /* note: this branch avoids raising overflow */
             fp_barrier(x + 0x1p120f);
             t = 1 - 0 / x;
diff --git a/dlls/msvcrt/tests/misc.c b/dlls/msvcrt/tests/misc.c
index b38915596df..3b05c892ffe 100644
--- a/dlls/msvcrt/tests/misc.c
+++ b/dlls/msvcrt/tests/misc.c
@@ -491,9 +491,29 @@ static void test_qsort_s(void)
         ok(tab[i] == i, "data sorted incorrectly on position %d: %d\n", i, tab[i]);
 }
 
+static int eq_nan(double a, double b) {
+    /* Check if NANs are equal, ignoring the quiet/signaling bit */
+    UINT64 ai = *(UINT64*)&a;
+    UINT64 bi = *(UINT64*)&b;
+    UINT64 qbit = (1ULL << 51);
+    return (ai & ~qbit) == (bi & ~qbit);
+}
+
+static int eq_nanf(float a, float b) {
+    /* Check if NANs are equal, ignoring the quiet/signaling bit */
+    DWORD ai = *(DWORD*)&a;
+    DWORD bi = *(DWORD*)&b;
+    DWORD qbit = (1 << 22);
+    return (ai & ~qbit) == (bi & ~qbit);
+}
+
 static void test_math_functions(void)
 {
     double ret;
+    UINT64 test_nan_i = 0xFFF0000000000042ULL; // Negative snan
+    double test_nan = *(double*)&test_nan_i;
+    DWORD test_nanf_i = 0xFF800042; // Negative snan
+    float test_nanf = *(float*)&test_nanf_i;
 
     errno = 0xdeadbeef;
     p_atan(NAN);
@@ -525,6 +545,13 @@ static void test_math_functions(void)
     errno = 0xdeadbeef;
     p_exp(INFINITY);
     ok(errno == 0xdeadbeef, "errno = %d\n", errno);
+
+    ok(eq_nan(test_nan, cosh(test_nan)), "cosh not preserving nan\n");
+    ok(eq_nan(test_nan, sinh(test_nan)), "sinh not preserving nan\n");
+    ok(eq_nan(test_nan, tanh(test_nan)), "tanh not preserving nan\n");
+    ok(eq_nanf(test_nanf, coshf(test_nanf)), "coshf not preserving nan\n");
+    ok(eq_nanf(test_nanf, sinhf(test_nanf)), "sinhf not preserving nan\n");
+    ok(eq_nanf(test_nanf, tanhf(test_nanf)), "tanhf not preserving nan\n");
 }
 
 static void __cdecl test_thread_func(void *end_thread_type)
-- 
2.25.1

    

Martin Storsjo

Marvin

tags

participants (2)