[Python-checkins] bpo-38525: Fix a segmentation fault when using reverse iterators of empty dict (GH-16846)

[Miss Islington (bot)]

https://github.com/python/cpython/commit/d73205d788a32148ba9a2beaa27badbd94ab65ff
commit: d73205d788a32148ba9a2beaa27badbd94ab65ff
branch: 3.8
author: Miss Islington (bot) <31488909+miss-islington at users.noreply.github.com>
committer: GitHub <noreply at github.com>
date: 2019-10-19T13:20:51-07:00
summary:

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


The reverse iterator for empty dictionaries was not handling correctly shared-key dictionaries.
(cherry picked from commit 24dc2f8c56697f9ee51a4887cf0814b6600c1815)

Co-authored-by: Dong-hee Na <donghee.na92 at gmail.com>

files:
A Misc/NEWS.d/next/Core and Builtins/2019-10-20-00-36-18.bpo-38525.Vty1cA.rst
M Lib/test/test_dict.py
M Objects/dictobject.c

diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py
index 13be857f7ab60..5b513765f7b08 100644
--- a/Lib/test/test_dict.py
+++ b/Lib/test/test_dict.py
@@ -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)])
diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-10-20-00-36-18.bpo-38525.Vty1cA.rst b/Misc/NEWS.d/next/Core and Builtins/2019-10-20-00-36-18.bpo-38525.Vty1cA.rst
new file mode 100644
index 0000000000000..c74d143762253
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2019-10-20-00-36-18.bpo-38525.Vty1cA.rst	
@@ -0,0 +1,2 @@
+Fix a segmentation fault when using reverse iterators of empty ``dict`` objects.
+Patch by Dong-hee Na and Inada Naoki.
diff --git a/Objects/dictobject.c b/Objects/dictobject.c
index 5dad1b6d9208c..a0208be9246d9 100644
--- a/Objects/dictobject.c
+++ b/Objects/dictobject.c
@@ -3462,10 +3462,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;