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
12 changes: 12 additions & 0 deletions Lib/test/test_dict.py
Original file line number Diff line number Diff line change
Expand Up @@ -1919,6 +1919,18 @@ def test_hash(self):
with self.assertRaisesRegex(TypeError, "unhashable type: 'list'"):
hash(fd)

def test_hash_deeply_nested(self):

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.

tests with recursions must be properly guarded with skips.

# This should raise a RecursionError and not crash.
# Nesting through *values*: unlike keys, values are not hashed at
# construction time, so hashing recurses the full depth.
# See https://github.com/python/cpython/issues/154318.
fd = frozendict({0: 0})
for _ in range(500_000):

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.

it is better to use exceeds_recursion_limit(), not just a random number.

fd = frozendict({0: fd})
with support.infinite_recursion():
with self.assertRaises(RecursionError):

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.

please, assert regex while hashing a frozendict

hash(fd)

Comment on lines +1922 to +1933
@support.cpython_only
def test_hash_cpython(self):
# Check that hash(frozendict) implementation is:
Expand Down
10 changes: 10 additions & 0 deletions Lib/test/test_tuple.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,16 @@ def check_one_exact(t, e32, e64):
check_one_exact((0.5, (), (-2, 3, (4, 6))), 714642271,
-1845940830829704396)

def test_hash_deeply_nested(self):

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.

same comments apply here as well.

# This should raise a RecursionError and not crash.
# See https://github.com/python/cpython/issues/154318.
t = ()
for _ in range(500_000):
t = (t,)
with support.infinite_recursion():
with self.assertRaises(RecursionError):
hash(t)

Comment on lines +122 to +131
# Various tests for hashing of tuples to check that we get few collisions.
# Does something only if RUN_ALL_HASH_TESTS is true.
#
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix a crash when hashing a deeply nested :class:`tuple` or :class:`frozendict`.
A :exc:`RecursionError` is now raised instead of overflowing the C stack.
10 changes: 10 additions & 0 deletions Objects/dictobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -8455,16 +8455,26 @@ frozendict_hash(PyObject *op)
PyDictObject *mp = _PyAnyDict_CAST(op);
Py_uhash_t hash = 0;

/* Hashing a value recurses back into frozendict_hash() when that value
is itself a frozendict, so guard the descent to raise RecursionError
rather than overflow the C stack on a deeply nested frozendict. Keys
are safe: their hashes are already computed at construction time. */
if (_Py_EnterRecursiveCall(" while hashing a frozendict")) {
return -1;
}

PyObject *value; // borrowed ref
Py_ssize_t pos = 0;
Py_hash_t key_hash;
while (_PyDict_Next(op, &pos, NULL, &value, &key_hash)) {
Py_hash_t pair_hash = frozendict_pair_hash(key_hash, value);
if (pair_hash == -1) {
_Py_LeaveRecursiveCall();
return -1;
}
hash ^= _shuffle_bits(pair_hash);
}
_Py_LeaveRecursiveCall();

/* Factor in the number of active entries */
hash ^= ((Py_uhash_t)mp->ma_used + 1) * 1927868237UL;
Expand Down
10 changes: 10 additions & 0 deletions Objects/tupleobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -380,16 +380,26 @@ tuple_hash(PyObject *op)

Py_ssize_t len = Py_SIZE(v);
PyObject **item = v->ob_item;

/* Hashing an item recurses back into tuple_hash() when that item is
itself a tuple, so guard the descent to raise RecursionError rather
than overflow the C stack on a deeply nested tuple. */
if (_Py_EnterRecursiveCall(" while hashing a tuple")) {
return -1;
}

acc = _PyTuple_HASH_XXPRIME_5;
for (Py_ssize_t i = 0; i < len; i++) {
Py_uhash_t lane = PyObject_Hash(item[i]);
if (lane == (Py_uhash_t)-1) {
_Py_LeaveRecursiveCall();
return -1;
}
acc += lane * _PyTuple_HASH_XXPRIME_2;
acc = _PyTuple_HASH_XXROTATE(acc);
acc *= _PyTuple_HASH_XXPRIME_1;
}
_Py_LeaveRecursiveCall();

/* Add input length, mangled to keep the historical value of hash(()). */
acc += len ^ (_PyTuple_HASH_XXPRIME_5 ^ 3527539UL);
Expand Down
Loading