Support Rewriter::addGCReference, now without leaks

This is for adding a guard on a non-immortal object, since we need
to be safe against that object getting deallocated and a different object
is allocated in its spot.

We had support for this already, but it leaked memory.  The biggest was
that we never freed our runtimeICs, so if those ended up getting any GC
references in them, then we would leak memory.  So I started freeing those,
but then that exposed the issue that the ICInvalidators expect that their
dependent ICs never get freed.  So I added back a mapping from ICSlotInfo->
ICInvalidators that reference them.

src/asm_writing/icinfo.cpp

@@ -35,18 +35,39 @@ using namespace pyston::assembler;
 
 #define MAX_RETRY_BACKOFF 1024
 
-// TODO not right place for this...
 int64_t ICInvalidator::version() {
     return cur_version;
 }
 
+ICInvalidator::~ICInvalidator() {
+    for (ICSlotInfo* slot : dependents) {
+        slot->invalidators.erase(std::find(slot->invalidators.begin(), slot->invalidators.end(), this));
+    }
+}
+
 void ICInvalidator::addDependent(ICSlotInfo* entry_info) {
-    dependents.insert(entry_info);
+    auto p = dependents.insert(entry_info);
+    bool was_inserted = p.second;
+    if (was_inserted)
+        entry_info->invalidators.push_back(this);
 }
 
 void ICInvalidator::invalidateAll() {
     cur_version++;
     for (ICSlotInfo* slot : dependents) {
+        bool found_self = false;
+        for (auto invalidator : slot->invalidators) {
+            if (invalidator == this) {
+                assert(!found_self);
+                found_self = true;
+            } else {
+                assert(invalidator->dependents.count(slot));
+                invalidator->dependents.erase(slot);
+            }
+        }
+        assert(found_self);
+
+        slot->invalidators.clear();
         slot->clear();
     }
     dependents.clear();
@@ -98,6 +119,8 @@ void ICSlotRewrite::commit(CommitHook* hook, std::vector<void*> gc_references,
     if (!still_valid) {
         if (VERBOSITY() >= 3)
             printf("not committing %s icentry since a dependency got updated before commit\n", debug_name);
+        for (auto p : gc_references)
+            Py_DECREF(p);
         return;
     }
 
@@ -106,8 +129,11 @@ void ICSlotRewrite::commit(CommitHook* hook, std::vector<void*> gc_references,
 
     bool do_commit = hook->finishAssembly(continue_point - slot_start);
 
-    if (!do_commit)
+    if (!do_commit) {
+        for (auto p : gc_references)
+            Py_DECREF(p);
         return;
+    }
 
     assert(!assembler.hasFailed());
 
@@ -204,25 +230,8 @@ ICSlotInfo* ICInfo::pickEntryForRewrite(const char* debug_name) {
     return NULL;
 }
 
-// Keep track of all ICInfo(s) that we create because they contain pointers to Pyston heap objects
-// that we have written into the generated code and we may need to scan those.
-static llvm::DenseSet<ICInfo*> ics_list;
 static llvm::DenseMap<void*, ICInfo*> ics_by_return_addr;
 
-void registerGCTrackedICInfo(ICInfo* ic) {
-#if MOVING_GC
-    assert(ics_list.count(ic) == 0);
-    ics_list.insert(ic);
-#endif
-}
-
-void deregisterGCTrackedICInfo(ICInfo* ic) {
-#if MOVING_GC
-    assert(ics_list.count(ic) == 1);
-    ics_list.erase(ic);
-#endif
-}
-
 ICInfo::ICInfo(void* start_addr, void* slowpath_rtn_addr, void* continue_addr, StackInfo stack_info, int num_slots,
                int slot_size, llvm::CallingConv::ID calling_conv, LiveOutSet _live_outs,
                assembler::GenericRegister return_register, TypeRecorder* type_recorder,
@@ -248,16 +257,15 @@ ICInfo::ICInfo(void* start_addr, void* slowpath_rtn_addr, void* continue_addr, S
     }
     if (slowpath_rtn_addr && !this->ic_global_decref_locations.empty())
         slowpath_decref_info = DecrefInfo((uint64_t)slowpath_rtn_addr, this->ic_global_decref_locations);
-
-#if MOVING_GC
-    assert(ics_list.count(this) == 0);
-#endif
 }
 
 ICInfo::~ICInfo() {
-#if MOVING_GC
-    assert(ics_list.count(this) == 0);
-#endif
+    for (auto& slot : slots) {
+        for (auto invalidator : slot.invalidators) {
+            assert(invalidator->dependents.count(&slot));
+            invalidator->dependents.erase(&slot);
+        }
+    }
 }
 
 DecrefInfo::DecrefInfo(uint64_t ip, std::vector<Location> locations) : ip(ip) {
@@ -323,7 +331,9 @@ std::unique_ptr<ICInfo> registerCompiledPatchpoint(uint8_t* start_addr, uint8_t*
 }
 
 void deregisterCompiledPatchpoint(ICInfo* ic) {
-    assert(ics_by_return_addr.count(ic->slowpath_rtn_addr));
+    ic->clearAll();
+
+    assert(ics_by_return_addr[ic->slowpath_rtn_addr] == ic);
     ics_by_return_addr.erase(ic->slowpath_rtn_addr);
 
     deregisterGCTrackedICInfo(ic);
@@ -403,7 +413,7 @@ void ICInfo::appendDecrefInfosTo(std::vector<DecrefInfo>& dest_decref_infos) {
 }
 
 void clearAllICs() {
-    for (auto&& p : ics_by_ast_node) {
+    for (auto&& p : ics_by_return_addr) {
         p.second->clearAll();
     }
 }

src/asm_writing/icinfo.h

@@ -63,6 +63,7 @@ public:
 
     std::vector<void*> gc_references;
     std::vector<DecrefInfo> decref_infos;
+    std::vector<ICInvalidator*> invalidators; // ICInvalidators that reference this slotinfo
 
     void clear();
 };
@@ -183,8 +184,10 @@ public:
     void appendDecrefInfosTo(std::vector<DecrefInfo>& dest_decref_infos);
 };
 
-void registerGCTrackedICInfo(ICInfo* ic);
-void deregisterGCTrackedICInfo(ICInfo* ic);
+inline void registerGCTrackedICInfo(ICInfo* ic) {
+}
+inline void deregisterGCTrackedICInfo(ICInfo* ic) {
+}
 
 class ICSetupInfo;
 struct CompiledFunction;

src/core/types.h

@@ -289,10 +289,14 @@ private:
 
 public:
     ICInvalidator() : cur_version(0) {}
+    ~ICInvalidator();
 
     void addDependent(ICSlotInfo* icentry);
     int64_t version();
     void invalidateAll();
+
+    friend class ICInfo;
+    friend class ICSlotInfo;
 };
 
 // Codegen types:

src/runtime/descr.cpp

@@ -527,6 +527,12 @@ Box* wrapperDescrTppCall(Box* _self, CallRewriteArgs* rewrite_args, ArgPassSpec
         }
     }
 
+    if (rewrite_args) {
+        // We are going to embed references to _self->d_base->wrapper and _self->d_wrapped
+        rewrite_args->obj->addGuard((intptr_t)_self);
+        rewrite_args->rewriter->addGCReference(_self);
+    }
+
     STAT_TIMER(t0, "us_timer_boxedwrapperdecsriptor_call", (_self->cls->is_user_defined ? 10 : 20));
 
     assert(_self->cls == &PyWrapperDescr_Type);

src/runtime/types.cpp

@@ -3761,6 +3761,12 @@ void BoxedClass::dealloc(Box* b) noexcept {
     if (PyObject_IS_GC(type))
         _PyObject_GC_UNTRACK(type);
 
+    type->next_ic.reset();
+    type->hasnext_ic.reset();
+    type->repr_ic.reset();
+    type->iter_ic.reset();
+    type->nonzero_ic.reset();
+
     // We can for the most part avoid this, but I think it's best not to:
     PyObject_ClearWeakRefs((PyObject*)type);
 
@@ -4616,6 +4622,9 @@ extern "C" void Py_Finalize() noexcept {
         Py_DECREF(b);
     }
     constants.clear();
+
+    clearAllICs();
+
     for (auto b : late_constants) {
         Py_DECREF(b);
     }
@@ -4628,6 +4637,9 @@ extern "C" void Py_Finalize() noexcept {
         }
         Py_DECREF(b);
     }
+
+    clearAllICs();
+
     // May need to run multiple collections to collect everything:
     while (PyGC_Collect())
         ;

src/runtime/types.h

@@ -183,7 +183,7 @@ public:
 
     HCAttrs attrs;
 
-    // TODO: these don't actually get deallocated right now
+    // Any new ics here need to get reflected in BoxedClass::dealloc
     std::unique_ptr<CallattrCapiIC> next_ic;
     std::unique_ptr<CallattrIC> hasnext_ic, repr_ic, iter_ic;
     std::unique_ptr<NonzeroIC> nonzero_ic;