Fix refcounting for a bunch of locals/globals functions

Most of these need to return borrowed references in order to match the C API.

src/capi/modsupport.cpp

@@ -453,7 +453,7 @@ extern "C" BORROWED(PyObject*) PyModule_GetDict(PyObject* _m) noexcept {
     BoxedModule* m = static_cast<BoxedModule*>(_m);
     assert(PyModule_Check(m));
 
-    return autoDecref(m->getAttrWrapper());
+    return m->getAttrWrapper();
 }
 
 extern "C" int PyModule_AddObject(PyObject* _m, const char* name, STOLEN(PyObject*) value) noexcept {

src/capi/typeobject.cpp

@@ -3466,7 +3466,7 @@ extern "C" int PyType_Ready(PyTypeObject* cls) noexcept {
     cls->giveAttrBorrowed("__base__", base);
 
     assert(cls->tp_dict == NULL);
-    cls->tp_dict = cls->getAttrWrapper();
+    cls->tp_dict = incref(cls->getAttrWrapper());
 
     assert(cls->tp_name);
 

src/codegen/irgen/hooks.cpp

@@ -469,6 +469,7 @@ extern "C" int PyEval_MergeCompilerFlags(PyCompilerFlags* cf) noexcept {
 }
 
 static void pickGlobalsAndLocals(Box*& globals, Box*& locals) {
+    assert(0 && "check refcounting");
     if (globals == None)
         globals = NULL;
 

src/codegen/unwinding.cpp

@@ -353,19 +353,12 @@ public:
         return getFrameInfo()->stmt;
     }
 
-    Box* getGlobals() {
-        assert(0 && "check refcounting (of callers)");
-        Box* r = getFrameInfo()->globals;
-        return incref(r);
-    }
-
-    Box* getGlobalsDict() {
-        Box* globals = getGlobals();
+    BORROWED(Box*) getGlobalsDict() {
+        Box* globals = getFrameInfo()->globals;
         if (!globals)
             return NULL;
 
         if (PyModule_Check(globals)) {
-            AUTO_DECREF(globals);
             return globals->getAttrWrapper();
         }
         return globals;
@@ -729,14 +722,14 @@ FunctionMetadata* getTopPythonFunction() {
     return frame_info->md;
 }
 
-Box* getGlobals() {
+BORROWED(Box*) getGlobals() {
     FrameInfo* frame_info = getTopFrameInfo();
     if (!frame_info)
         return NULL;
     return frame_info->globals;
 }
 
-Box* getGlobalsDict() {
+BORROWED(Box*) getGlobalsDict() {
     Box* globals = getGlobals();
     if (globals && PyModule_Check(globals))
         globals = globals->getAttrWrapper();
@@ -874,7 +867,7 @@ DeoptState getDeoptState() {
     return rtn;
 }
 
-Box* fastLocalsToBoxedLocals() {
+BORROWED(Box*) fastLocalsToBoxedLocals() {
     return getPythonFrameInfo(0)->updateBoxedLocals();
 }
 
@@ -891,7 +884,7 @@ static BoxedDict* localsForFrame(Box** vregs, CFG* cfg) {
     return rtn;
 }
 
-Box* FrameInfo::updateBoxedLocals() {
+BORROWED(Box*) FrameInfo::updateBoxedLocals() {
     STAT_TIMER(t0, "us_timer_updateBoxedLocals", 0);
 
     FrameInfo* frame_info = this;
@@ -922,15 +915,17 @@ Box* FrameInfo::updateBoxedLocals() {
         Box* val = closure->elts[derefInfo.offset];
         Box* boxedName = name.getBox();
         if (val != NULL) {
+            assert(0 &&" add refcounting");
             d->d[boxedName] = val;
         } else {
+            assert(0 &&" add refcounting");
             d->d.erase(boxedName);
         }
     }
 
     if (!frame_info->boxedLocals) {
-        frame_info->boxedLocals = incref(d);
-        return d;
+        frame_info->boxedLocals = d;
+        return frame_info->boxedLocals;
     }
 
     AUTO_DECREF(d);
@@ -949,7 +944,7 @@ Box* FrameInfo::updateBoxedLocals() {
         }
     }
 
-    return incref(frame_info->boxedLocals);
+    return frame_info->boxedLocals;
 }
 
 AST_stmt* PythonFrameIterator::getCurrentStatement() {
@@ -964,7 +959,7 @@ FunctionMetadata* PythonFrameIterator::getMD() {
     return impl->getMD();
 }
 
-Box* PythonFrameIterator::getGlobalsDict() {
+BORROWED(Box*) PythonFrameIterator::getGlobalsDict() {
     return impl->getGlobalsDict();
 }
 

src/codegen/unwinding.h

@@ -36,8 +36,8 @@ bool isUnwinding(); // use this instead of std::uncaught_exception
 
 void setupUnwinding();
 BoxedModule* getCurrentModule();
-Box* getGlobals();     // returns either the module or a globals dict
-Box* getGlobalsDict(); // always returns a dict-like object
+BORROWED(Box*) getGlobals();     // returns either the module or a globals dict
+BORROWED(Box*) getGlobalsDict(); // always returns a dict-like object
 CompiledFunction* getCFForAddress(uint64_t addr);
 
 class PythonUnwindSession;
@@ -66,7 +66,7 @@ std::string getCurrentPythonLine();
 void logByCurrentPythonLine(const std::string& stat_name);
 
 // Adds stack locals and closure locals into the locals dict, and returns it.
-Box* fastLocalsToBoxedLocals();
+BORROWED(Box*) fastLocalsToBoxedLocals();
 
 class PythonFrameIteratorImpl;
 class PythonFrameIterator {
@@ -79,7 +79,7 @@ public:
     FrameInfo* getFrameInfo();
     bool exists() { return impl.get() != NULL; }
     AST_stmt* getCurrentStatement();
-    Box* getGlobalsDict();
+    BORROWED(Box*) getGlobalsDict();
 
     PythonFrameIterator(PythonFrameIterator&& rhs);
     void operator=(PythonFrameIterator&& rhs);

src/core/types.h

@@ -722,8 +722,7 @@ public:
     void delattr(BoxedString* attr, DelattrRewriteArgs* rewrite_args);
 
     // Only valid for hc-backed instances:
-    // Maybe this should return a borrowed reference?
-    Box* getAttrWrapper();
+    BORROWED(Box*) getAttrWrapper();
 
     Box* reprIC();
     BoxedString* reprICAsString();
@@ -982,7 +981,7 @@ struct FrameInfo {
     FrameInfo* back;
     FunctionMetadata* md;
 
-    Box* updateBoxedLocals();
+    BORROWED(Box*) updateBoxedLocals();
 
     FrameInfo(ExcInfo exc)
         : exc(exc),

src/runtime/builtin_modules/builtins.cpp

@@ -83,7 +83,7 @@ extern "C" Box* dir(Box* obj) {
 
 extern "C" Box* vars(Box* obj) {
     if (!obj)
-        return fastLocalsToBoxedLocals();
+        return incref(PyEval_GetLocals());
 
     static BoxedString* dict_str = getStaticString("__dict__");
     Box* rtn = getattrInternal<ExceptionStyle::CAPI>(obj, dict_str);
@@ -1251,7 +1251,7 @@ public:
     static Box* __reduce__(Box* self) {
         RELEASE_ASSERT(isSubclass(self->cls, BaseException), "");
         BoxedException* exc = static_cast<BoxedException*>(self);
-        return BoxedTuple::create({ self->cls, EmptyTuple, autoDecref(self->getAttrWrapper()) });
+        return BoxedTuple::create({ self->cls, EmptyTuple, self->getAttrWrapper() });
     }
 };
 
@@ -1388,19 +1388,17 @@ public:
     }
 };
 
-Box* globals() {
-    // TODO is it ok that we don't return a real dict here?
-    return getGlobalsDict();
+static Box* globals() {
+    return incref(getGlobalsDict());
 }
 
-Box* locals() {
-    return fastLocalsToBoxedLocals();
+static Box* locals() {
+    return incref(fastLocalsToBoxedLocals());
 }
 
 extern "C" BORROWED(PyObject*) PyEval_GetLocals(void) noexcept {
     try {
-        assert(0 && "check refcounting");
-        return locals();
+        return fastLocalsToBoxedLocals();
     } catch (ExcInfo e) {
         setCAPIException(e);
         return NULL;
@@ -1409,7 +1407,7 @@ extern "C" BORROWED(PyObject*) PyEval_GetLocals(void) noexcept {
 
 extern "C" BORROWED(PyObject*) PyEval_GetGlobals(void) noexcept {
     try {
-        return autoXDecref(globals());
+        return getGlobalsDict();
     } catch (ExcInfo e) {
         setCAPIException(e);
         return NULL;
@@ -1681,7 +1679,7 @@ Box* input(Box* prompt) {
         str++;
 
     Box* gbls = globals();
-    Box* lcls = locals();
+    Box* lcls = PyEval_GetLocals();
 
     // CPython has these safety checks that the builtin functions exist
     // in the current global scope.

src/runtime/classobj.cpp

@@ -223,7 +223,7 @@ static Box* classobjGetattribute(Box* _cls, Box* _attr) {
     // These are special cases in CPython as well:
     if (attr->data()[0] == '_' && attr->data()[1] == '_') {
         if (attr->s() == "__dict__")
-            return cls->getAttrWrapper();
+            return incref(cls->getAttrWrapper());
 
         if (attr->s() == "__bases__")
             return incref(cls->bases);
@@ -461,7 +461,7 @@ static Box* _instanceGetattribute(Box* _inst, BoxedString* attr_str, bool raise_
     // These are special cases in CPython as well:
     if (attr_str->data()[0] == '_' && attr_str->data()[1] == '_') {
         if (attr_str->s() == "__dict__")
-            return inst->getAttrWrapper();
+            return incref(inst->getAttrWrapper());
 
         if (attr_str->s() == "__class__")
             return incref(inst->inst_cls);

src/runtime/frame.cpp

@@ -94,7 +94,7 @@ public:
         if (f->hasExited())
             return incref(f->_locals);
 
-        return f->frame_info->updateBoxedLocals();
+        return incref(f->frame_info->updateBoxedLocals());
     }
 
     static Box* globals(Box* obj, void*) {
@@ -103,7 +103,7 @@ public:
         if (!f->_globals) {
             Box* globals = f->frame_info->globals;
             if (globals && PyModule_Check(globals))
-                f->_globals = globals->getAttrWrapper();
+                f->_globals = incref(globals->getAttrWrapper());
             else {
                 f->_globals = incref(globals);
             }

src/runtime/objmodel.cpp

@@ -768,7 +768,7 @@ BoxedClass* BoxedClass::create(BoxedClass* metaclass, BoxedClass* base, int attr
 
 void BoxedClass::finishInitialization() {
     assert(!this->tp_dict);
-    this->tp_dict = this->getAttrWrapper();
+    this->tp_dict = incref(this->getAttrWrapper());
 
     commonClassSetup(this);
     tp_flags |= Py_TPFLAGS_READY;

src/runtime/types.cpp

@@ -1333,13 +1333,13 @@ Box* typeCall(Box* obj, BoxedTuple* vararg, BoxedDict* kwargs) {
 
 static Box* typeDict(Box* obj, void* context) {
     if (obj->cls->instancesHaveHCAttrs())
-        return PyDictProxy_New(autoDecref(obj->getAttrWrapper()));
+        return PyDictProxy_New(obj->getAttrWrapper());
     abort();
 }
 
 static Box* typeSubDict(Box* obj, void* context) {
     if (obj->cls->instancesHaveHCAttrs())
-        return obj->getAttrWrapper();
+        return incref(obj->getAttrWrapper());
     if (obj->cls->instancesHaveDictAttrs())
         return obj->getDict();
     abort();
@@ -2739,7 +2739,7 @@ void Box::giveAttrDescriptor(const char* attr, Box* (*get)(Box*, void*), void (*
     giveAttr(bstr, new (pyston_getset_cls) BoxedGetsetDescriptor(bstr, get, set, NULL));
 }
 
-Box* Box::getAttrWrapper() {
+BORROWED(Box*) Box::getAttrWrapper() {
     assert(cls->instancesHaveHCAttrs());
     HCAttrs* attrs = getHCAttrsPtr();
     HiddenClass* hcls = attrs->hcls;
@@ -2748,7 +2748,7 @@ Box* Box::getAttrWrapper() {
         hcls = root_hcls;
 
     if (hcls->type == HiddenClass::DICT_BACKED) {
-        return incref(attrs->attr_list->attrs[0]);
+        return attrs->attr_list->attrs[0];
     }
 
     int offset = hcls->getAttrwrapperOffset();
@@ -2758,15 +2758,17 @@ Box* Box::getAttrWrapper() {
             auto new_hcls = hcls->getAttrwrapperChild();
             appendNewHCAttr(aw, NULL);
             attrs->hcls = new_hcls;
+            Py_DECREF(aw);
             return aw;
         } else {
             assert(hcls->type == HiddenClass::SINGLETON);
             appendNewHCAttr(aw, NULL);
             hcls->appendAttrwrapper();
+            Py_DECREF(aw);
             return aw;
         }
     }
-    return incref(attrs->attr_list->attrs[offset]);
+    return attrs->attr_list->attrs[offset];
 }
 
 extern "C" PyObject* PyObject_GetAttrWrapper(PyObject* obj) noexcept {