Coredumpers from Michael Hudson, mutating dicts while printing or

converting to string.
Critical bugfix candidate -- if you take this seriously <wink>.

diff --git a/Lib/test/test_mutants.py b/Lib/test/test_mutants.py
index 3cd35d17a5ffce4eefe1ea1df258f8fd38bbc85e..f981fe67b094ab3ef6e918956520f331043788e7 100644 (file)
--- a/Lib/test/test_mutants.py
+++ b/Lib/test/test_mutants.py
@@ -1,5 +1,6 @@
-from test_support import verbose
+from test_support import verbose, TESTFN
 import random
+import os
 
 # From SF bug #422121:  Insecurities in dict comparison.
 
@@ -151,3 +152,66 @@ def test(n):
 
 # See last comment block for clues about good values for n.
 test(100)
+
+##########################################################################
+# Another segfault bug, distilled by Micheal Hundson from a c.l.py post.
+
+class Child:
+    def __init__(self, parent):
+        self.__dict__['parent'] = parent
+    def __getattr__(self, attr):
+        self.parent.a = 1
+        self.parent.b = 1
+        self.parent.c = 1
+        self.parent.d = 1
+        self.parent.e = 1
+        self.parent.f = 1
+        self.parent.g = 1
+        self.parent.h = 1
+        self.parent.i = 1
+        return getattr(self.parent, attr)
+
+class Parent:
+    def __init__(self):
+        self.a = Child(self)
+
+# Hard to say what this will print!  May vary from time to time.  But
+# we're specifically trying to test the tp_print slot here, and this is
+# the clearest way to do it.  We print the result to a temp file so that
+# the expected-output file doesn't need to change.
+
+f = open(TESTFN, "w")
+print >> f, Parent().__dict__
+f.close()
+os.unlink(TESTFN)
+
+##########################################################################
+# And another core-dumper from Michael Hudson.
+
+dict = {}
+
+# Force dict to malloc its table.
+for i in range(1, 10):
+    dict[i] = i
+
+f = open(TESTFN, "w")
+
+class Machiavelli:
+    def __repr__(self):
+        dict.clear()
+
+        # Michael sez:  "doesn't crash without this.  don't know why."
+        # Tim sez:  "luck of the draw; crashes with or without for me."
+        print >> f
+
+        return `"machiavelli"`
+
+    def __hash__(self):
+        return 0
+
+dict[Machiavelli()] = Machiavelli()
+
+print >> f, str(dict)
+f.close()
+os.unlink(TESTFN)
+del f, dict

diff --git a/Objects/dictobject.c b/Objects/dictobject.c
index 857b3c6ffd514d9cd30f1350c2403070f841d583..8b58166ca43ec5f006f6faa005f08a56957077e0 100644 (file)
--- a/Objects/dictobject.c
+++ b/Objects/dictobject.c
@@ -740,7 +740,6 @@ dict_print(register dictobject *mp, register FILE *fp, register int flags)
 {
        register int i;
        register int any;
-       register dictentry *ep;
 
        i = Py_ReprEnter((PyObject*)mp);
        if (i != 0) {
@@ -752,19 +751,27 @@ dict_print(register dictobject *mp, register FILE *fp, register int flags)
 
        fprintf(fp, "{");
        any = 0;
-       for (i = 0, ep = mp->ma_table; i < mp->ma_size; i++, ep++) {
-               if (ep->me_value != NULL) {
+       for (i = 0; i < mp->ma_size; i++) {
+               dictentry *ep = mp->ma_table + i;
+               PyObject *pvalue = ep->me_value;
+               if (pvalue != NULL) {
+                       /* Prevent PyObject_Repr from deleting value during
+                          key format */
+                       Py_INCREF(pvalue);
                        if (any++ > 0)
                                fprintf(fp, ", ");
                        if (PyObject_Print((PyObject *)ep->me_key, fp, 0)!=0) {
+                               Py_DECREF(pvalue);
                                Py_ReprLeave((PyObject*)mp);
                                return -1;
                        }
                        fprintf(fp, ": ");
                        if (PyObject_Print(ep->me_value, fp, 0) != 0) {
+                               Py_DECREF(pvalue);
                                Py_ReprLeave((PyObject*)mp);
                                return -1;
                        }
+                       Py_DECREF(pvalue);
                }
        }
        fprintf(fp, "}");
@@ -779,7 +786,6 @@ dict_repr(dictobject *mp)
        PyObject *sepa, *colon;
        register int i;
        register int any;
-       register dictentry *ep;
 
        i = Py_ReprEnter((PyObject*)mp);
        if (i != 0) {
@@ -792,13 +798,19 @@ dict_repr(dictobject *mp)
        sepa = PyString_FromString(", ");
        colon = PyString_FromString(": ");
        any = 0;
-       for (i = 0, ep = mp->ma_table; i < mp->ma_size && v; i++, ep++) {
-               if (ep->me_value != NULL) {
+       for (i = 0; i < mp->ma_size && v; i++) {
+               dictentry *ep = mp->ma_table + i;
+               PyObject *pvalue = ep->me_value;
+               if (pvalue != NULL) {
+                       /* Prevent PyObject_Repr from deleting value during
+                          key format */
+                       Py_INCREF(pvalue);
                        if (any++)
                                PyString_Concat(&v, sepa);
                        PyString_ConcatAndDel(&v, PyObject_Repr(ep->me_key));
                        PyString_Concat(&v, colon);
-                       PyString_ConcatAndDel(&v, PyObject_Repr(ep->me_value));
+                       PyString_ConcatAndDel(&v, PyObject_Repr(pvalue));
+                       Py_DECREF(pvalue);
                }
        }
        PyString_ConcatAndDel(&v, PyString_FromString("}"));