]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
gh-153200: Fix math.isqrt() for int subclasses with overridden comparison operators...
authorSerhiy Storchaka <storchaka@gmail.com>
Mon, 6 Jul 2026 18:10:23 +0000 (21:10 +0300)
committerGitHub <noreply@github.com>
Mon, 6 Jul 2026 18:10:23 +0000 (21:10 +0300)
The final check-and-correct comparison in the arbitrary precision path
could call a comparison operator overridden in an int subclass.
Compare by value with int's tp_richcompare.

Lib/test/test_math_integer.py
Misc/NEWS.d/next/Library/2026-07-06-12-00-00.gh-issue-153200.isqrtLt.rst [new file with mode: 0644]
Modules/mathintegermodule.c

index 09a98d93bd636c983a38bf672f5bc2df0977d530..d1bc776dc42391a485f5a6ab5ce36f12887a3bfe 100644 (file)
@@ -15,6 +15,19 @@ class MyIndexable(object):
     def __index__(self):
         return self.value
 
+# int subclass with broken arithmetic operators; implementations must
+# convert their arguments to exact ints instead of using these.
+class BadIntSubclass(int):
+    def _binop(self, other='ignored', mod=None):
+        return 42
+    __add__ = __radd__ = __sub__ = __rsub__ = _binop
+    __mul__ = __rmul__ = __mod__ = __rmod__ = _binop
+    __divmod__ = __rdivmod__ = __pow__ = __rpow__ = _binop
+    __floordiv__ = __rfloordiv__ = _binop
+    __lshift__ = __rlshift__ = __rshift__ = __rrshift__ = _binop
+    __and__ = __rand__ = __or__ = __ror__ = __xor__ = __rxor__ = _binop
+    __lt__ = __le__ = __gt__ = __ge__ = _binop
+
 # Here's a pure Python version of the math.integer.factorial algorithm, for
 # documentation and comparison purposes.
 #
@@ -226,6 +239,11 @@ class IntMathTests(unittest.TestCase):
         self.assertIntEqual(isqrt(False), 0)
         self.assertIntEqual(isqrt(MyIndexable(1729)), 41)
 
+        # Overridden operators of an int subclass must not affect the
+        # result.
+        self.assertIntEqual(isqrt(BadIntSubclass(10**20)), 10**10)
+        self.assertIntEqual(isqrt(BadIntSubclass(10**20 - 1)), 10**10 - 1)
+
         with self.assertRaises(ValueError):
             isqrt(MyIndexable(-3))
 
diff --git a/Misc/NEWS.d/next/Library/2026-07-06-12-00-00.gh-issue-153200.isqrtLt.rst b/Misc/NEWS.d/next/Library/2026-07-06-12-00-00.gh-issue-153200.isqrtLt.rst
new file mode 100644 (file)
index 0000000..a27ac73
--- /dev/null
@@ -0,0 +1,3 @@
+Fix :func:`math.isqrt` returning an incorrect result for arguments not
+less than 2**64 that are instances of an :class:`int` subclass with an
+overridden comparison operator.
index cfad4154b2d361126d20bc9d11497a16c7c84f48..0f660d461e349f877e31fc1a4f6fe3b94ffaa4ca 100644 (file)
@@ -454,16 +454,20 @@ math_integer_isqrt(PyObject *module, PyObject *n)
     /* The correct result is either a or a - 1. Figure out which, and
        decrement a if necessary. */
 
-    /* a_too_large = n < a * a */
+    /* a_too_large = n < a * a.  Compare by value: n can be an instance
+       of an int subclass with an overridden __lt__ method. */
     b = PyNumber_Multiply(a, a);
     if (b == NULL) {
         goto error;
     }
-    a_too_large = PyObject_RichCompareBool(n, b, Py_LT);
+    PyObject *cmp = PyLong_Type.tp_richcompare(n, b, Py_LT);
     Py_DECREF(b);
-    if (a_too_large == -1) {
+    if (cmp == NULL) {
         goto error;
     }
+    assert(PyBool_Check(cmp));
+    a_too_large = (cmp == Py_True);
+    Py_DECREF(cmp);
 
     if (a_too_large) {
         Py_SETREF(a, PyNumber_Subtract(a, _PyLong_GetOne()));