Debugging helper: invalidate freed objects

Without this, one could use freed objects without issues,
until something else was allocated in that space.  And even then,
it would still be a valid object.

So, in debug mode overwrite the data with garbage to try to surface
these issues.

This exposed an issue with our "nonheap roots" handling, where we
weren't scanning all of the memory that they pointed to.  This is
mostly fine, but there are some cases (time.gmtime) where gc-allocated
memory would be stored in these objects.  So, now you have to register
the size of the object, and the memory range will be scanned conservatively.

src/capi/typeobject.cpp

@@ -2930,7 +2930,7 @@ extern "C" void PyType_Modified(PyTypeObject* type) noexcept {
 extern "C" int PyType_Ready(PyTypeObject* cls) noexcept {
     ASSERT(!cls->is_pyston_class, "should not call this on Pyston classes");
 
-    gc::registerNonheapRootObject(cls);
+    gc::registerNonheapRootObject(cls, sizeof(PyTypeObject));
 
     // unhandled fields:
     int ALLOWABLE_FLAGS = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_CHECKTYPES

src/gc/collector.cpp

@@ -162,12 +162,13 @@ static std::unordered_set<void*> nonheap_roots;
 // way to verify it's not a nonheap root (the full check requires a hashtable lookup).
 static void* max_nonheap_root = 0;
 static void* min_nonheap_root = (void*)~0;
-void registerNonheapRootObject(void* obj) {
+void registerNonheapRootObject(void* obj, int size) {
     // I suppose that things could work fine even if this were true, but why would it happen?
     assert(global_heap.getAllocationFromInteriorPointer(obj) == NULL);
     assert(nonheap_roots.count(obj) == 0);
 
     nonheap_roots.insert(obj);
+    registerPotentialRootRange(obj, ((uint8_t*)obj) + size);
 
     max_nonheap_root = std::max(obj, max_nonheap_root);
     min_nonheap_root = std::min(obj, min_nonheap_root);
@@ -269,16 +270,6 @@ void markPhase() {
     threading::visitAllStacks(&visitor);
     gatherInterpreterRoots(&visitor);
 
-    for (void* p : nonheap_roots) {
-        Box* b = reinterpret_cast<Box*>(p);
-        BoxedClass* cls = b->cls;
-
-        if (cls) {
-            ASSERT(cls->gc_visit, "%s", getTypeName(b));
-            cls->gc_visit(&visitor, b);
-        }
-    }
-
     for (auto h : *getRootHandles()) {
         visitor.visit(h->value);
     }

src/gc/collector.h

@@ -31,7 +31,8 @@ void deregisterPermanentRoot(void* root_obj);
 // Register an object that was not allocated through this collector, as a root for this collector.
 // The motivating usecase is statically-allocated PyTypeObject objects, which are full Python objects
 // even if they are not heap allocated.
-void registerNonheapRootObject(void* obj);
+// This memory will be scanned conservatively.
+void registerNonheapRootObject(void* obj, int size);
 
 void registerPotentialRootRange(void* start, void* end);
 

src/gc/heap.cpp

@@ -470,7 +470,9 @@ SmallArena::Block** SmallArena::_freeChain(Block** head, std::vector<Box*>& weak
             } else {
                 if (_doFree(al, &weakly_referenced)) {
                     b->isfree.set(atom_idx);
-                    // memset(al->user_data, 0, b->size - sizeof(GCAllocation));
+#ifndef NDEBUG
+                    memset(al->user_data, 0xbb, b->size - sizeof(GCAllocation));
+#endif
                 }
             }
         }

src/runtime/capi.cpp

@@ -85,21 +85,6 @@ extern "C" void _PyErr_BadInternalCall(const char* filename, int lineno) noexcep
     Py_FatalError("unimplemented");
 }
 
-extern "C" PyVarObject* PyObject_InitVar(PyVarObject* op, PyTypeObject* tp, Py_ssize_t size) noexcept {
-    assert(gc::isValidGCObject(op));
-    assert(gc::isValidGCObject(tp));
-
-    RELEASE_ASSERT(op, "");
-    RELEASE_ASSERT(tp, "");
-
-    gc::setIsPythonObject(op);
-
-    Py_TYPE(op) = tp;
-    op->ob_size = size;
-
-    return op;
-}
-
 extern "C" PyObject* PyObject_Format(PyObject* obj, PyObject* format_spec) noexcept {
     PyObject* empty = NULL;
     PyObject* result = NULL;

src/runtime/types.cpp

@@ -95,7 +95,8 @@ bool IN_SHUTDOWN = false;
 extern "C" PyObject* PystonType_GenericAlloc(BoxedClass* cls, Py_ssize_t nitems) noexcept {
     assert(cls);
 
-    const size_t size = _PyObject_VAR_SIZE(cls, nitems);
+    // See PyType_GenericAlloc for note about the +1 here:
+    const size_t size = _PyObject_VAR_SIZE(cls, nitems + 1);
 
 #ifndef NDEBUG
 #if 0
@@ -145,7 +146,10 @@ extern "C" PyObject* PystonType_GenericAlloc(BoxedClass* cls, Py_ssize_t nitems)
 extern "C" PyObject* PyType_GenericAlloc(PyTypeObject* type, Py_ssize_t nitems) noexcept {
     PyObject* obj;
     const size_t size = _PyObject_VAR_SIZE(type, nitems + 1);
-    /* note that we need to add one, for the sentinel [CPython comment] */
+    // I don't understand why there is a +1 in this method; _PyObject_NewVar doesn't do that.
+    // CPython has the following comment:
+    /* note that we need to add one, for the sentinel */
+    // I think that regardless of the reasoning behind them having it, we should do what they do?
 
     if (PyType_IS_GC(type))
         obj = _PyObject_GC_Malloc(size);
@@ -2228,6 +2232,21 @@ inline void initUserAttrs(Box* obj, BoxedClass* cls) {
 
 extern "C" void PyCallIter_AddHasNext();
 
+extern "C" PyVarObject* PyObject_InitVar(PyVarObject* op, PyTypeObject* tp, Py_ssize_t size) noexcept {
+    assert(gc::isValidGCObject(op));
+    assert(gc::isValidGCObject(tp));
+
+    RELEASE_ASSERT(op, "");
+    RELEASE_ASSERT(tp, "");
+
+    gc::setIsPythonObject(op);
+
+    Py_TYPE(op) = tp;
+    op->ob_size = size;
+
+    return op;
+}
+
 extern "C" PyObject* PyObject_Init(PyObject* op, PyTypeObject* tp) noexcept {
     assert(op);
     assert(tp);