Skip to content

Avoid overflow in integer modulus and floor division opcodes - #575

Open
raashish1601 wants to merge 1 commit into
pydata:masterfrom
raashish1601:fix/int-mod-floordiv-overflow
Open

raashish1601 wants to merge 1 commit into
pydata:masterfrom
raashish1601:fix/int-mod-floordiv-overflow

Conversation

@raashish1601

Copy link
Copy Markdown

INT_MIN % -1 and INT_MIN // -1 overflow in C. On x86 this is a hardware trap, so evaluating them kills the whole Python process:

import numpy as np, numexpr as ne
a = np.array([np.iinfo(np.int64).min]); b = np.array([-1])
ne.evaluate("a % b")   # Floating point exception (core dumped)
ne.evaluate("a // b")  # same

The floor-modulus fixup ((a % b) + b) % b also overflows when b is close to the integer limits, which gives wrong results instead of a crash. With int32, -1 % INT_MIN returns 2147483647 and -5 % INT_MIN returns 2147483643; NumPy gives -1 and -5.

This handles b == -1 explicitly (% gives 0, // gives -a computed in unsigned arithmetic, so INT_MIN // -1 wraps to INT_MIN like NumPy), and only adds b to the C remainder when the remainder and b have different signs, so the sum can't overflow. Same change for the int and long long opcodes, the same way the shift opcodes were guarded in #559.

Added test_mod_floordiv_extreme_values, which compares against NumPy for both dtypes. It crashes the interpreter on main and passes with this change. I built the extension in a python:3.12 Docker image and ran numexpr.tests.test_numexpr (6043 passed). Also added a line to the release notes.

INT_MIN % -1 and INT_MIN // -1 overflow in C and crash the process on
x86. The floor-modulus fixup ((a % b) + b) % b also overflowed for
divisors close to the integer limits, e.g. -1 % INT_MIN gave INT_MAX.
Handle b == -1 explicitly and only add b when the remainder and the
divisor have different signs, which matches NumPy.
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