xxx wip

src/asm_writing/rewriter.cpp

@@ -270,6 +270,8 @@ void Rewriter::assertArgsInPlace() {
 void RewriterVar::addGuard(uint64_t val) {
     STAT_TIMER(t0, "us_timer_rewriter", 10);
 
+    assert(this->reftype == RefType::UNKNOWN || this->vrefcount > 0);
+
     if (isConstant()) {
         RELEASE_ASSERT(constant_value == val, "added guard which is always false");
         return;
@@ -323,6 +325,8 @@ void Rewriter::_addGuard(RewriterVar* var, RewriterVar* val_constant) {
 void RewriterVar::addGuardNotEq(uint64_t val) {
     STAT_TIMER(t0, "us_timer_rewriter", 10);
 
+    assert(this->reftype == RefType::UNKNOWN || this->vrefcount > 0);
+
     RewriterVar* val_var = rewriter->loadConst(val);
     rewriter->addAction([=]() { rewriter->_addGuardNotEq(this, val_var); }, { this, val_var }, ActionType::GUARD);
 }
@@ -354,6 +358,8 @@ void Rewriter::_addGuardNotEq(RewriterVar* var, RewriterVar* val_constant) {
 void RewriterVar::addAttrGuard(int offset, uint64_t val, bool negate) {
     STAT_TIMER(t0, "us_timer_rewriter", 10);
 
+    assert(this->reftype == RefType::UNKNOWN || this->vrefcount > 0);
+
     if (!attr_guards.insert(std::make_tuple(offset, val, negate)).second)
         return; // duplicate guard detected
 
@@ -405,6 +411,8 @@ void Rewriter::_addAttrGuard(RewriterVar* var, int offset, RewriterVar* val_cons
 RewriterVar* RewriterVar::getAttr(int offset, Location dest, assembler::MovType type) {
     STAT_TIMER(t0, "us_timer_rewriter", 10);
 
+    assert(this->reftype == RefType::UNKNOWN || this->vrefcount > 0);
+
     RewriterVar* result = rewriter->createNewVar();
     rewriter->addAction([=]() { rewriter->_getAttr(result, this, offset, dest, type); }, { this }, ActionType::NORMAL);
     return result;
@@ -434,6 +442,8 @@ void Rewriter::_getAttr(RewriterVar* result, RewriterVar* ptr, int offset, Locat
 RewriterVar* RewriterVar::getAttrDouble(int offset, Location dest) {
     STAT_TIMER(t0, "us_timer_rewriter", 10);
 
+    assert(this->reftype == RefType::UNKNOWN || this->vrefcount > 0);
+
     RewriterVar* result = rewriter->createNewVar();
     rewriter->addAction([=]() { rewriter->_getAttrDouble(result, this, offset, dest); }, { this }, ActionType::NORMAL);
     return result;
@@ -456,6 +466,8 @@ void Rewriter::_getAttrDouble(RewriterVar* result, RewriterVar* ptr, int offset,
 RewriterVar* RewriterVar::getAttrFloat(int offset, Location dest) {
     STAT_TIMER(t0, "us_timer_rewriter", 10);
 
+    assert(this->reftype == RefType::UNKNOWN || this->vrefcount > 0);
+
     RewriterVar* result = rewriter->createNewVar();
     rewriter->addAction([=]() { rewriter->_getAttrFloat(result, this, offset, dest); }, { this }, ActionType::NORMAL);
     return result;
@@ -637,6 +649,9 @@ void Rewriter::_toBool(RewriterVar* result, RewriterVar* var, Location dest) {
 void RewriterVar::setAttr(int offset, RewriterVar* val) {
     STAT_TIMER(t0, "us_timer_rewriter", 10);
 
+    for (auto a : {this, val})
+        assert(a->reftype == RefType::UNKNOWN || a->vrefcount > 0);
+
     rewriter->addAction([=]() { rewriter->_setAttr(this, offset, val); }, { this, val }, ActionType::MUTATION);
 }
 
@@ -673,6 +688,8 @@ void RewriterVar::dump() {
     printf("RewriterVar at %p: %ld locations:\n", this, locations.size());
     for (Location l : locations)
         l.dump();
+    if (is_constant)
+        printf("Constant value: 0x%lx\n", this->constant_value);
 }
 
 assembler::Immediate RewriterVar::tryGetAsImmediate(bool* is_immediate) {
@@ -908,6 +925,9 @@ static const Location caller_save_registers[]{
 
 RewriterVar* Rewriter::call(bool has_side_effects, void* func_addr, const RewriterVar::SmallVector& args,
                             const RewriterVar::SmallVector& args_xmm) {
+    for (auto a : args)
+        assert(a->reftype == RefType::UNKNOWN || a->vrefcount > 0);
+
     RewriterVar* result = createNewVar();
     RewriterVar::SmallVector uses;
     for (RewriterVar* v : args) {
@@ -1255,6 +1275,10 @@ void Rewriter::commit() {
         }
     }
 
+    for (RewriterVar& var : vars) {
+        assert(var.vrefcount == 0);
+    }
+
     assertConsistent();
 
     // Emit assembly for each action, and set done_guarding when
@@ -1481,6 +1505,8 @@ bool Rewriter::finishAssembly(int continue_offset) {
 void Rewriter::commitReturning(RewriterVar* var) {
     STAT_TIMER(t0, "us_timer_rewriter", 10);
 
+    assert(var->vrefcount == 0);
+
     addAction([=]() {
         assembler->comment("commitReturning");
         var->getInReg(getReturnDestination(), true /* allow_constant_in_reg */);

src/asm_writing/rewriter.h

@@ -200,6 +200,12 @@ class Rewriter;
 class RewriterVar;
 class RewriterAction;
 
+enum class RefType {
+    UNKNOWN,
+    OWNED,
+    BORROWED,
+};
+
 // This might make more sense as an inner class of Rewriter, but
 // you can't forward-declare that :/
 class RewriterVar {
@@ -216,9 +222,53 @@ public:
     void setAttr(int offset, RewriterVar* other);
     RewriterVar* cmp(AST_TYPE::AST_TYPE cmp_type, RewriterVar* other, Location loc = Location::any());
     RewriterVar* toBool(Location loc = Location::any());
-    void incref();
-    void decref();
-    void xdecref();
+
+    RewriterVar* setType(RefType type) {
+        assert(this->reftype == RefType::UNKNOWN);
+        assert(type != RefType::UNKNOWN);
+        this->reftype = type;
+        this->vrefcount = 1;
+        return this;
+    }
+
+    RewriterVar* asBorrowed() {
+        if (this->reftype == RefType::UNKNOWN)
+            return setType(RefType::BORROWED);
+
+        assert(this->reftype == RefType::BORROWED);
+        return this->incvref();
+    }
+
+    RewriterVar* incvref() {
+        assert(reftype == RefType::OWNED || reftype == RefType::BORROWED);
+        assert(vrefcount > 0 || (reftype == RefType::BORROWED && vrefcount >= 0));
+        vrefcount++;
+        return this;
+    }
+
+    RewriterVar* decvref() {
+        assert(reftype == RefType::OWNED || reftype == RefType::BORROWED);
+        assert(vrefcount > 0);
+        vrefcount--;
+
+        if (reftype == RefType::OWNED && vrefcount == 0)
+            decref();
+        return this;
+    }
+
+    RewriterVar* materializeVref() {
+        assert(reftype == RefType::OWNED || reftype == RefType::BORROWED);
+        assert(vrefcount > 0);
+        vrefcount--;
+
+        if (reftype == RefType::OWNED && vrefcount == 0) {
+            // handoff, do nothing
+        } else {
+            incref();
+        }
+        return this;
+    }
+
 
     template <typename Src, typename Dst> inline RewriterVar* getAttrCast(int offset, Location loc = Location::any());
 
@@ -230,6 +280,10 @@ private:
     llvm::SmallVector<Location, 4> locations;
     bool isInLocation(Location l);
 
+    void incref();
+    void decref();
+    void xdecref();
+
     // uses is a vector of the indices into the Rewriter::actions vector
     // indicated the actions that use this variable.
     // During the assembly-emitting phase, next_use is used to keep track of the next
@@ -250,6 +304,9 @@ private:
     bool is_arg;
     bool is_constant;
 
+    RefType reftype = RefType::UNKNOWN;
+    int vrefcount;
+
     uint64_t constant_value;
     Location arg_loc;
     std::pair<int /*offset*/, int /*size*/> scratch_allocation;
@@ -275,10 +332,21 @@ private:
     RewriterVar& operator=(const RewriterVar&) = delete;
 
 public:
-    RewriterVar(Rewriter* rewriter) : rewriter(rewriter), next_use(0), is_arg(false), is_constant(false) {
+    RewriterVar(Rewriter* rewriter)
+        : rewriter(rewriter),
+          next_use(0),
+          is_arg(false),
+          is_constant(false),
+          vrefcount(0) {
         assert(rewriter);
     }
 
+    // XXX: for testing, reset these on deallocation so that we will see the next time they get set.
+    ~RewriterVar() {
+        reftype = (RefType)-1;
+        vrefcount = -11;
+    }
+
     Rewriter* getRewriter() { return rewriter; }
 
     friend class Rewriter;

src/codegen/ast_interpreter.cpp

@@ -417,7 +417,7 @@ Box* ASTInterpreter::executeInner(ASTInterpreter& interpreter, CFGBlock* start_b
             if (v.o) {
                 Py_XDECREF(v.o);
                 if (v.var)
-                    v.var->decref();
+                    v.var->decvref();
             }
             v = interpreter.visit_stmt(s);
         }
@@ -467,7 +467,7 @@ void ASTInterpreter::doStore(AST_Name* node, STOLEN(Value) value) {
         setGlobal(globals, name.getBox(), value.o);
         Py_DECREF(value.o);
         if (jit)
-            value.var->decref();
+            value.var->decvref();
     } else if (vst == ScopeInfo::VarScopeType::NAME) {
         assert(0 && "check refcounting");
         if (jit)
@@ -554,8 +554,7 @@ void ASTInterpreter::doStore(AST_expr* node, Value value) {
 Value ASTInterpreter::getNone() {
     RewriterVar* v = NULL;
     if (jit) {
-        v = jit->imm(None);
-        v->incref();
+        v = jit->imm(None)->setType(RefType::BORROWED);
     }
     return Value(incref(None), v);
 }
@@ -575,8 +574,8 @@ Value ASTInterpreter::visit_binop(AST_BinOp* node) {
     Py_DECREF(left.o);
     Py_DECREF(right.o);
     if (jit) {
-        left.var->decref();
-        right.var->decref();
+        left.var->decvref();
+        right.var->decvref();
     }
     return r;
 }
@@ -863,8 +862,8 @@ Value ASTInterpreter::visit_augBinOp(AST_AugBinOp* node) {
     Py_DECREF(right.o);
 
     if (jit) {
-        left.var->decref();
-        right.var->decref();
+        left.var->decvref();
+        right.var->decvref();
     }
     return r;
 }
@@ -877,7 +876,7 @@ Value ASTInterpreter::visit_langPrimitive(AST_LangPrimitive* node) {
         v = Value(getPystonIter(val.o), jit ? jit->emitGetPystonIter(val) : NULL);
         Py_DECREF(val.o);
         if (jit)
-            val.var->decref();
+            val.var->decvref();
     } else if (node->opcode == AST_LangPrimitive::IMPORT_FROM) {
         assert(node->args.size() == 2);
         assert(node->args[0]->type == AST_TYPE::Name);
@@ -938,7 +937,7 @@ Value ASTInterpreter::visit_langPrimitive(AST_LangPrimitive* node) {
         v = Value(boxBool(nonzero(obj.o)), jit ? jit->emitNonzero(obj) : NULL);
         Py_DECREF(obj.o);
         if (jit)
-            obj.var->decref();
+            obj.var->decvref();
     } else if (node->opcode == AST_LangPrimitive::SET_EXC_INFO) {
         assert(node->args.size() == 3);
 
@@ -965,7 +964,7 @@ Value ASTInterpreter::visit_langPrimitive(AST_LangPrimitive* node) {
         v = Value(boxBool(hasnext(obj.o)), jit ? jit->emitHasnext(obj) : NULL);
         Py_DECREF(obj.o);
         if (jit)
-            obj.var->decref();
+            obj.var->decvref();
     } else if (node->opcode == AST_LangPrimitive::PRINT_EXPR) {
         abortJITing();
         Value obj = visit_expr(node->args[0]);
@@ -1144,8 +1143,8 @@ Value ASTInterpreter::visit_makeFunction(AST_MakeFunction* mkfn) {
         if (jit) {
             auto prev_func_var = func.var;
             func.var = jit->emitRuntimeCall(NULL, decorators[i], ArgPassSpec(1), { func }, NULL);
-            decorators[i].var->decref();
-            prev_func_var->decref();
+            decorators[i].var->decvref();
+            prev_func_var->decvref();
         }
     }
     return func;
@@ -1322,8 +1321,8 @@ Value ASTInterpreter::visit_print(AST_Print* node) {
 
     if (jit) {
         if (node->dest)
-            dest.var->decref();
-        var.var->decref();
+            dest.var->decvref();
+        var.var->decvref();
     }
 
     return Value();
@@ -1350,8 +1349,8 @@ Value ASTInterpreter::visit_compare(AST_Compare* node) {
     Py_DECREF(left.o);
     Py_DECREF(right.o);
     if (jit) {
-        left.var->decref();
-        right.var->decref();
+        left.var->decvref();
+        right.var->decvref();
     }
     return r;
 }
@@ -1487,9 +1486,9 @@ Value ASTInterpreter::visit_call(AST_Call* node) {
         Py_DECREF(e);
 
     if (jit) {
-        func.var->decref();
+        func.var->decvref();
         for (auto e : args_vars)
-            e->decref();
+            e->decvref();
     }
 
     return v;
@@ -1515,8 +1514,7 @@ Value ASTInterpreter::visit_num(AST_Num* node) {
     Py_INCREF(o);
     RewriterVar* v = NULL;
     if (jit) {
-        v = jit->imm(o);
-        v->incref();
+        v = jit->imm(o)->setType(RefType::BORROWED);
     }
     return Value(o, v);
 }

src/codegen/baseline_jit.cpp

@@ -168,7 +168,7 @@ RewriterVar* JitFragmentWriter::emitAugbinop(AST_expr* node, RewriterVar* lhs, R
 
 RewriterVar* JitFragmentWriter::emitBinop(AST_expr* node, RewriterVar* lhs, RewriterVar* rhs, int op_type) {
     /// XXX increase this too much for testing
-    return emitPPCall((void*)binop, { lhs, rhs, imm(op_type) }, 2, 640, node);
+    return emitPPCall((void*)binop, { lhs, rhs, imm(op_type) }, 2, 640, node)->setType(RefType::OWNED);
 }
 
 RewriterVar* JitFragmentWriter::emitCallattr(AST_expr* node, RewriterVar* obj, BoxedString* attr, CallattrFlags flags,
@@ -266,8 +266,7 @@ RewriterVar* JitFragmentWriter::emitCreateTuple(const llvm::ArrayRef<RewriterVar
     auto num = values.size();
     RewriterVar* r;
     if (num == 0) {
-        r = imm(EmptyTuple);
-        r->incref();
+        r = imm(EmptyTuple)->setType(RefType::BORROWED);
     } else if (num == 1)
         r = call(false, (void*)BoxedTuple::create1, values[0]);
     else if (num == 2)
@@ -278,7 +277,7 @@ RewriterVar* JitFragmentWriter::emitCreateTuple(const llvm::ArrayRef<RewriterVar
         r = call(false, (void*)createTupleHelper, imm(num), allocArgs(values));
 
     for (auto v : values)
-        v->decref();
+        v->decvref();
 
     return r;
 }
@@ -304,8 +303,7 @@ RewriterVar* JitFragmentWriter::emitGetBlockLocal(InternedString s, int vreg) {
     auto it = local_syms.find(s);
     if (it == local_syms.end())
         return emitGetLocal(s, vreg);
-    // TODO xincref?
-    it->second->incref();
+    it->second->incvref();
     return it->second;
 }
 
@@ -325,11 +323,14 @@ RewriterVar* JitFragmentWriter::emitGetClsAttr(RewriterVar* obj, BoxedString* s)
 
 RewriterVar* JitFragmentWriter::emitGetGlobal(Box* global, BoxedString* s) {
     if (s->s() == "None") {
-        RewriterVar* r = imm(None);
-        r->incref();
+        RewriterVar* r = imm(None)->setType(RefType::BORROWED);
         return r;
     }
-    return emitPPCall((void*)getGlobal, { imm(global), imm(s) }, 2, 512);
+
+    RewriterVar* args[] = { NULL, NULL };
+    args[0] = imm(global)->asBorrowed()->decvref();
+    args[1] = imm(s)->asBorrowed()->decvref();
+    return emitPPCall((void*)getGlobal, args, 2, 512)->setType(RefType::OWNED);
 }
 
 RewriterVar* JitFragmentWriter::emitGetItem(AST_expr* node, RewriterVar* value, RewriterVar* slice) {
@@ -338,7 +339,7 @@ RewriterVar* JitFragmentWriter::emitGetItem(AST_expr* node, RewriterVar* value,
 
 RewriterVar* JitFragmentWriter::emitGetLocal(InternedString s, int vreg) {
     assert(vreg >= 0);
-    RewriterVar* val_var = vregs_array->getAttr(vreg * 8);
+    RewriterVar* val_var = vregs_array->getAttr(vreg * 8)->setType(RefType::OWNED);
     addAction([=]() { _emitGetLocal(val_var, s.c_str()); }, { val_var }, ActionType::NORMAL);
     return val_var;
 }
@@ -405,7 +406,7 @@ RewriterVar* JitFragmentWriter::emitRuntimeCall(AST_expr* node, RewriterVar* obj
     if (keyword_names)
         call_args.push_back(imm(keyword_names));
 
-    return emitPPCall((void*)runtimeCall, call_args, 2, 640, node, type_recorder);
+    return emitPPCall((void*)runtimeCall, call_args, 2, 640, node, type_recorder)->setType(RefType::OWNED);
 #else
     RewriterVar* argspec_var = imm(argspec.asInt());
     RewriterVar* keyword_names_var = keyword_names ? imm(keyword_names) : nullptr;
@@ -476,7 +477,7 @@ void JitFragmentWriter::emitExec(RewriterVar* code, RewriterVar* globals, Rewrit
 void JitFragmentWriter::emitJump(CFGBlock* b) {
     for (auto v : local_syms) {
         if (v.second)
-            v.second->decref(); // xdecref?
+            v.second->decvref(); // xdecref?
     }
 
     RewriterVar* next = imm(b);
@@ -508,9 +509,11 @@ void JitFragmentWriter::emitRaise3(RewriterVar* arg0, RewriterVar* arg1, Rewrite
 void JitFragmentWriter::emitReturn(RewriterVar* v) {
     for (auto v : local_syms) {
         if (v.second)
-            v.second->decref();
+            v.second->decvref();
     }
 
+    v->materializeVref();
+
     addAction([=]() { _emitReturn(v); }, { v }, ActionType::NORMAL);
 }
 
@@ -523,7 +526,7 @@ void JitFragmentWriter::emitSetBlockLocal(InternedString s, RewriterVar* v) {
     local_syms[s] = v;
     if (prev) {
         // TODO: xdecref?
-        prev->decref();
+        prev->decvref();
     }
 }
 
@@ -559,10 +562,10 @@ void JitFragmentWriter::emitSetLocal(InternedString s, int vreg, bool set_closur
 #endif
              v);
     } else {
-        RewriterVar* prev = vregs_array->getAttr(8 * vreg);
+        RewriterVar* prev = vregs_array->getAttr(8 * vreg)->setType(RefType::OWNED);
         vregs_array->setAttr(8 * vreg, v);
         // XXX: this either needs to be an xdecref or we should check liveness analysis.
-        prev->decref();
+        prev->decvref();
         //prev->xdecref();
     }
 }

src/runtime/objmodel.cpp

@@ -874,8 +874,8 @@ template <Rewritable rewritable> Box* Box::getattr(BoxedString* attr, GetattrRew
             } else {
                 RewriterVar* r_attrs
                     = rewrite_args->obj->getAttr(cls->attrs_offset + offsetof(HCAttrs, attr_list), Location::any());
-                RewriterVar* r_rtn
-                    = r_attrs->getAttr(offset * sizeof(Box*) + offsetof(HCAttrs::AttrList, attrs), Location::any());
+                RewriterVar* r_rtn = r_attrs->getAttr(offset * sizeof(Box*) + offsetof(HCAttrs::AttrList, attrs),
+                                                      Location::any())->setType(RefType::BORROWED);
                 rewrite_args->setReturn(r_rtn, ReturnConvention::HAS_RETURN);
             }
         }
@@ -1019,8 +1019,10 @@ void Box::setattr(BoxedString* attr, Box* val, SetattrRewriteArgs* rewrite_args)
                     RewriterVar* r_hattrs
                         = rewrite_args->obj->getAttr(cls->attrs_offset + offsetof(HCAttrs, attr_list), Location::any());
 
-                    r_hattrs->getAttr(offset * sizeof(Box*) + offsetof(HCAttrs::AttrList, attrs))->decref();
-                    rewrite_args->attrval->incref();
+                    r_hattrs->getAttr(offset * sizeof(Box*) + offsetof(HCAttrs::AttrList, attrs))
+                        ->setType(RefType::OWNED)
+                        ->decvref();
+                    rewrite_args->attrval->materializeVref();
                     r_hattrs->setAttr(offset * sizeof(Box*) + offsetof(HCAttrs::AttrList, attrs),
                                       rewrite_args->attrval);
 
@@ -1038,6 +1040,7 @@ void Box::setattr(BoxedString* attr, Box* val, SetattrRewriteArgs* rewrite_args)
             // make sure we don't need to rearrange the attributes
             assert(new_hcls->getStrAttrOffsets().lookup(attr) == hcls->attributeArraySize());
 
+            assert(!rewrite_args && "check rewriter refcounting");
             this->appendNewHCAttr(val, rewrite_args);
             attrs->hcls = new_hcls;
 
@@ -1278,8 +1281,12 @@ template <Rewritable rewritable> Box* typeLookup(BoxedClass* cls, BoxedString* a
             obj_saved->addAttrGuard(offsetof(BoxedClass, tp_version_tag), (intptr_t)cls->tp_version_tag);
             if (!val)
                 rewrite_args->setReturn(NULL, ReturnConvention::NO_RETURN);
-            else
-                rewrite_args->setReturn(rewrite_args->rewriter->loadConst((int64_t)val), ReturnConvention::HAS_RETURN);
+            else {
+                // typeLookup is special since it doesn't even return a vref
+                rewrite_args->setReturn(
+                    rewrite_args->rewriter->loadConst((int64_t)val)->setType(RefType::BORROWED),
+                    ReturnConvention::HAS_RETURN);
+            }
         }
         return val;
     }
@@ -1376,8 +1383,8 @@ Box* nondataDescriptorInstanceSpecialCases(GetattrRewriteArgs* rewrite_args, Box
         } else {
             *bind_obj_out = incref(im_self);
             if (rewrite_args) {
-                r_im_func->incref();
-                r_im_self->incref();
+                r_im_func->incvref();
+                r_im_self->incvref();
                 rewrite_args->setReturn(r_im_func, ReturnConvention::HAS_RETURN);
                 *r_bind_obj_out = r_im_self;
             }
@@ -2585,6 +2592,10 @@ extern "C" void setattr(Box* obj, BoxedString* attr, Box* attr_val) {
     assert(!obj->cls->tp_setattr);
 
     if (rewriter.get()) {
+        rewriter->getArg(0)->setType(RefType::BORROWED);
+        rewriter->getArg(1)->setType(RefType::BORROWED);
+        rewriter->getArg(2)->setType(RefType::BORROWED);
+
         auto r_cls = rewriter->getArg(0)->getAttr(offsetof(Box, cls));
         // rewriter->trap();
         r_cls->addAttrGuard(offsetof(BoxedClass, tp_setattr), 0);
@@ -3218,7 +3229,7 @@ Box* callattrInternal(Box* obj, BoxedString* attr, LookupScope scope, CallattrRe
         auto r_rtn = rewrite_args->rewriter->call(true, (void*)Helper::call, arg_vec);
         rewrite_args->setReturn(r_rtn, S == CXX ? ReturnConvention::HAS_RETURN : ReturnConvention::CAPI_RETURN);
         if (r_bind_obj)
-            r_bind_obj->decref();
+            r_bind_obj->decvref();
 
         void* _args[2] = { args, const_cast<std::vector<BoxedString*>*>(keyword_names) };
         Box* rtn = Helper::call(val, argspec, arg1, arg2, arg3, _args);
@@ -3240,8 +3251,8 @@ Box* callattrInternal(Box* obj, BoxedString* attr, LookupScope scope, CallattrRe
     Py_DECREF(val);
     if (rewrite_args) {
         if (r_bind_obj)
-            r_bind_obj->decref();
-        r_val->decref();
+            r_bind_obj->decvref();
+        r_val->decvref();
     }
     return r;
 }
@@ -3553,19 +3564,6 @@ void rearrangeArgumentsInternal(ParamReceiveSpec paramspec, const ParamNames* pa
                 Py_INCREF(oargs[i]);
             }
         }
-        if (rewrite_args) {
-            if (num_output_args >= 1)
-                rewrite_args->arg1->incref();
-            if (num_output_args >= 2)
-                rewrite_args->arg2->incref();
-            if (num_output_args >= 3)
-                rewrite_args->arg3->incref();
-            if (num_output_args >= 3) {
-                for (int i = 0; i < num_output_args - 3; i++) {
-                    rewrite_args->args->getAttr(i * sizeof(Box*))->incref();
-                }
-            }
-        }
         return;
     }
 
@@ -4132,8 +4130,9 @@ Box* callFunc(BoxedFunctionBase* func, CallRewriteArgs* rewrite_args, ArgPassSpe
         Py_DECREF(oargs[i]);
     }
     if (rewrite_args) {
+        RELEASE_ASSERT(num_output_args <= 3, "figure out vrefs for arg array");
         for (int i = 0; i < num_output_args; i++) {
-            getArg(i, rewrite_args)->decref();
+            getArg(i, rewrite_args)->decvref();
         }
     }
 
@@ -4323,7 +4322,7 @@ Box* callCLFunc(FunctionMetadata* md, CallRewriteArgs* rewrite_args, int num_out
         if (num_output_args >= 4)
             arg_vec.push_back(rewrite_args->args);
 
-        rewrite_args->out_rtn = rewrite_args->rewriter->call(true, func_ptr, arg_vec);
+        rewrite_args->out_rtn = rewrite_args->rewriter->call(true, func_ptr, arg_vec)->setType(RefType::OWNED);
         if (S == CXX && chosen_cf->exception_style == CAPI)
             rewrite_args->rewriter->checkAndThrowCAPIException(rewrite_args->out_rtn);
 
@@ -4730,6 +4729,8 @@ Box* binopInternal(Box* lhs, Box* rhs, int op_type, bool inplace, BinopRewriteAr
     if (rewrite_args) {
         CallattrRewriteArgs srewrite_args(rewrite_args->rewriter, rewrite_args->lhs, rewrite_args->destination);
         srewrite_args.arg1 = rewrite_args->rhs;
+        rewrite_args->lhs->incvref();
+        rewrite_args->rhs->incvref();
         lrtn = callattrInternal1<CXX, REWRITABLE>(lhs, op_name, CLASS_ONLY, &srewrite_args, ArgPassSpec(1), rhs);
 
         if (!srewrite_args.isSuccessful()) {
@@ -4753,6 +4754,8 @@ Box* binopInternal(Box* lhs, Box* rhs, int op_type, bool inplace, BinopRewriteAr
     if (lrtn) {
         if (lrtn != NotImplemented) {
             if (rewrite_args) {
+                rewrite_args->lhs->decvref();
+                rewrite_args->rhs->decvref();
                 assert(rewrite_args->out_rtn);
                 rewrite_args->out_success = true;
             }
@@ -4835,14 +4838,17 @@ extern "C" Box* binop(Box* lhs, Box* rhs, int op_type) {
     Box* rtn;
     if (rewriter.get()) {
         // rewriter->trap();
-        BinopRewriteArgs rewrite_args(rewriter.get(), rewriter->getArg(0), rewriter->getArg(1),
+        BinopRewriteArgs rewrite_args(rewriter.get(), rewriter->getArg(0)->setType(RefType::BORROWED),
+                                      rewriter->getArg(1)->setType(RefType::BORROWED),
                                       rewriter->getReturnDestination());
         rtn = binopInternal<REWRITABLE>(lhs, rhs, op_type, false, &rewrite_args);
         assert(rtn);
         if (!rewrite_args.out_success) {
             rewriter.reset(NULL);
-        } else
+        } else {
+            rewrite_args.out_rtn->materializeVref();
             rewriter->commitReturning(rewrite_args.out_rtn);
+        }
     } else {
         rtn = binopInternal<NOT_REWRITABLE>(lhs, rhs, op_type, false, NULL);
     }
@@ -6387,7 +6393,7 @@ extern "C" Box* getGlobal(Box* globals, BoxedString* name) {
                 if (r) {
                     if (rewriter.get()) {
                         RewriterVar* r_rtn = rewrite_args.getReturn(ReturnConvention::HAS_RETURN);
-                        r_rtn->incref();
+                        r_rtn->materializeVref();
                         rewriter->commitReturning(r_rtn);
                     }