Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions Lib/test/test_builtin.py
Original file line number Diff line number Diff line change
Expand Up @@ -2229,6 +2229,31 @@ def __getitem__(self, index):
self.assertComplexesAreIdentical(sum([1.0, complex(1, -0.0)]),
complex(2, -0.0))

def test_sum_float_subclass(self):
# gh-151060: the C fast-sum loop must not free a float subclass with
# the exact-float deallocator. A generator keeps each item's only
# reference in sum(), so the specialized DECREF runs the deallocator.
class F(float):
count = 0
def __del__(self):
F.count += 1

self.assertEqual(sum((F(i) for i in range(5)), 1j), complex(10, 1))
self.assertEqual(F.count, 5)

def test_sum_int_subclass(self):
# gh-151060: likewise, the float and complex fast-sum loops must not
# free an int subclass with the exact-int deallocator.
class I(int):
count = 0
def __del__(self):
I.count += 1

self.assertEqual(sum((I(i) for i in range(5)), 1.0), 11.0)
self.assertEqual(I.count, 5)
self.assertEqual(sum((I(i) for i in range(5)), 1j), complex(10, 1))
self.assertEqual(I.count, 10)

@requires_IEEE_754
@skip_if_double_rounding
@support.cpython_only # Other implementations may choose a different algorithm
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix a crash or memory corruption in :func:`sum` when summing a
:class:`float` subclass instance into a complex running total.
14 changes: 7 additions & 7 deletions Python/bltinmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -2981,15 +2981,15 @@ builtin_sum_impl(PyObject *module, PyObject *iterable, PyObject *start)
_Py_DECREF_SPECIALIZED(item, _PyFloat_ExactDealloc);
continue;
}
if (PyLong_Check(item)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change really necessary? we lose a fast past for int subclasses here or am I missing something?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is needed in combination with the _Py_DECREF_SPECIALIZED that was added. I think we can keep the PyLong_Check and use the normal decref. Not sure either approach is much better for performance.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but what I meant is that we do not need to have a _Py_DECREF_SPECIALIZED for the int case right? I'm not even sure why we need the float case. Was there a real need for exact floats or was it an historical behavior? I don't remember anymore :(

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The specialized DECREF was added by commit da6c785, when sum() already checked for PyFloat_CheckExact():

-                Py_DECREF(item);
+                _Py_DECREF_SPECIALIZED(item, _PyFloat_ExactDealloc);

Supporting int and float subclasses require to decide how to deal with overriden __index__() and __float__() methods.

class StrangeInt(int):
    def __index__(self):
        return 42.0

a = 1
b = StrangeInt(2)
print(sum([a, b]))

class StrangeFloat(float):
    def __float__(self):
        return 42.0

a = 1.0
b = StrangeFloat(2.0)
print(sum([a, b]))

On Python 3.16, this code displays 3 and 3.0. So it seems like __index__() and __float__() methods are not called in these cases.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change really necessary? we lose a fast past for int subclasses here or am I missing something?

I think that specialization for subclasses was added not intentionally. In other cases we use strict checks.

WRT performance, I doubt we will have too much from specialized decrefs. But lets keep this to emphasize that we deal with base classes only.

Supporting int and float subclasses require to decide how to deal with overriden __index__() and __float__() methods.

All examples, where it matters, looks broken for me (like above).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok we can keep the strict checks only and wonder later about what to do with the checks. AFAIR the PyLong_AsDouble doesn'z care about conversion functions but it also doesn't check for exact type.

if (PyLong_CheckExact(item)) {
double value = PyLong_AsDouble(item);
if (value != -1.0 || !PyErr_Occurred()) {
re_sum = cs_add(re_sum, value);
Py_DECREF(item);
_Py_DECREF_SPECIALIZED(item, _PyLong_ExactDealloc);
continue;
}
else {
Py_DECREF(item);
_Py_DECREF_SPECIALIZED(item, _PyLong_ExactDealloc);
Py_DECREF(iter);
return NULL;
}
Expand Down Expand Up @@ -3033,20 +3033,20 @@ builtin_sum_impl(PyObject *module, PyObject *iterable, PyObject *start)
Py_DECREF(item);
continue;
}
if (PyLong_Check(item)) {
if (PyLong_CheckExact(item)) {
double value = PyLong_AsDouble(item);
if (value != -1.0 || !PyErr_Occurred()) {
re_sum = cs_add(re_sum, value);
Py_DECREF(item);
_Py_DECREF_SPECIALIZED(item, _PyLong_ExactDealloc);
continue;
}
else {
Py_DECREF(item);
_Py_DECREF_SPECIALIZED(item, _PyLong_ExactDealloc);
Py_DECREF(iter);
return NULL;
}
}
if (PyFloat_Check(item)) {
if (PyFloat_CheckExact(item)) {
double value = PyFloat_AS_DOUBLE(item);
re_sum = cs_add(re_sum, value);
_Py_DECREF_SPECIALIZED(item, _PyFloat_ExactDealloc);
Expand Down
Loading