Opened 9 years ago
Closed 9 years ago
#703 closed defect (fixed)
Python 3 sortable_serialise() broken on 32 bit
Reported by: | Barry Warsaw | Owned by: | Olly Betts |
---|---|---|---|
Priority: | normal | Milestone: | 1.3.5 |
Component: | Xapian-bindings (Python) | Version: | 1.3.3 |
Severity: | normal | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | Operating System: | All |
Description (last modified by )
sortable_serialise() on Python 3 (Xapian 1.3) has two bugs on 32 bit systems. You can see this with the following example:
$ python3 Python 3.5.1+ (default, Jan 5 2016, 11:13:30) [GCC 5.3.1 20160101] on linux Type "help", "copyright", "credits" or "license" for more information. >>> from xapian import * >>> sortable_serialise(14215209984) OverflowError: Python int too large to convert to C long During handling of the above exception, another exception occurred: Traceback (most recent call last): File "<stdin>", line 1, in <module> SystemError: <built-in function sortable_serialise> returned a result with an error set >>>
First, the conversion of the int to the C API's double can't handle the 33+-bit size of the value, thus you get the OverflowError
. However, sortable_serialise() also doesn't properly check for and handle the exception, but instead returns a non-NULL result which triggers the SystemError
once control percolates back up to the eval loop.
Change History (7)
comment:1 by , 9 years ago
Description: | modified (diff) |
---|
comment:2 by , 9 years ago
Component: | Other → Xapian-bindings (Python) |
---|---|
Milestone: | → 1.3.x |
Owner: | changed from | to
comment:3 by , 9 years ago
We see this currently on Ubuntu Xenial 16.04 since Debian's apt-xapian-index isn't ported to Python 3 yet. In Ubuntu we have Xapian 1.3.3 as the binary package python3-xapian1.3. We haven't upgraded to 1.3.4 yet.
comment:4 by , 9 years ago
Version: | → 1.3.3 |
---|
You can probably workaround by converting to float
on the Python side, i.e.:
sortable_serialise(float(14215209984))
While that can lose precision for a suitably large number, the C++ function takes a double
, so you're going to lose it anyway. Plus this looks like a time_t
which isn't going to hit that issue for quite some time.
comment:5 by , 9 years ago
The SWIG fix I had in mind is in the recently released SWIG 3.0.8:
2015-12-23: ahnolds [Python] Fixes for conversion of signed and unsigned integer types: No longer check for PyInt objects in Python3. Because PyInt_Check and friends are #defined to the corresponding PyLong methods, this had caused errors in Python3 where values greater than what could be stored in a long were incorrectly interpreted as the value -1 with the Python error indicator set to OverflowError. This applies to both the conversions PyLong->long and PyLong->double. Conversion from PyLong to long, unsigned long, long long, and unsigned long long now raise OverflowError instead of TypeError in both Python2 and Python3 for PyLong values outside the range expressible by the corresponding C type. This matches the existing behavior for other integral types (signed and unsigned ints, shorts, and chars), as well as the conversion for PyInt to all numeric types. This also indirectly applies to the size_t and ptrdiff_t types, which depend on the conversions for unsigned long and long.
comment:6 by , 9 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
With 3.4.4, I don't get the SystemError
, but I do get b'^'
which isn't right - that's the serialisation of -1
. With SWIG git master (which is just a few commits after 3.0.8), I get a more plausible answer (I didn't think to check unserialising it again while I still had it built).
We need to check that upgrading SWIG doesn't regress any other languages, but at least we know what the fix for this is now.
comment:7 by , 9 years ago
Milestone: | 1.3.x → 1.3.5 |
---|---|
Resolution: | → fixed |
Status: | assigned → closed |
SWIG version used to bootstrap snapshots and releases updated in [b643e151dc3b906b8ffc6f215e393f1fe5017156].
Needs fixing before 1.4.0, so setting milestone.
I think I saw some SWIG fixes in this area go in recently, so it might just be a matter of upgrading to a newer SWIG. We probably want to upgrade to a recent SWIG before 1.4.0 anyway.
Which Xapian version are you using, in case that matters? I don't have a 32-bit box to hand to check with.