Skip to content

[mypyc] Fix int subclass crashes and precision of int true division and float/int comparisons - #22418

Open
Jahongir-Qurbonov wants to merge 4 commits into
python:masterfrom
Jahongir-Qurbonov:mypyc-int-subclass-and-precision
Open

Jahongir-Qurbonov wants to merge 4 commits into
python:masterfrom
Jahongir-Qurbonov:mypyc-int-subclass-and-precision

Conversation

@Jahongir-Qurbonov

Copy link
Copy Markdown
Contributor

This fixes three related bugs in int/float primitives.

1. Crashes with large int subclass values. When an int value doesn't fit in a short int, unboxing keeps the original object, so a long tagged int can be an instance of an int subclass. The slow paths of int operations used PyNumber_* / PyObject_RichCompareBool, which call dunders the subclass overrides:

# sub.py (not compiled)
class Str(int):
    def __sub__(self, o): return 1.5
    def __eq__(self, o): raise RuntimeError()

# compiled
def sub(a: int, b: int) -> int: return a - b
def eq(a: int, b: int) -> bool: return a == b

sub(Str(2**70), 2**70)  # segfault (float read as an int object)
eq(Str(2**70), 2**70)   # "fatal: out of memory", process aborts

Small values never call the overrides. The slow paths now first convert operands to exact ints using int's own nb_int slot, so no overrides are called. Normalizing at unboxing time isn't possible, because wrapper functions and attribute setters borrow the argument. CPyTagged_TrueDivide now also checks for errors from PyFloat_AsDouble.

2. int / int was not correctly rounded for operands larger than 2**53. For example, (2**53 + 1) / 3 gave 3002399751580330.5 instead of 3002399751580331.0. The double fast path is now only used if both operands fit in 53 bits, as in CPython.

3. float vs int comparisons converted the int to a float. 2.0**53 == 2**53 + 1 was True, and 1.0 < 2**1100 raised OverflowError. Comparisons now use a new CPyFloat_CompareTagged primitive. It compares as doubles only if the int fits in 53 bits; otherwise it falls back to a rich comparison. Int literals are still converted at compile time when the conversion is exact. Arithmetic is unchanged.

Added run tests for all three, and irbuild tests for the comparisons.

Unboxing an int keeps the original object when the value doesn't fit in
a short int, so a long tagged int can refer to an instance of an int
subclass. The slow paths of int operations used the generic PyNumber_*
and rich comparison APIs, which call any dunder methods the subclass
overrides. A non-int result was then read as an int object (crash or
garbage), and an exception in an operation that can't fail aborted the
process with "fatal: out of memory".

Convert operands to exact int objects in the slow paths, using int's own
nb_int slot so that no overrides are called. This matches the behavior
for small values, which never call the overrides. Also check for errors
from PyFloat_AsDouble in CPyTagged_TrueDivide.
The fast path of int true division converted short int operands to
doubles before dividing, which rounds twice if an operand can't be
represented exactly as a double. For example, (2**53 + 1) / 3 returned
3002399751580330.5 instead of 3002399751580331.0.

Only use the fast path if the absolute values of both operands are at
most 2**53 (like CPython), and otherwise fall back to the int object
implementation, which is correctly rounded.
Comparisons between float and int converted the int to a double first,
which loses precision for ints with an absolute value larger than 2**53
and raises OverflowError for very large ints. For example,
2.0**53 == 2**53 + 1 was True, and 1.0 < 2**1100 raised OverflowError.
CPython compares the values exactly.

Use a new primitive CPyFloat_CompareTagged for these comparisons. It
compares as doubles if the int is a short int that fits in 53 bits, and
otherwise falls back to a rich comparison of the boxed values. Int
literals are still converted to float at compile time if the conversion
is exact. Arithmetic operations are unchanged.
Int literals such as 2**53 don't fit in a short int on 32-bit platforms, so the generated IR is different.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant