Skip to content

Commit 02202c1

Browse files
gh-140551: Fix dict crash if clear is called at lookup stage (#140558)
Co-authored-by: Inada Naoki <songofacandy@gmail.com>
1 parent 9f8d005 commit 02202c1

File tree

3 files changed

+54
-50
lines changed

3 files changed

+54
-50
lines changed

Lib/test/test_dict.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1601,6 +1601,26 @@ def __hash__(self):
16011601
with self.assertRaises(KeyError):
16021602
d.get(key2)
16031603

1604+
def test_clear_at_lookup(self):
1605+
class X:
1606+
def __hash__(self):
1607+
return 1
1608+
def __eq__(self, other):
1609+
nonlocal d
1610+
d.clear()
1611+
1612+
d = {}
1613+
for _ in range(10):
1614+
d[X()] = None
1615+
1616+
self.assertEqual(len(d), 1)
1617+
1618+
d = {}
1619+
for _ in range(10):
1620+
d.setdefault(X(), None)
1621+
1622+
self.assertEqual(len(d), 1)
1623+
16041624

16051625
class CAPITest(unittest.TestCase):
16061626

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fixed crash in :class:`dict` if :meth:`dict.clear` is called at the lookup
2+
stage. Patch by Mikhail Efimov and Inada Naoki.

Objects/dictobject.c

Lines changed: 32 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1775,6 +1775,14 @@ static inline int
17751775
insert_combined_dict(PyInterpreterState *interp, PyDictObject *mp,
17761776
Py_hash_t hash, PyObject *key, PyObject *value)
17771777
{
1778+
// gh-140551: If dict was cleared in _Py_dict_lookup,
1779+
// we have to resize one more time to force general key kind.
1780+
if (DK_IS_UNICODE(mp->ma_keys) && !PyUnicode_CheckExact(key)) {
1781+
if (insertion_resize(mp, 0) < 0)
1782+
return -1;
1783+
assert(mp->ma_keys->dk_kind == DICT_KEYS_GENERAL);
1784+
}
1785+
17781786
if (mp->ma_keys->dk_usable <= 0) {
17791787
/* Need to resize. */
17801788
if (insertion_resize(mp, 1) < 0) {
@@ -1871,38 +1879,31 @@ insertdict(PyInterpreterState *interp, PyDictObject *mp,
18711879
PyObject *key, Py_hash_t hash, PyObject *value)
18721880
{
18731881
PyObject *old_value;
1882+
Py_ssize_t ix;
18741883

18751884
ASSERT_DICT_LOCKED(mp);
18761885

1877-
if (DK_IS_UNICODE(mp->ma_keys) && !PyUnicode_CheckExact(key)) {
1878-
if (insertion_resize(mp, 0) < 0)
1879-
goto Fail;
1880-
assert(mp->ma_keys->dk_kind == DICT_KEYS_GENERAL);
1881-
}
1882-
1883-
if (_PyDict_HasSplitTable(mp)) {
1884-
Py_ssize_t ix = insert_split_key(mp->ma_keys, key, hash);
1886+
if (_PyDict_HasSplitTable(mp) && PyUnicode_CheckExact(key)) {
1887+
ix = insert_split_key(mp->ma_keys, key, hash);
18851888
if (ix != DKIX_EMPTY) {
18861889
insert_split_value(interp, mp, key, value, ix);
18871890
Py_DECREF(key);
18881891
Py_DECREF(value);
18891892
return 0;
18901893
}
1891-
1892-
/* No space in shared keys. Resize and continue below. */
1893-
if (insertion_resize(mp, 1) < 0) {
1894+
// No space in shared keys. Go to insert_combined_dict() below.
1895+
}
1896+
else {
1897+
ix = _Py_dict_lookup(mp, key, hash, &old_value);
1898+
if (ix == DKIX_ERROR)
18941899
goto Fail;
1895-
}
18961900
}
18971901

1898-
Py_ssize_t ix = _Py_dict_lookup(mp, key, hash, &old_value);
1899-
if (ix == DKIX_ERROR)
1900-
goto Fail;
1901-
19021902
if (ix == DKIX_EMPTY) {
1903-
assert(!_PyDict_HasSplitTable(mp));
1904-
/* Insert into new slot. */
1905-
assert(old_value == NULL);
1903+
// insert_combined_dict() will convert from non DICT_KEYS_GENERAL table
1904+
// into DICT_KEYS_GENERAL table if key is not Unicode.
1905+
// We don't convert it before _Py_dict_lookup because non-Unicode key
1906+
// may change generic table into Unicode table.
19061907
if (insert_combined_dict(interp, mp, hash, key, value) < 0) {
19071908
goto Fail;
19081909
}
@@ -4374,6 +4375,7 @@ dict_setdefault_ref_lock_held(PyObject *d, PyObject *key, PyObject *default_valu
43744375
PyDictObject *mp = (PyDictObject *)d;
43754376
PyObject *value;
43764377
Py_hash_t hash;
4378+
Py_ssize_t ix;
43774379
PyInterpreterState *interp = _PyInterpreterState_GET();
43784380

43794381
ASSERT_DICT_LOCKED(d);
@@ -4409,17 +4411,8 @@ dict_setdefault_ref_lock_held(PyObject *d, PyObject *key, PyObject *default_valu
44094411
return 0;
44104412
}
44114413

4412-
if (!PyUnicode_CheckExact(key) && DK_IS_UNICODE(mp->ma_keys)) {
4413-
if (insertion_resize(mp, 0) < 0) {
4414-
if (result) {
4415-
*result = NULL;
4416-
}
4417-
return -1;
4418-
}
4419-
}
4420-
4421-
if (_PyDict_HasSplitTable(mp)) {
4422-
Py_ssize_t ix = insert_split_key(mp->ma_keys, key, hash);
4414+
if (_PyDict_HasSplitTable(mp) && PyUnicode_CheckExact(key)) {
4415+
ix = insert_split_key(mp->ma_keys, key, hash);
44234416
if (ix != DKIX_EMPTY) {
44244417
PyObject *value = mp->ma_values->values[ix];
44254418
int already_present = value != NULL;
@@ -4432,27 +4425,22 @@ dict_setdefault_ref_lock_held(PyObject *d, PyObject *key, PyObject *default_valu
44324425
}
44334426
return already_present;
44344427
}
4435-
4436-
/* No space in shared keys. Resize and continue below. */
4437-
if (insertion_resize(mp, 1) < 0) {
4438-
goto error;
4439-
}
4428+
// No space in shared keys. Go to insert_combined_dict() below.
44404429
}
4441-
4442-
assert(!_PyDict_HasSplitTable(mp));
4443-
4444-
Py_ssize_t ix = _Py_dict_lookup(mp, key, hash, &value);
4445-
if (ix == DKIX_ERROR) {
4446-
if (result) {
4447-
*result = NULL;
4430+
else {
4431+
ix = _Py_dict_lookup(mp, key, hash, &value);
4432+
if (ix == DKIX_ERROR) {
4433+
if (result) {
4434+
*result = NULL;
4435+
}
4436+
return -1;
44484437
}
4449-
return -1;
44504438
}
44514439

44524440
if (ix == DKIX_EMPTY) {
4453-
assert(!_PyDict_HasSplitTable(mp));
44544441
value = default_value;
44554442

4443+
// See comment to this function in insertdict.
44564444
if (insert_combined_dict(interp, mp, hash, Py_NewRef(key), Py_NewRef(value)) < 0) {
44574445
Py_DECREF(key);
44584446
Py_DECREF(value);
@@ -4477,12 +4465,6 @@ dict_setdefault_ref_lock_held(PyObject *d, PyObject *key, PyObject *default_valu
44774465
*result = incref_result ? Py_NewRef(value) : value;
44784466
}
44794467
return 1;
4480-
4481-
error:
4482-
if (result) {
4483-
*result = NULL;
4484-
}
4485-
return -1;
44864468
}
44874469

44884470
int

0 commit comments

Comments
 (0)