bpo-38392: PyObject_GC_Track() validates object in debug mode (GH-16615)

In debug mode, PyObject_GC_Track() now calls tp_traverse() of the
object type to ensure that the object is valid: test that objects
visited by tp_traverse() are valid.

Fix pyexpat.c: only track the parser in the GC once the parser is
fully initialized.

4 files changed, 27 insertions, 4 deletions

diff --git a/Misc/NEWS.d/next/Core and Builtins/2019-10-07-22-51-39.bpo-38392.KaXXps.rst b/Misc/NEWS.d/next/Core and Builtins/2019-10-07-22-51-39.bpo-38392.KaXXps.rst
new file mode 100644
index 0000000000..07358ece75
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2019-10-07-22-51-39.bpo-38392.KaXXps.rst
@@ -0,0 +1,3 @@
+In debug mode, :c:func:`PyObject_GC_Track` now calls ``tp_traverse()`` of
+the object type to ensure that the object is valid: test that objects
+visited by ``tp_traverse()`` are valid.
diff --git a/Modules/_elementtree.c b/Modules/_elementtree.c
index 9050144b70..c096eab43e 100644
--- a/Modules/_elementtree.c
+++ b/Modules/_elementtree.c
@@ -2317,8 +2317,6 @@ create_elementiter(ElementObject *self, PyObject *tag, int gettext)
     Py_INCREF(self);
     it->root_element = self;
 
-    PyObject_GC_Track(it);
-
     it->parent_stack = PyMem_New(ParentLocator, INIT_PARENT_STACK_SIZE);
     if (it->parent_stack == NULL) {
         Py_DECREF(it);
@@ -2328,6 +2326,8 @@ create_elementiter(ElementObject *self, PyObject *tag, int gettext)
     it->parent_stack_used = 0;
     it->parent_stack_size = INIT_PARENT_STACK_SIZE;
 
+    PyObject_GC_Track(it);
+
     return (PyObject *)it;
 }
 
diff --git a/Modules/gcmodule.c b/Modules/gcmodule.c
index 107ce39967..2cbf573cf4 100644
--- a/Modules/gcmodule.c
+++ b/Modules/gcmodule.c
@@ -1922,6 +1922,18 @@ _PyGC_Dump(PyGC_Head *g)
     _PyObject_Dump(FROM_GC(g));
 }
 
+static int
+visit_validate(PyObject *op, void *parent_raw)
+{
+    PyObject *parent = _PyObject_CAST(parent_raw);
+    if (_PyObject_IsFreed(op)) {
+        _PyObject_ASSERT_FAILED_MSG(parent,
+                                    "PyObject_GC_Track() object is not valid");
+    }
+    return 0;
+}
+
+
 /* extension modules might be compiled with GC support so these
    functions must always be available */
 
@@ -1935,6 +1947,13 @@ PyObject_GC_Track(void *op_raw)
                                     "by the garbage collector");
     }
     _PyObject_GC_TRACK(op);
+
+#ifdef Py_DEBUG
+    /* Check that the object is valid: validate objects traversed
+       by tp_traverse() */
+    traverseproc traverse = Py_TYPE(op)->tp_traverse;
+    (void)traverse(op, visit_validate, op);
+#endif
 }
 
 void
diff --git a/Modules/pyexpat.c b/Modules/pyexpat.c
index d9123354bb..90167342fe 100644
--- a/Modules/pyexpat.c
+++ b/Modules/pyexpat.c
@@ -938,7 +938,6 @@ pyexpat_xmlparser_ExternalEntityParserCreate_impl(xmlparseobject *self,
     new_parser->handlers = 0;
     new_parser->intern = self->intern;
     Py_XINCREF(new_parser->intern);
-    PyObject_GC_Track(new_parser);
 
     if (self->buffer != NULL) {
         new_parser->buffer = PyMem_Malloc(new_parser->buffer_size);
@@ -975,6 +974,8 @@ pyexpat_xmlparser_ExternalEntityParserCreate_impl(xmlparseobject *self,
                                    handler_info[i].handler);
         }
     }
+
+    PyObject_GC_Track(new_parser);
     return (PyObject *)new_parser;
 }
 
@@ -1122,7 +1123,6 @@ newxmlparseobject(const char *encoding, const char *namespace_separator, PyObjec
     self->handlers = NULL;
     self->intern = intern;
     Py_XINCREF(self->intern);
-    PyObject_GC_Track(self);
 
     /* namespace_separator is either NULL or contains one char + \0 */
     self->itself = XML_ParserCreate_MM(encoding, &ExpatMemoryHandler,
@@ -1152,6 +1152,7 @@ newxmlparseobject(const char *encoding, const char *namespace_separator, PyObjec
     }
     clear_handlers(self, 1);
 
+    PyObject_GC_Track(self);
     return (PyObject*)self;
 }