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.
@deepkilo

Copy link
Copy Markdown

Downstream regression evidence for this PR and #578: we retained these repairs while upgrading a quantitative-research runtime and reran bounded integer checks on Linux x86_64, GIL CPython 3.15.0 / NumPy 2.5.4 / a locally patched NumExpr 2.14.2 build.

For both int32 and int64, the following two parameterized cases passed, including INT_MIN / -1, divisors at both integer limits, mixed signs, and min/max over the integer extremes:

import numpy as np
import numexpr as ne

for dtype in ('i4', 'i8'):
    limits = np.iinfo(dtype)
    a = np.array([limits.min, limits.min, -1, 5, -5, limits.max, 7], dtype=dtype)
    b = np.array([-1, limits.min, limits.min, limits.max,
                  limits.max, limits.min, -1], dtype=dtype)
    np.testing.assert_array_equal(ne.evaluate('a % b'), np.mod(a, b))
    with np.errstate(over='ignore'):
        expected = a // b
    np.testing.assert_array_equal(ne.evaluate('a // b'), expected)
    for operation in ('min', 'max'):
        np.testing.assert_equal(ne.evaluate(operation + '(a)'),
                                getattr(np, operation)(a))

This checks NumPy-compatible wrap behavior for INT_MIN // -1; it does not claim that the mathematical result fits a signed integer. Our build includes both #575 and #578 plus unrelated concurrency repairs, so this is combined-build validation, not isolated-PR testing. The earlier upstream reproductions already explain the defects; sharing current-environment validation without opening duplicate issues or duplicating this comment on #578.

Disclosure: OpenAI Codex (GPT-6 family; exact variant/effort unavailable) assisted with qualification and drafting. This is not a claim that the operator manually performed every step.

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.

2 participants