gh-130317: Fix test_pack_unpack_roundtrip() and add docs (#133204)

* Skip sNaN's testing in 32-bit mode.
* Drop float_set_snan() helper.
* Use memcpy() workaround for sNaN's in PyFloat_Unpack4().
* Document, that sNaN's may not be preserved by PyFloat_Pack/Unpack API.

diff --git a/Doc/c-api/float.rst b/Doc/c-api/float.rst
index 1da37a5bcaeef902d3f4b4b2783d51cd55ba5299..c5a7653efca26bc13ee8b7e16c0bfd22aec6031b 100644 (file)
--- a/Doc/c-api/float.rst
+++ b/Doc/c-api/float.rst
@@ -96,6 +96,9 @@ NaNs (if such things exist on the platform) isn't handled correctly, and
 attempting to unpack a bytes string containing an IEEE INF or NaN will raise an
 exception.
 
+Note that NaNs type may not be preserved on IEEE platforms (silent NaN become
+quiet), for example on x86 systems in 32-bit mode.
+
 On non-IEEE platforms with more precision, or larger dynamic range, than IEEE
 754 supports, not all values can be packed; on non-IEEE platforms with less
 precision, or smaller dynamic range, not all values can be unpacked. What

diff --git a/Lib/test/test_capi/test_float.py b/Lib/test/test_capi/test_float.py
index 44570bf637956530050ac32336b997c124ea91f3..c832207fef0c1aa6bfd75514a74c562c82b95e3c 100644 (file)
--- a/Lib/test/test_capi/test_float.py
+++ b/Lib/test/test_capi/test_float.py
@@ -180,12 +180,6 @@ class CAPIFloatTest(unittest.TestCase):
                             self.assertEqual(value2, value)
 
     @unittest.skipUnless(HAVE_IEEE_754, "requires IEEE 754")
-    # Skip on x86 (32-bit), since these tests fail. The problem is that sNaN
-    # doubles become qNaN doubles just by the C calling convention, there is no
-    # way to preserve sNaN doubles between C function calls. But tests pass
-    # on Windows x86.
-    @unittest.skipIf((sys.maxsize == 2147483647) and not(sys.platform == 'win32'),
-                     'test fails on x86 (32-bit)')
     def test_pack_unpack_roundtrip_for_nans(self):
         pack = _testcapi.float_pack
         unpack = _testcapi.float_unpack
@@ -193,7 +187,16 @@ class CAPIFloatTest(unittest.TestCase):
         for _ in range(10):
             for size in (2, 4, 8):
                 sign = random.randint(0, 1)
-                signaling = random.randint(0, 1)
+                if sys.maxsize != 2147483647:  # not it 32-bit mode
+                    signaling = random.randint(0, 1)
+                else:
+                    # Skip sNaN's on x86 (32-bit).  The problem is that sNaN
+                    # doubles become qNaN doubles just by the C calling
+                    # convention, there is no way to preserve sNaN doubles
+                    # between C function calls with the current
+                    # PyFloat_Pack/Unpack*() API.  See also gh-130317 and
+                    # e.g. https://developercommunity.visualstudio.com/t/155064
+                    signaling = 0
                 quiet = int(not signaling)
                 if size == 8:
                     payload = random.randint(signaling, 1 << 50)
@@ -209,12 +212,6 @@ class CAPIFloatTest(unittest.TestCase):
                     with self.subTest(data=data, size=size, endian=endian):
                         data1 = data if endian == BIG_ENDIAN else data[::-1]
                         value = unpack(data1, endian)
-                        if signaling and sys.platform == 'win32':
-                            # On Windows x86, sNaN becomes qNaN when returned
-                            # from function.  That's a known bug, e.g.
-                            # https://developercommunity.visualstudio.com/t/155064
-                            # (see also gh-130317).
-                            value = _testcapi.float_set_snan(value)
                         data2 = pack(size, value, endian)
                         self.assertTrue(math.isnan(value))
                         self.assertEqual(data1, data2)

diff --git a/Modules/_testcapi/clinic/float.c.h b/Modules/_testcapi/clinic/float.c.h
index 0710e4df1963cb1361d7c44cd1126c7233734985..d5a00c8072da1ef38da075cc54a9f7b612e1ee92 100644 (file)
--- a/Modules/_testcapi/clinic/float.c.h
+++ b/Modules/_testcapi/clinic/float.c.h
@@ -81,13 +81,4 @@ _testcapi_float_unpack(PyObject *module, PyObject *const *args, Py_ssize_t nargs
 exit:
     return return_value;
 }
-
-PyDoc_STRVAR(_testcapi_float_set_snan__doc__,
-"float_set_snan($module, obj, /)\n"
-"--\n"
-"\n"
-"Make a signaling NaN.");
-
-#define _TESTCAPI_FLOAT_SET_SNAN_METHODDEF    \
-    {"float_set_snan", (PyCFunction)_testcapi_float_set_snan, METH_O, _testcapi_float_set_snan__doc__},
-/*[clinic end generated code: output=1b0e9b05e1f50712 input=a9049054013a1b77]*/
+/*[clinic end generated code: output=b43dfd3a77fe04ba input=a9049054013a1b77]*/

diff --git a/Modules/_testcapi/float.c b/Modules/_testcapi/float.c
index 007884bc1f9da3f7d8d89abf6d29b310b0405359..e3869134c84d43c03b7951cd11867ca49ff08e94 100644 (file)
--- a/Modules/_testcapi/float.c
+++ b/Modules/_testcapi/float.c
@@ -157,42 +157,9 @@ test_string_to_double(PyObject *self, PyObject *Py_UNUSED(ignored))
 }
 
 
-/*[clinic input]
-_testcapi.float_set_snan
-
-    obj: object
-    /
-
-Make a signaling NaN.
-[clinic start generated code]*/
-
-static PyObject *
-_testcapi_float_set_snan(PyObject *module, PyObject *obj)
-/*[clinic end generated code: output=f43778a70f60aa4b input=c1269b0f88ef27ac]*/
-{
-    if (!PyFloat_Check(obj)) {
-        PyErr_SetString(PyExc_ValueError, "float-point number expected");
-        return NULL;
-    }
-    double d = ((PyFloatObject *)obj)->ob_fval;
-    if (!isnan(d)) {
-        PyErr_SetString(PyExc_ValueError, "nan expected");
-        return NULL;
-    }
-    uint64_t v;
-    memcpy(&v, &d, 8);
-    v &= ~(1ULL << 51); /* make sNaN */
-
-    // gh-130317: memcpy() is needed to preserve the sNaN flag on x86 (32-bit)
-    PyObject *res = PyFloat_FromDouble(0.0);
-    memcpy(&((PyFloatObject *)res)->ob_fval, &v, 8);
-    return res;
-}
-
 static PyMethodDef test_methods[] = {
     _TESTCAPI_FLOAT_PACK_METHODDEF
     _TESTCAPI_FLOAT_UNPACK_METHODDEF
-    _TESTCAPI_FLOAT_SET_SNAN_METHODDEF
     {"test_string_to_double", test_string_to_double, METH_NOARGS},
     {NULL},
 };

diff --git a/Objects/floatobject.c b/Objects/floatobject.c
index 76ed24d29fd25c342aa5fbecd57aba86657ea1a2..692b6aa2cabd5ee48297d692ac26c454b0592027 100644 (file)
--- a/Objects/floatobject.c
+++ b/Objects/floatobject.c
@@ -2495,12 +2495,10 @@ PyFloat_Unpack4(const char *data, int le)
 
             if ((v & (1 << 22)) == 0) {
                 double y = x; /* will make qNaN double */
-                union double_val {
-                    double d;
-                    uint64_t u64;
-                } *py = (union double_val *)&y;
-
-                py->u64 &= ~(1ULL << 51); /* make sNaN */
+                uint64_t u64;
+                memcpy(&u64, &y, 8);
+                u64 &= ~(1ULL << 51); /* make sNaN */
+                memcpy(&y, &u64, 8);
                 return y;
             }
         }