Skip to content

bpo-38525: Fix a segmentation fault when using reverse iterators of empty dict #16846

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Oct 19, 2019
Merged

bpo-38525: Fix a segmentation fault when using reverse iterators of empty dict #16846

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6f2dda4
bpo-38525: Fix dictreviter_iternext for empty dict
corona10 Oct 19, 2019
dc1a73a
bpo-38525: Apply codereview
corona10 Oct 19, 2019
2d1c588
bpo-38525: Apply vstinner's review
corona10 Oct 19, 2019
15183c5
bpo-38525: Add dict() test case
corona10 Oct 19, 2019
d92a103
Update news
corona10 Oct 19, 2019
8618a7a
bpo-38525: di_pos must be initialized from ma_used when dict is key s…
corona10 Oct 19, 2019
b86f723
bpo-38525: update
corona10 Oct 19, 2019
bf61754
bpo-38525: Update
corona10 Oct 19, 2019
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_dict.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1312,6 +1312,31 @@ def test_reversed(self):
self.assertEqual(list(r), list('dcba'))
self.assertRaises(StopIteration, next, r)

def test_reverse_iterator_for_empty_dict(self):
# bpo-38525: revered iterator should work properly

# empty dict is directly used for reference count test
self.assertEqual(list(reversed({})), [])
self.assertEqual(list(reversed({}.items())), [])
self.assertEqual(list(reversed({}.values())), [])
self.assertEqual(list(reversed({}.keys())), [])

# dict() and {} don't trigger the same code path
self.assertEqual(list(reversed(dict())), [])
self.assertEqual(list(reversed(dict().items())), [])
self.assertEqual(list(reversed(dict().values())), [])
self.assertEqual(list(reversed(dict().keys())), [])

def test_reverse_iterator_for_shared_shared_dicts(self):
class A:
def __init__(self, x, y):
if x: self.x = x
if y: self.y = y

self.assertEqual(list(reversed(A(1, 2).__dict__)), ['y', 'x'])
self.assertEqual(list(reversed(A(1, 0).__dict__)), ['x'])
self.assertEqual(list(reversed(A(0, 1).__dict__)), ['y'])

def test_dict_copy_order(self):
# bpo-34320
od = collections.OrderedDict([('a', 1), ('b', 2)])
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Core and Builtins/2019-10-20-00-36-18.bpo-38525.Vty1cA.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix a segmentation fault when using reverse iterators of empty ``dict`` objects.
Patch by Dong-hee Na and Inada Naoki.
9 changes: 7 additions & 2 deletions Objects/dictobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3452,10 +3452,15 @@ dictiter_new(PyDictObject *dict, PyTypeObject *itertype)
di->di_dict = dict;
di->di_used = dict->ma_used;
di->len = dict->ma_used;
if ((itertype == &PyDictRevIterKey_Type ||
if (itertype == &PyDictRevIterKey_Type ||
itertype == &PyDictRevIterItem_Type ||
itertype == &PyDictRevIterValue_Type) && dict->ma_used) {
itertype == &PyDictRevIterValue_Type) {
if (dict->ma_values) {
di->di_pos = dict->ma_used - 1;
}
else {
di->di_pos = dict->ma_keys->dk_nentries - 1;
}
}
else {
di->di_pos = 0;
Expand Down