From 58d23ae9a919520ee5b849a4c6c48fa833c9958d Mon Sep 17 00:00:00 2001 From: Tim Peters Date: Thu, 20 Mar 2003 18:31:20 +0000 Subject: SF bug 705836: struct.pack of floats in non-native endian order pack_float, pack_double, save_float: All the routines for creating IEEE-format packed representations of floats and doubles simply ignored that rounding can (in rare cases) propagate out of a long string of 1 bits. At worst, the end-off carry can (by mistake) interfere with the exponent value, and then unpacking yields a result wrong by a factor of 2. In less severe cases, it can end up losing more low-order bits than intended, or fail to catch overflow *caused* by rounding. --- Lib/test/test_struct.py | 46 ++++++++++++++++++++++++++++++++++++++++++++++ Misc/NEWS | 14 +++++++++++--- Modules/cPickle.c | 42 ++++++++++++++++++++++++++++++------------ Modules/structmodule.c | 49 +++++++++++++++++++++++++++++++++++++------------ 4 files changed, 124 insertions(+), 27 deletions(-) diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py index 34abad5..a5d91a2 100644 --- a/Lib/test/test_struct.py +++ b/Lib/test/test_struct.py @@ -393,3 +393,49 @@ def test_p_code(): (code, input, got, expectedback)) test_p_code() + + +########################################################################### +# SF bug 705836. "f" had a severe rounding bug, where a carry +# from the low-order discarded bits could propagate into the exponent +# field, causing the result to be wrong by a factor of 2. + +def test_705836(): + import math + + for base in range(1, 33): + # smaller <- largest representable float less than base. + delta = 0.5 + while base - delta / 2.0 != base: + delta /= 2.0 + smaller = base - delta + # Packing this rounds away a solid string of trailing 1 bits. + packed = struct.pack("f", smaller) + verify(bigpacked == string_reverse(packed), + ">f pack should be byte-reversal of f", bigpacked)[0] + verify(base == unpacked) + + # Largest finite IEEE single. + big = (1 << 24) - 1 + big = math.ldexp(big, 127 - 23) + packed = struct.pack(">f", big) + unpacked = struct.unpack(">f", packed)[0] + verify(big == unpacked) + + # The same, but tack on a 1 bit so it rounds up to infinity. + big = (1 << 25) - 1 + big = math.ldexp(big, 127 - 24) + try: + packed = struct.pack(">f", big) + except OverflowError: + pass + else: + TestFailed("expected OverflowError") + +test_705836() diff --git a/Misc/NEWS b/Misc/NEWS index 9335010..56f0a80 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -2,6 +2,14 @@ What's New in Python 2.2.3 ? Release date: XX-XXX-2003 ============================ +- SF #705836: The platform-independent routines for packing floats in + IEEE formats (struct.pack's f, d codes; pickle and + cPickle's protocol 1 pickling of floats) ignored that rounding can + cause a carry to propagate. The worst consequence was that, in rare + cases, f could produce strings that, when unpacked again, + were a factor of 2 away from the original float. This has been + fixed. + - Backported SF patch #676342: after using pdb, the readline command completion was botched. @@ -31,7 +39,7 @@ Release date: XX-XXX-2003 value, but according to PEP 237 it really needs to be 1 in Python 2.2.3 and 2.3. (SF #660455) -- SF bug #678518: fix some bugs in the parser module. +- SF bug #678518: fix some bugs in the parser module. - Bastion.py and rexec.py are disabled. These modules are not safe in Python 2.2. or 2.3. @@ -103,7 +111,7 @@ Release date: XX-XXX-2003 - SF #570655, fix misleading option text for bdist_rpm -- Distutils: Allow unknown keyword arguments to the setup() function +- Distutils: Allow unknown keyword arguments to the setup() function and the Extension constructor, printing a warning about them instead of reporting an error and stopping. @@ -419,7 +427,7 @@ Library gauss() may see different results now. - The randint() method is rehabilitated (i.e. no longer deprecated). - + - In copy.py: when an object is copied through its __reduce__ method, there was no check for a __setstate__ method on the result [SF patch 565085]; deepcopy should treat instances of custom diff --git a/Modules/cPickle.c b/Modules/cPickle.c index ae2df8c..214dc62 100644 --- a/Modules/cPickle.c +++ b/Modules/cPickle.c @@ -705,7 +705,7 @@ get(Picklerobject *self, PyObject *id) { static int put(Picklerobject *self, PyObject *ob) { - if (ob->ob_refcnt < 2 || self->fast) + if (ob->ob_refcnt < 2 || self->fast) return 0; return put2(self, ob); @@ -921,7 +921,7 @@ fast_save_enter(Picklerobject *self, PyObject *obj) return 1; } -int +int fast_save_leave(Picklerobject *self, PyObject *obj) { if (self->fast_container-- >= PY_CPICKLE_FAST_LIMIT) { @@ -1064,12 +1064,8 @@ save_float(Picklerobject *self, PyObject *args) { return -1; } - if (e >= 1024) { - /* XXX 1024 itself is reserved for Inf/NaN */ - PyErr_SetString(PyExc_OverflowError, - "float too large to pack with d format"); - return -1; - } + if (e >= 1024) + goto Overflow; else if (e < -1022) { /* Gradual underflow */ f = ldexp(f, 1022 + e); @@ -1083,9 +1079,26 @@ save_float(Picklerobject *self, PyObject *args) { /* fhi receives the high 28 bits; flo the low 24 bits (== 52 bits) */ f *= 268435456.0; /* 2**28 */ fhi = (long) floor(f); /* Truncate */ + assert(fhi < 268435456); + f -= (double)fhi; f *= 16777216.0; /* 2**24 */ flo = (long) floor(f + 0.5); /* Round */ + assert(flo <= 16777216); + if (flo >> 24) { + /* The carry propagated out of a string of 24 1 bits. */ + flo = 0; + ++fhi; + if (fhi >> 28) { + /* And it also progagated out of the next + * 28 bits. + */ + fhi = 0; + ++e; + if (e >= 2047) + goto Overflow; + } + } /* First byte */ *p = (s<<7) | (e>>4); @@ -1131,6 +1144,11 @@ save_float(Picklerobject *self, PyObject *args) { } return 0; + + Overflow: + PyErr_SetString(PyExc_OverflowError, + "float too large to pack with d format"); + return -1; } @@ -2103,9 +2121,9 @@ dump(Picklerobject *self, PyObject *args) { static PyObject * Pickle_clear_memo(Picklerobject *self, PyObject *args) { - if (!PyArg_ParseTuple(args,":clear_memo")) + if (!PyArg_ParseTuple(args,":clear_memo")) return NULL; - if (self->memo) + if (self->memo) PyDict_Clear(self->memo); Py_INCREF(Py_None); return Py_None; @@ -2120,7 +2138,7 @@ Pickle_getvalue(Picklerobject *self, PyObject *args) { Pdata *data; /* Can be called by Python code or C code */ - if (args && !PyArg_ParseTuple(args, "|i:getvalue", &clear)) + if (args && !PyArg_ParseTuple(args, "|i:getvalue", &clear)) return NULL; /* Check to make sure we are based on a list */ @@ -2482,7 +2500,7 @@ static PyMemberDef Pickler_members[] = { }; static PyGetSetDef Pickler_getsets[] = { - {"persistent_id", (getter)Pickler_get_pers_func, + {"persistent_id", (getter)Pickler_get_pers_func, (setter)Pickler_set_pers_func}, {"inst_persistent_id", NULL, (setter)Pickler_set_inst_pers_func}, {"memo", (getter)Pickler_get_memo, (setter)Pickler_set_memo}, diff --git a/Modules/structmodule.c b/Modules/structmodule.c index b76e311..c2b0f21 100644 --- a/Modules/structmodule.c +++ b/Modules/structmodule.c @@ -225,12 +225,8 @@ pack_float(double x, /* The number to pack */ return -1; } - if (e >= 128) { - /* XXX 128 itself is reserved for Inf/NaN */ - PyErr_SetString(PyExc_OverflowError, - "float too large to pack with f format"); - return -1; - } + if (e >= 128) + goto Overflow; else if (e < -126) { /* Gradual underflow */ f = ldexp(f, 126 + e); @@ -243,6 +239,14 @@ pack_float(double x, /* The number to pack */ f *= 8388608.0; /* 2**23 */ fbits = (long) floor(f + 0.5); /* Round */ + assert(fbits <= 8388608); + if (fbits >> 23) { + /* The carry propagated out of a string of 23 1 bits. */ + fbits = 0; + ++e; + if (e >= 255) + goto Overflow; + } /* First byte */ *p = (s<<7) | (e>>1); @@ -261,6 +265,11 @@ pack_float(double x, /* The number to pack */ /* Done */ return 0; + + Overflow: + PyErr_SetString(PyExc_OverflowError, + "float too large to pack with f format"); + return -1; } static int @@ -296,12 +305,8 @@ pack_double(double x, /* The number to pack */ return -1; } - if (e >= 1024) { - /* XXX 1024 itself is reserved for Inf/NaN */ - PyErr_SetString(PyExc_OverflowError, - "float too large to pack with d format"); - return -1; - } + if (e >= 1024) + goto Overflow; else if (e < -1022) { /* Gradual underflow */ f = ldexp(f, 1022 + e); @@ -315,9 +320,24 @@ pack_double(double x, /* The number to pack */ /* fhi receives the high 28 bits; flo the low 24 bits (== 52 bits) */ f *= 268435456.0; /* 2**28 */ fhi = (long) floor(f); /* Truncate */ + assert(fhi < 268435456); + f -= (double)fhi; f *= 16777216.0; /* 2**24 */ flo = (long) floor(f + 0.5); /* Round */ + assert(flo <= 16777216); + if (flo >> 24) { + /* The carry propagated out of a string of 24 1 bits. */ + flo = 0; + ++fhi; + if (fhi >> 28) { + /* And it also progagated out of the next 28 bits. */ + fhi = 0; + ++e; + if (e >= 2047) + goto Overflow; + } + } /* First byte */ *p = (s<<7) | (e>>4); @@ -353,6 +373,11 @@ pack_double(double x, /* The number to pack */ /* Done */ return 0; + + Overflow: + PyErr_SetString(PyExc_OverflowError, + "float too large to pack with d format"); + return -1; } static PyObject * -- cgit v0.12