classification
Title: unable to serialize Infinity or NaN on ARM using marshal
Type: compile error Stage: patch review
Components: Interpreter Core Versions: Python 2.7
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: mark.dickinson Nosy List: Arfrever, aurel32, djc, doko, loewis, mark.dickinson, mirell, nirs, tim.peters
Priority: normal Keywords: patch

Created on 2007-07-28 08:41 by doko, last changed 2012-04-22 17:16 by loewis. This issue is now closed.

Files
File name Uploaded Description Edit
arm-float.diff doko, 2007-07-28 08:41 patch for 2.5
arm-float2.diff mark.dickinson, 2009-02-13 21:59
arm-oabi-float-2.7.patch nirs, 2012-03-13 11:13 Same patch for 2.7
Messages (18)
msg52959 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2007-07-28 08:41
forwarded from http://bugs.debian.org/434905, patch by
Aurelien Jarno.

python2.5 is unable to serialize a floating point variable which equals Infinity or NaN on ARM using marshal.

The problem is that python2.5 wants to manage the floating points in memory by itself, and assume that they are only little and big endian IEEE formats. Otherwise it fallback to a generic code, which does not handle NaN or Infinity.

The attached patched fixed that by adding mixed-endian IEEE format, as it can be found on ARM old-ABI.
msg55628 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2007-09-04 03:01
I'm not sure like the naming of the format. "mixed-endian" could mean
anything. I doubt IEEE specifies this as a possible byte representation
(but then, I'm uncertain whether IEEE specifies big-endian and
little-endian, either).

One option would be to call it "ARM, mixed-endian", assuming this can be
only found on ARM.

Another option would be to call it "IEEE,
bytes-big-words-little-endian", describing precisely how the format
works (IIUC).

Tim, any opinion?
msg70141 - (view) Author: Aurelien Jarno (aurel32) Date: 2008-07-22 09:04
AFAIK, this "mixed-endian" format is only used on little endian ARM 
(old-ABI only). 

That is true that IEEE 754 does not specify any format. I used the big 
and little endian code as a template to add the "ARM format", hence 
IEEE in the name. "mixed-endian" is the term usually used to describe 
this format in the ARM community. I am not opposed to any other name.

OTOH, if you consider that IEEE does not specify any format, "IEEE, 
little-endian" and "IEEE, big-endian" are also not correct.
msg81893 - (view) Author: Mark Miller (mirell) Date: 2009-02-13 10:02
This still occurs in the latest SVN checkout, and is preventing Python
from building on ARMV4L and ARMV5L OABI, dying during the test_float.py
compilation. The patch appears to solve this problem, however I have not
run a full suite of tests to ensure it doesn't have any regression issues.

Since this has been broken since 2007, could this be looked at in
getting merged?
msg81914 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-02-13 12:30
I'll take a look at this, provided Tim doesn't mind me stealing his
issue.  (Please steal it back if so.)

Mark, could you please post the output from test_float?
msg81922 - (view) Author: Mark Miller (mirell) Date: 2009-02-13 13:35
The following is where it fails un-patched:

Compiling /usr/local/lib/python2.6/test/test_float.py ...
Traceback (most recent call last):
  File "/usr/local/lib/python2.6/compileall.py", line 156, in <module>
    exit_status = int(not main())
  File "/usr/local/lib/python2.6/compileall.py", line 146, in main
    force, rx, quiet):
  File "/usr/local/lib/python2.6/compileall.py", line 83, in compile_dir
    if not compile_dir(fullname, maxlevels - 1, dfile, force, rx, quiet):
  File "/usr/local/lib/python2.6/compileall.py", line 65, in compile_dir
    ok = py_compile.compile(fullname, None, dfile, True)
  File "/usr/local/lib/python2.6/py_compile.py", line 138, in compile
    marshal.dump(codeobject, fc)
ValueError: unmarshallable object
make: *** [libinstall] Error 1

This is both on armv4l and armv5l OABI.
msg81943 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-02-13 15:58
Thanks, Mark.

A few comments:

- The patch seems incomplete.  There are other places in the source tree 
that care about endianness.  I haven't done a thorough search, but the 
native endianness support in the struct module comes to mind.  There are 
also some more obscure places---for example, there's at least one piece of 
code (in Python/compile.c) that tries to distinguish between the doubles 
0.0 and -0.0 by looking only at the first and last bytes of the double, 
and that doesn't work for this particular mixed-endian format.
I guess the patch could go in as it is, but adding only partial support 
for mixed-endian doubles seems dangerous.

- I know very little about ARM, so please correct me if I'm talking 
rubbish here.  But if I understand correctly, this mixed-endian format is 
only an issue for the old ABI, and isn't relevant for the newer "Embedded" 
ABIs.  So the need for this is likely to diminish over the next few years.  
Is that correct?

- Adding support for mixed-endian doubles probably has to be considered a 
new feature, so could not go in until 2.7, which is probably still a good 
few months away (I'm guessing a year or more, but there's no roadmap at 
the moment).

How prevalent is the old ABI?  How widespread do you think it's likely to 
be in, say, 3 or 5 years' time?  Is there going to be a real need to be 
able to run Python 2.7 and Python 3.1 on mixed-endian platforms?

I'm -1 on adding mixed-endian support, especially if all that's gained is 
an ability to deal with infinities, nans and signed zeros.  It would mean 
adding a significant amount of code that's going to be awkward to maintain 
and test, and that seems likely to become redundant within a few years.

I'd upgrade that -1 to a -0 in the presence: of (1) an OABI ARM buildbot, 
(2) a developer who's available to do testing on ARM, and (3) a more 
complete patch.
msg81945 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-02-13 16:11
> native endianness support in the struct module comes to mind

Sorry:  ignore that.  The patch already covers this, since the struct 
module just uses _PyFloat_{Unp,P}ack8.
msg81950 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-02-13 16:32
We still need to fix the compile failure somehow, though...

Mark, is there any way you can isolate the test(s) in test_float that are 
causing compile failure?  I'm suspicious of test_inf_as_str and 
test_nan_as_str.  Does test_float compile if you remove these two tests?
msg81974 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-02-13 21:59
I think my -1 for adding the new format was premature:  I was hoping to 
find a way to fix marshal for the 'unknown' format, but the cleanest 
solution does indeed appear to be to add the mixed-endian format.  And 
apart from the Python/compile.c oddity I can't find anywhere besides 
Objects/floatobject.c that needs to be changed, so the patch only has to 
touch one file.

Here's a reworked version of Aurelien Jarno's patch, against the trunk.  
The key differences, from more significant to less, are as follows:

- I've abstracted out the 3 different byte orders, so that the same code 
does the packing and unpacking for all 3 different formats.  I think 
things should be safer and more maintainable this way (and it makes life 
easier when some other wacky byteorder comes along).

- Rename the format to "IEEE, ARM mixed-endian".  Note that these mixed-
endian doubles are perfectly valid IEEE 754 doubles, so the name should 
start with "IEEE" so that the various IEEE 754-specific tests are enabled 
on ARM.  The IEEE 754 standard has precisely nothing to say about 
endianness or serialization of floats as byte sequences:   the words 
'endian', 'byte' and 'octet' don't even appear anywhere in the standard.

- In float.__setformat__, don't allow setting of "IEEE, ARM mixed-endian" 
for the 'float' type, only for the 'double' type.  (If I understand 
correctly, when the double type is ARM mixed-endian the float type will 
always be little-endian;  there are no mixed-endian single-precision 
floats to worry about.)

- whitespace cleanups, expanded comments and docstrings, and line length 
fixes.

I've tested this on two versions of OS X: 10.5.6/Intel (little-endian), 
and 10.4.10/PPC (big-endian), with no (new) test failures.

I fully expect that this patch will cause test failures on ARM as a result 
of the IEEE tests being enabled, but that's better than a build failure.  
If there's someone who's willing to provide feedback I'll work to fix 
those test failures.

I'm still not sure whether this can be a candidate 2.6 and 3.0.  Martin, 
do you have any thoughts on this?

To Mark Miller and Aurelien Jarno:  is either of you in a position to test 
this patch on the relevant mixed-endian platforms?
msg81983 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2009-02-14 00:23
> I'm still not sure whether this can be a candidate 2.6 and 3.0.  Martin, 
> do you have any thoughts on this?

I think this qualifies as a new port. In the past, we have avoided
adding new ports in bugfix releases.

As for adding it to 2.7/3.1: I'm also skeptical. We have a (fairly) new
policy of not adding support for minority platforms, based on the belief
that it is more effort than it is worth. Now, ARM in itself might be
considered a minority platform (for Python); however, I can accept
continued support for ARM based on the number of units available in the
real world, and based on the fact that the chip is still being
manufactured. Whether we need to support some old Linux ABI, I really
don't know, and I'm -0.
msg81986 - (view) Author: Mark Miller (mirell) Date: 2009-02-14 02:55
I am in a position to test as much as needed. I am attempting to get
Gentoo's ARM/MIPS/Embedded distribution up to date on Python, and
noticed this build break. (Typically most embedded architectures are
several releases behind.)

I'll try this new patch right now.

As for it only being with OABI, that's correct, and yes, it will
decrease in numbers as time goes on, since most higher-end embedded
processors are EABI currently. 

The concerns are valid, I understand, but since it seems to be a
relatively minor change, I wouldn't see the real harm in this case,
especially since otherwise Python seems to work just fine under ARM OABI. 

But yes, if you're looking at it from a perspective as if it will still
be used prevalently in 3-5 years, it's use will be minor, with OABI
especially, but that's always the trend in embedded architectures.

Regardless, I'm very grateful for both Aurelian Jarno and Mark Dickinson
for coming up with a patch that works to get me past my build breaks.
msg81988 - (view) Author: Mark Miller (mirell) Date: 2009-02-14 03:18
The new patch works correctly, by the way, on ARMv4L and ARMv5L OABI boards.
msg82003 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2009-02-14 10:20
Thanks Martin and Mark Miller for the comments and testing.

I'm going to close this as "won't fix".  This doesn't preclude ARM OABI 
becoming a supported platform at some point in the future, just not
right now.
msg155589 - (view) Author: Nir Soffer (nirs) * Date: 2012-03-13 11:13
As someone who has to develop on ARM OABI, I find this won't fix policy rather frustrating.

If you happen to need this patch on 2.7, this is the same patch as arm-float2.diff, which can be applied cleanly to release 2.7.2.

Changes from arm-float2.diff:
- Remove whitespace only changes
- Replace tabs with spaces
- Fixed indentation in changed code

Enjoy.
msg158201 - (view) Author: Dirkjan Ochtman (djc) * (Python committer) Date: 2012-04-13 08:26
Could we reconsider ARM support at this time? Seems like ARM support has been surging over the past few years, and it's becoming more supported by Linux distributions. Seems like a pity to leave a patch like this out here.
msg158962 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2012-04-22 13:11
[Dirkjan]
> Could we reconsider ARM support at this time?

Note that it's just the Linux old ABI (OABI) that needs this patch;  ARM / Linux using the new family of ABIs (EABI) should be fine.  IIUC, this old ABI is being phased out, but I have no idea what the current real-world situation is here.

Maybe it's worth reopening a discussion on python-dev?  I'm not sure what the current policies are.
msg158974 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2012-04-22 17:16
This issue remains as "won't fix". ARM is supported; just OABI is not, and never will be. If anybody needs that, they will have to maintain their own fork of Python.
History
Date User Action Args
2012-04-22 17:16:30loewissetmessages: + msg158974
2012-04-22 13:11:43mark.dickinsonsetmessages: + msg158962
2012-04-13 21:41:11Arfreversetnosy: + Arfrever
2012-04-13 08:26:24djcsetnosy: + djc
messages: + msg158201
2012-03-13 11:13:08nirssetfiles: + arm-oabi-float-2.7.patch
nosy: + nirs
messages: + msg155589

2009-02-14 10:20:41mark.dickinsonsetstatus: open -> closed
resolution: wont fix
messages: + msg82003
2009-02-14 07:31:45loewissetversions: - Python 2.6, Python 2.5
2009-02-14 03:18:17mirellsetmessages: + msg81988
2009-02-14 02:55:08mirellsetmessages: + msg81986
versions: + Python 2.5, - Python 3.0, Python 3.1
2009-02-14 00:23:22loewissetmessages: + msg81983
2009-02-13 21:59:45mark.dickinsonsetfiles: + arm-float2.diff
stage: patch review
messages: + msg81974
versions: + Python 3.0, Python 3.1, - Python 2.5
2009-02-13 16:32:33mark.dickinsonsetmessages: + msg81950
2009-02-13 16:11:01mark.dickinsonsetmessages: + msg81945
2009-02-13 15:58:36mark.dickinsonsetmessages: + msg81943
2009-02-13 13:35:50mirellsetmessages: + msg81922
2009-02-13 12:30:24mark.dickinsonsetassignee: tim.peters -> mark.dickinson
messages: + msg81914
nosy: + mark.dickinson
2009-02-13 10:02:09mirellsetnosy: + mirell
type: compile error
messages: + msg81893
versions: + Python 2.6, Python 2.7
2008-07-22 09:04:08aurel32setnosy: + aurel32
messages: + msg70141
2007-09-04 03:01:01loewissetassignee: tim.peters
messages: + msg55628
nosy: + loewis, tim.peters
2007-07-28 08:41:18dokocreate