This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: struct.pack of floats in non-native endian order
Type: Stage:
Components: Extension Modules Versions: Python 3.0, Python 2.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: mark.dickinson Nosy List: christian.heimes, collinwinter, facundobatista, mark.dickinson, richardh2003, tim.peters
Priority: normal Keywords: patch

Created on 2003-03-18 20:17 by richardh2003, last changed 2022-04-10 16:07 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
705836.patch mark.dickinson, 2008-01-20 05:10
705836_v2.patch mark.dickinson, 2008-03-08 18:13 Updated patch
Messages (14)
msg15183 - (view) Author: Richard Hill (richardh2003) Date: 2003-03-18 20:17
Python version 2.1 (2.2.1 behaves the same).
Windows 2000 and RH Linux 8.0

This was run on an Intel platform.

>>> v = 7.0 + .1 + .1 + .1 + .1 + .1 + .1 + .1 + .1 + .1 
+ .1
>>> v
7.9999999999999964
>>> struct.pack( "f", v )
'\x00\x00\x00A'
>>> struct.pack( ">f", v )
'@\x80\x00\x00'

These two should be the same byte pattern (only 
reversed)!

>>> struct.unpack( ">f", '@\x80\x00\x00' )
(4.0,)

!!!!!

msg15184 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2003-03-19 19:01
Logged In: YES 
user_id=31435

Yuck.  It's a bug in not accounting for that rounding can 
spill over the original bit width.  structmodule's pack_float() 
and pack_double() both have this flaw, although the one in 
pack_double() is much subtler.  A similar cut-and-paste 
bug is in cPicke's save_float().  I'll fix all this.

Note:  while "<f"'s result should be the byte-reversal 
of ">f"'s, there's no defined relationship between either of 
those and plain "f".  "f" is platform-native in all respects.  "<f" 
and ">f" force an IEEE-like encoding, even on non-IEEE 
platforms.
msg15185 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2003-03-20 05:25
Logged In: YES 
user_id=31435

Boosted priority because this is an especially bad kind of 
bug:  mysterious, subtle, and very rare ("most" in-range 
floats pack under "<f" and ">f" without problems; a problem 
requires that a carry out of the 25th most-significant-bit 
propagate thru a solid string of 24 1 bits).

For 2.2 I expect to check in a quick hack.  In 2.3 this code 
needs refactoring (structmodule and cPickle shouldn't have 
duplicated this delicate code)
msg15186 - (view) Author: Richard Hill (richardh2003) Date: 2003-03-20 12:33
Logged In: YES 
user_id=737060

Thanks for getting back to me.

Your comment regarding IEEE formats is very interesting, I 
didn't know about this behaviour.
msg15187 - (view) Author: Tim Peters (tim.peters) * (Python committer) Date: 2003-03-20 18:48
Logged In: YES 
user_id=31435

Fixed.  In the 2.2 branch:

Lib/test/test_struct.py; new revision: 1.14.12.1
Misc/NEWS; new revision: 1.337.2.4.2.68
Modules/cPickle.c; new revision: 2.73.2.1.2.4
Modules/structmodule.c; new revision: 2.51.8.2

For 2.3:

Lib/test/test_struct.py; new revision: 1.17
Misc/NEWS; new revision: 1.700
Modules/cPickle.c; new revision: 2.141
Modules/structmodule.c; new revision: 2.58
msg15188 - (view) Author: Collin Winter (collinwinter) * (Python committer) Date: 2007-03-29 22:38
Reopening. The test case committed in r31892 is broken, specifically the part that checks for an OverflowError towards the end: the TestFailed exception is only instanced, never raised. Actually raising the TestFailed instance causes the test to fail.
msg60170 - (view) Author: Facundo Batista (facundobatista) * (Python committer) Date: 2008-01-19 14:28
A lot of water passed around this bridge, but I don't know if this is fixed:

In the trunk right, now:

>>> v = 7.0 + .1 + .1 + .1 + .1 + .1 + .1 + .1 + .1 + .1 + .1
>>> v
7.9999999999999964
>>> p = struct.pack( ">f", v )
>>> p
'A\x00\x00\x00'
>>> struct.unpack(">f", p)
(8.0,)
msg60255 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-01-20 01:51
It's a little odd:  the relevant code is in floatobject.c, in _PyFloat_Pack4.  The 
issue is what happens when a Python float (stored internally as a platform double) 
is packed as an IEEE-754 single-precision float. The current code doesn't behave 
consistently across platforms:

 - if the platform float format is unknown, the code turns a Python float into an 
IEEE-754 float anyway, and goes to great lengths to raise OverflowError when the 
Python float is too large to fit into an IEEE single-precision float.

 - if the platform float format is recognised as IEEE-754, then a C cast is used 
to turn the Python float (stored as a double) into a single-precision float;  for 
something that's too large for single precision, the natural result of that 
conversion is an IEEE-754 infinity.  I'm not 100% sure that the C standard (even 
C99) actually guarantees this, or whether there's a danger of floating-point 
exceptions being raised here.

I think the behaviour should be consistent:  either always raise OverflowError or 
always return the 4-byte sequence corresponding to an infinity.  I'm not sure 
which.  From the test-suite, it's clear that the original intention was that 
OverflowError should be raised.  On the other hand, 98.22% of Python users (i.e. 
those on IEEE-754 platforms) have been living happily with the 'output an 
infinity' behaviour for the last 5 years without noticing.  And we've been moving 
towards exposing infinities and NaNs more within Python.

One the OverflowError/infinity decision is made, this is a relatively easy fix, 
and I'd be happy to take care of it.

Christian, do you have any thoughts on this issue?
msg60256 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-01-20 02:07
Aha:  the C99 standard, section 6.3.1.5, says:

When a double is demoted to float, a long double is demoted to double or
float, or a value being represented in greater precision and range than required by its
semantic type (see 6.3.1.8) is explicitly converted to its semantic type [...] if the value 
being converted is outside the range of values that can be represented, the behavior is 
undefined.

So the current code likely has different behaviours even on different IEEE-754 platforms.
msg60262 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-01-20 05:10
Here's a patch that fixes the test that Collin mentioned to reflect what's actually been 
happening for the last nearly 5 years, and changes _PyFloat_Pack4 and _PyFloat_Pack8, as 
follows.

When packing a float that's too large for the destination format (e.g. pack(">f", 1e39)):

 - before the patch, _PyFloat_Pack* gives an OverflowError on non-IEEE-754 platforms and
   an IEEE infinity on IEEE-754 platforms.

 - after the patch, _PyFloat_Pack* gives an IEEE infinity on all platforms.

This patch doesn't fix the problem that the cast from double to float on IEEE machines 
involves potentially undefined behaviour;  I think that should be considered a separate 
issue.
msg60265 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2008-01-20 08:57
Sounds like a good idea to me. Although all major platforms support IEEE
Python should guarantee a stable behavior on all platforms.

The lines should be a safe replacement of the downcast:
double fromd;
float tof;
tof = abs(fromd) >= FLT_MAX ? FLT_MAX : fromd;
tof = (float)copysign((double)tof, fromd);

By the way the release notes of Python should mention the our work on
better IEEE-754 and numeric support in bold letters. "Python 2.6 - now
with even better support for professional math in the core". It's a good
advertisement.  :)
msg62087 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-02-06 02:38
I'm stealing this issue from Tim, and downgrading the priority to normal 
(it was the original bug that was high priority).
msg63405 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-03-08 18:13
Coming back to this, I think that it actually *is* clear what
struct(">f", large_float) should do:  it should raise OverflowError.
This fits in well with the general philosophy that Python (at least
tries to) follow for floating-point exceptions.

The attached patch (705836_v2.patch) fixes this.  All tests pass with 
this patch applied.

I'll leave this around for a few days in case anyone wants to comment on 
it;  then I plan to apply it to the trunk.
msg63528 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2008-03-14 14:24
Fixed in r61383.
History
Date User Action Args
2022-04-10 16:07:43adminsetgithub: 38176
2008-03-14 14:24:51mark.dickinsonsetstatus: open -> closed
messages: + msg63528
2008-03-08 18:13:55mark.dickinsonsetfiles: + 705836_v2.patch
keywords: + patch
messages: + msg63405
2008-02-06 02:38:46mark.dickinsonsetpriority: high -> normal
assignee: tim.peters -> mark.dickinson
messages: + msg62087
versions: + Python 2.6, Python 3.0
2008-01-20 08:57:22christian.heimessetmessages: + msg60265
2008-01-20 05:10:50mark.dickinsonsetfiles: + 705836.patch
messages: + msg60262
2008-01-20 02:07:48mark.dickinsonsetmessages: + msg60256
2008-01-20 01:51:25mark.dickinsonsetnosy: + christian.heimes, mark.dickinson
messages: + msg60255
2008-01-19 14:28:28facundobatistasetnosy: + facundobatista
messages: + msg60170
2003-03-18 20:17:11richardh2003create