Conservatively scan the ICInvalidators stored in the rewriter.

We need to do this because the ICInvalidator pointers are stored inside
Pyston heap objects.

src/asm_writing/icinfo.cpp

@@ -128,6 +128,12 @@ void ICSlotRewrite::commit(CommitHook* hook) {
     llvm::sys::Memory::InvalidateInstructionCache(slot_start, ic->getSlotSize());
 }
 
+void ICSlotRewrite::gc_visit(GCVisitor* visitor) {
+    for (auto& dependency : dependencies) {
+        visitor->visitPotentialRedundant(dependency.first);
+    }
+}
+
 void ICSlotRewrite::addDependenceOn(ICInvalidator& invalidator) {
     dependencies.push_back(std::make_pair(&invalidator, invalidator.version()));
 }

src/asm_writing/icinfo.h

@@ -82,6 +82,8 @@ public:
 
     ICSlotInfo* prepareEntry();
 
+    void gc_visit(gc::GCVisitor* visitor);
+
     void addDependenceOn(ICInvalidator&);
     void commit(CommitHook* hook);
     void abort();

src/asm_writing/rewriter.cpp

@@ -1376,6 +1376,10 @@ void Rewriter::addDependenceOn(ICInvalidator& invalidator) {
     rewrite->addDependenceOn(invalidator);
 }
 
+void Rewriter::gc_visit(GCVisitor* visitor) {
+    rewrite->gc_visit(visitor);
+}
+
 Location Rewriter::allocScratch() {
     assertPhaseEmitting();
 

src/asm_writing/rewriter.h

@@ -28,6 +28,7 @@
 #include "asm_writing/assembler.h"
 #include "asm_writing/icinfo.h"
 #include "core/threading.h"
+#include "gc/gc.h"
 
 namespace pyston {
 
@@ -331,7 +332,7 @@ enum class ActionType { NORMAL, GUARD, MUTATION };
 // non-NULL fake pointer, definitely legit
 #define LOCATION_PLACEHOLDER ((RewriterVar*)1)
 
-class Rewriter : public ICSlotRewrite::CommitHook {
+class Rewriter : public ICSlotRewrite::CommitHook, public gc::GCVisitable {
 private:
     class RegionAllocator {
     public:
@@ -586,6 +587,8 @@ public:
 
     void addDependenceOn(ICInvalidator&);
 
+    void gc_visit(gc::GCVisitor* visitor);
+
     static Rewriter* createRewriter(void* rtn_addr, int num_args, const char* debug_name);
 
     static bool isLargeConstant(int64_t val) { return (val < (-1L << 31) || val >= (1L << 31) - 1); }

src/gc/collector.h

@@ -47,8 +47,8 @@ public:
 
     void operator=(Box* b) { value = b; }
 
-    operator Box*() { return value; }
     Box* operator->() { return value; }
+    Box* get() { return value; }
 };
 
 bool isNonheapRoot(void* p);

src/gc/gc.h

@@ -43,6 +43,15 @@ namespace pyston {
 
 class Box;
 
+namespace gc {
+class GCVisitable;
+}
+
+namespace threading {
+void pushGCObject(gc::GCVisitable* obj);
+void popGCObject(gc::GCVisitable* obj);
+}
+
 namespace gc {
 
 class TraceStack;
@@ -156,25 +165,40 @@ public:
 
 // This is a way to call gc_visit on objects whose lifetime is bound to the stack,
 // but may be contained within a unique_ptr or some other container.
-class ScanningHandle {
-    GCVisitable* obj;
+template <typename T> class UniqueScanningHandle {
+    T* obj = NULL;
 
 public:
-    void push();
-    void pop();
+    UniqueScanningHandle(T* obj) : obj(obj) {
+#if MOVING_GC
+        if (obj) {
+            threading::pushGCObject(obj);
+        }
+#endif
+    }
 
-    ScanningHandle(GCVisitable* obj) : obj(obj) {
+    ~UniqueScanningHandle() {
 #if MOVING_GC
         if (obj) {
-            push();
+            threading::popGCObject(obj);
         }
 #endif
+        delete obj;
     }
 
-    ~ScanningHandle() {
+    T* operator->() { return obj; }
+    T* get() { return obj; }
+    void reset(T* t = nullptr) {
 #if MOVING_GC
         if (obj) {
-            pop();
+            threading::popGCObject(obj);
+        }
+#endif
+        delete obj;
+        obj = t;
+#if MOVING_GC
+        if (t) {
+            threading::pushGCObject(t);
         }
 #endif
     }

src/gc/heap.cpp

@@ -45,14 +45,6 @@ template <> void return_temporary_buffer<pyston::Box*>(pyston::Box** p) {
 namespace pyston {
 namespace gc {
 
-void ScanningHandle::push() {
-    threading::pushGCObject(obj);
-}
-
-void ScanningHandle::pop() {
-    threading::popGCObject(obj);
-}
-
 bool _doFree(GCAllocation* al, std::vector<Box*>* weakly_referenced);
 
 // lots of linked lists around here, so let's just use template functions for operations on them.

src/runtime/objmodel.cpp

@@ -1550,7 +1550,7 @@ extern "C" Box* getclsattr(Box* obj, BoxedString* attr) {
     }
 
 #if 0
-    std::unique_ptr<Rewriter> rewriter(Rewriter::createRewriter(__builtin_extract_return_addr(__builtin_return_address(0)), 2, 1, "getclsattr"));
+    gc::UniqueScanningHandle<Rewriter> rewriter(Rewriter::createRewriter(__builtin_extract_return_addr(__builtin_return_address(0)), 2, 1, "getclsattr"));
 
     if (rewriter.get()) {
         //rewriter->trap();
@@ -1562,7 +1562,7 @@ extern "C" Box* getclsattr(Box* obj, BoxedString* attr) {
             rewriter->commit();
         }
 #else
-    std::unique_ptr<Rewriter> rewriter(
+    gc::UniqueScanningHandle<Rewriter> rewriter(
         Rewriter::createRewriter(__builtin_extract_return_addr(__builtin_return_address(0)), 2, "getclsattr"));
 
     if (rewriter.get()) {
@@ -1993,7 +1993,7 @@ template <ExceptionStyle S> Box* _getattrEntry(Box* obj, BoxedString* attr, void
 #endif
     }
 
-    std::unique_ptr<Rewriter> rewriter(Rewriter::createRewriter(return_addr, 2, "getattr"));
+    gc::UniqueScanningHandle<Rewriter> rewriter(Rewriter::createRewriter(return_addr, 2, "getattr"));
 
 #if 0 && STAT_TIMERS
     static uint64_t* st_id = Stats::getStatCounter("us_timer_slowpath_getattr_patchable");
@@ -2262,7 +2262,7 @@ extern "C" void setattr(Box* obj, BoxedString* attr, Box* attr_val) {
         return;
     }
 
-    std::unique_ptr<Rewriter> rewriter(
+    gc::UniqueScanningHandle<Rewriter> rewriter(
         Rewriter::createRewriter(__builtin_extract_return_addr(__builtin_return_address(0)), 3, "setattr"));
 
     setattrofunc tp_setattro = obj->cls->tp_setattro;
@@ -2360,7 +2360,7 @@ extern "C" bool nonzero(Box* obj) {
     static StatCounter slowpath_nonzero("slowpath_nonzero");
     slowpath_nonzero.log();
 
-    std::unique_ptr<Rewriter> rewriter(
+    gc::UniqueScanningHandle<Rewriter> rewriter(
         Rewriter::createRewriter(__builtin_extract_return_addr(__builtin_return_address(0)), 1, "nonzero"));
 
     RewriterVar* r_obj = NULL;
@@ -2456,16 +2456,18 @@ extern "C" bool nonzero(Box* obj) {
     static BoxedString* len_str = internStringImmortal("__len__");
 
     // try __nonzero__
-    CallRewriteArgs crewrite_args(rewriter.get(), r_obj, rewriter ? rewriter->getReturnDestination() : Location());
-    Box* rtn = callattrInternal0<CXX>(obj, nonzero_str, CLASS_ONLY, rewriter ? &crewrite_args : NULL, ArgPassSpec(0));
+    CallRewriteArgs crewrite_args(rewriter.get(), r_obj,
+                                  rewriter.get() ? rewriter->getReturnDestination() : Location());
+    Box* rtn
+        = callattrInternal0<CXX>(obj, nonzero_str, CLASS_ONLY, rewriter.get() ? &crewrite_args : NULL, ArgPassSpec(0));
     if (!crewrite_args.out_success)
         rewriter.reset();
 
     if (!rtn) {
         // try __len__
         crewrite_args
-            = CallRewriteArgs(rewriter.get(), r_obj, rewriter ? rewriter->getReturnDestination() : Location());
-        rtn = callattrInternal0<CXX>(obj, len_str, CLASS_ONLY, rewriter ? &crewrite_args : NULL, ArgPassSpec(0));
+            = CallRewriteArgs(rewriter.get(), r_obj, rewriter.get() ? rewriter->getReturnDestination() : Location());
+        rtn = callattrInternal0<CXX>(obj, len_str, CLASS_ONLY, rewriter.get() ? &crewrite_args : NULL, ArgPassSpec(0));
         if (!crewrite_args.out_success)
             rewriter.reset();
 
@@ -2738,7 +2740,7 @@ extern "C" i64 unboxedLen(Box* obj) {
     static StatCounter slowpath_unboxedlen("slowpath_unboxedlen");
     slowpath_unboxedlen.log();
 
-    std::unique_ptr<Rewriter> rewriter(
+    gc::UniqueScanningHandle<Rewriter> rewriter(
         Rewriter::createRewriter(__builtin_extract_return_addr(__builtin_return_address(0)), 1, "unboxedLen"));
 
     BoxedInt* lobj;
@@ -2932,7 +2934,7 @@ Box* _callattrEntry(Box* obj, BoxedString* attr, CallattrFlags flags, Box* arg1,
     // Uncomment this to help debug if callsites aren't getting rewritten:
     // printf("Slowpath call: %p (%s.%s)\n", return_addr, obj->cls->tp_name, attr->c_str());
 
-    std::unique_ptr<Rewriter> rewriter(Rewriter::createRewriter(return_addr, num_orig_args, "callattr"));
+    gc::UniqueScanningHandle<Rewriter> rewriter(Rewriter::createRewriter(return_addr, num_orig_args, "callattr"));
     Box* rtn;
 
     LookupScope scope = flags.cls_only ? CLASS_ONLY : CLASS_OR_INST;
@@ -4055,7 +4057,8 @@ static Box* runtimeCallEntry(Box* obj, ArgPassSpec argspec, Box* arg1, Box* arg2
         assert(argspec.num_keywords == keyword_names->size());
         num_orig_args++;
     }
-    std::unique_ptr<Rewriter> rewriter(Rewriter::createRewriter(return_addr, num_orig_args, "runtimeCall"));
+    gc::UniqueScanningHandle<Rewriter> rewriter(Rewriter::createRewriter(return_addr, num_orig_args, "runtimeCall"));
+
     Box* rtn;
 
 #if 0 && STAT_TIMERS
@@ -4258,7 +4261,7 @@ extern "C" Box* binop(Box* lhs, Box* rhs, int op_type) {
     // int id = Stats::getStatId("slowpath_binop_" + *getTypeName(lhs) + op_name + *getTypeName(rhs));
     // Stats::log(id);
 
-    std::unique_ptr<Rewriter> rewriter((Rewriter*)NULL);
+    gc::UniqueScanningHandle<Rewriter> rewriter((Rewriter*)NULL);
     // Currently can't patchpoint user-defined binops since we can't assume that just because
     // resolving it one way right now (ex, using the value from lhs.__add__) means that later
     // we'll resolve it the same way, even for the same argument types.
@@ -4295,7 +4298,7 @@ extern "C" Box* augbinop(Box* lhs, Box* rhs, int op_type) {
     // int id = Stats::getStatId("slowpath_augbinop_" + *getTypeName(lhs) + op_name + *getTypeName(rhs));
     // Stats::log(id);
 
-    std::unique_ptr<Rewriter> rewriter((Rewriter*)NULL);
+    gc::UniqueScanningHandle<Rewriter> rewriter((Rewriter*)NULL);
     // Currently can't patchpoint user-defined binops since we can't assume that just because
     // resolving it one way right now (ex, using the value from lhs.__add__) means that later
     // we'll resolve it the same way, even for the same argument types.
@@ -4596,7 +4599,7 @@ extern "C" Box* compare(Box* lhs, Box* rhs, int op_type) {
     slowpath_compare.log();
     static StatCounter nopatch_compare("nopatch_compare");
 
-    std::unique_ptr<Rewriter> rewriter(
+    gc::UniqueScanningHandle<Rewriter> rewriter(
         Rewriter::createRewriter(__builtin_extract_return_addr(__builtin_return_address(0)), 3, "compare"));
 
     if (rewriter.get()) {
@@ -4655,11 +4658,11 @@ extern "C" Box* unaryop(Box* operand, int op_type) {
 
     BoxedString* op_name = getOpName(op_type);
 
-    std::unique_ptr<Rewriter> rewriter(
+    gc::UniqueScanningHandle<Rewriter> rewriter(
         Rewriter::createRewriter(__builtin_extract_return_addr(__builtin_return_address(0)), 1, "unaryop"));
 
     Box* rtn = NULL;
-    if (rewriter) {
+    if (rewriter.get()) {
         CallRewriteArgs srewrite_args(rewriter.get(), rewriter->getArg(0), rewriter->getReturnDestination());
         rtn = callattrInternal0<CXX>(operand, op_name, CLASS_ONLY, &srewrite_args, ArgPassSpec(0));
         if (srewrite_args.out_success && rtn)
@@ -4893,7 +4896,7 @@ extern "C" Box* getitem(Box* target, Box* slice) {
     static StatCounter slowpath_getitem("slowpath_getitem");
     slowpath_getitem.log();
 
-    std::unique_ptr<Rewriter> rewriter(
+    gc::UniqueScanningHandle<Rewriter> rewriter(
         Rewriter::createRewriter(__builtin_extract_return_addr(__builtin_return_address(0)), 2, "getitem"));
 
     Box* rtn;
@@ -4927,7 +4930,7 @@ extern "C" Box* getitem_capi(Box* target, Box* slice) noexcept {
     static StatCounter slowpath_getitem("slowpath_getitem");
     slowpath_getitem.log();
 
-    std::unique_ptr<Rewriter> rewriter(
+    gc::UniqueScanningHandle<Rewriter> rewriter(
         Rewriter::createRewriter(__builtin_extract_return_addr(__builtin_return_address(0)), 2, "getitem"));
 
     Box* rtn;
@@ -4956,7 +4959,7 @@ extern "C" void setitem(Box* target, Box* slice, Box* value) {
     static StatCounter slowpath_setitem("slowpath_setitem");
     slowpath_setitem.log();
 
-    std::unique_ptr<Rewriter> rewriter(
+    gc::UniqueScanningHandle<Rewriter> rewriter(
         Rewriter::createRewriter(__builtin_extract_return_addr(__builtin_return_address(0)), 3, "setitem"));
 
     static BoxedString* setitem_str = internStringImmortal("__setitem__");
@@ -4992,7 +4995,7 @@ extern "C" void delitem(Box* target, Box* slice) {
     static StatCounter slowpath_delitem("slowpath_delitem");
     slowpath_delitem.log();
 
-    std::unique_ptr<Rewriter> rewriter(
+    gc::UniqueScanningHandle<Rewriter> rewriter(
         Rewriter::createRewriter(__builtin_extract_return_addr(__builtin_return_address(0)), 2, "delitem"));
 
     static BoxedString* delitem_str = internStringImmortal("__delitem__");
@@ -5155,7 +5158,7 @@ extern "C" Box* createBoxedIterWrapper(Box* o) {
 extern "C" Box* createBoxedIterWrapperIfNeeded(Box* o) {
     STAT_TIMER(t0, "us_timer_slowpath_createBoxedIterWrapperIfNeeded", 10);
 
-    std::unique_ptr<Rewriter> rewriter(Rewriter::createRewriter(
+    gc::UniqueScanningHandle<Rewriter> rewriter(Rewriter::createRewriter(
         __builtin_extract_return_addr(__builtin_return_address(0)), 1, "createBoxedIterWrapperIfNeeded"));
 
     static BoxedString* hasnext_str = internStringImmortal("__hasnext__");
@@ -5646,7 +5649,7 @@ extern "C" Box* getGlobal(Box* globals, BoxedString* name) {
     }
 
     { /* anonymous scope to make sure destructors get run before we err out */
-        std::unique_ptr<Rewriter> rewriter(
+        gc::UniqueScanningHandle<Rewriter> rewriter(
             Rewriter::createRewriter(__builtin_extract_return_addr(__builtin_return_address(0)), 3, "getGlobal"));
 
         Box* r;