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.

Author: skirpichev

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

Lib/test/test_capi/test_float.py

@@ -180,20 +180,23 @@ def test_pack_unpack_roundtrip(self):
                             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
 
         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 @@ def test_pack_unpack_roundtrip_for_nans(self):
                     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)

Modules/_testcapi/clinic/float.c.h

@@ -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},
 };

Modules/_testcapi/float.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;
             }
         }