diff --git a/Include/internal/pycore_cell.h b/Include/internal/pycore_cell.h index 3493eb91514a13..223d836938246d 100644 --- a/Include/internal/pycore_cell.h +++ b/Include/internal/pycore_cell.h @@ -21,15 +21,32 @@ PyCell_SwapTakeRef(PyCellObject *cell, PyObject *value, int* result) PyObject *old_value = NULL; *result = 0; Py_BEGIN_CRITICAL_SECTION(cell); - if(Py_CHECKWRITE(cell)){ + do { + if(!Py_CHECKWRITE(cell)){ + PyRegion_CLEARLOCAL(value); + *result = -1; + break; + } + + // Simple and fast clear + if (PyRegion_IsLocal(cell)) { + old_value = cell->ob_ref; + FT_ATOMIC_STORE_PTR_RELEASE(cell->ob_ref, value); + break; + } + old_value = cell->ob_ref; + if (PyRegion_AddLocalRef(old_value)) { + assert(false && "This should never fail, since we have a ref to cell"); + } + if (PyRegion_TakeRef(cell, value)) { + PyRegion_CLEARLOCAL(value); + *result = -1; + break; + } FT_ATOMIC_STORE_PTR_RELEASE(cell->ob_ref, value); - } - else { - *result = -1; - PyRegion_RemoveLocalRef(value); - Py_XDECREF(value); - } + PyRegion_RemoveRef(cell, old_value); + } while (false); Py_END_CRITICAL_SECTION(); return old_value; } diff --git a/Include/internal/pycore_opcode_metadata.h b/Include/internal/pycore_opcode_metadata.h index 124e96b5ab1e53..202d9575b3ad0b 100644 --- a/Include/internal/pycore_opcode_metadata.h +++ b/Include/internal/pycore_opcode_metadata.h @@ -1258,10 +1258,10 @@ const struct opcode_metadata _PyOpcode_opcode_metadata[267] = { [SEND_GEN] = { true, INSTR_FMT_IBC, HAS_ARG_FLAG | HAS_DEOPT_FLAG }, [SETUP_ANNOTATIONS] = { true, INSTR_FMT_IX, HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [SET_ADD] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, - [SET_FUNCTION_ATTRIBUTE] = { true, INSTR_FMT_IB, HAS_ARG_FLAG }, + [SET_FUNCTION_ATTRIBUTE] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG }, [SET_UPDATE] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [STORE_ATTR] = { true, INSTR_FMT_IBC000, HAS_ARG_FLAG | HAS_NAME_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, - [STORE_ATTR_INSTANCE_VALUE] = { true, INSTR_FMT_IBC000, HAS_ARG_FLAG | HAS_EXIT_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, + [STORE_ATTR_INSTANCE_VALUE] = { true, INSTR_FMT_IBC000, HAS_ARG_FLAG | HAS_DEOPT_FLAG | HAS_EXIT_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [STORE_ATTR_SLOT] = { true, INSTR_FMT_IBC000, HAS_ARG_FLAG | HAS_DEOPT_FLAG | HAS_EXIT_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [STORE_ATTR_WITH_HINT] = { true, INSTR_FMT_IBC000, HAS_ARG_FLAG | HAS_NAME_FLAG | HAS_DEOPT_FLAG | HAS_EXIT_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, [STORE_DEREF] = { true, INSTR_FMT_IB, HAS_ARG_FLAG | HAS_FREE_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG }, diff --git a/Include/internal/pycore_uop_metadata.h b/Include/internal/pycore_uop_metadata.h index f64291eb37a34c..4d66b9dda7920a 100644 --- a/Include/internal/pycore_uop_metadata.h +++ b/Include/internal/pycore_uop_metadata.h @@ -189,7 +189,7 @@ const uint16_t _PyUop_Flags[MAX_UOP_ID+1] = { [_LOAD_ATTR_CLASS] = HAS_ESCAPES_FLAG, [_LOAD_ATTR_PROPERTY_FRAME] = HAS_ARG_FLAG | HAS_DEOPT_FLAG, [_GUARD_DORV_NO_DICT] = HAS_EXIT_FLAG, - [_STORE_ATTR_INSTANCE_VALUE] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, + [_STORE_ATTR_INSTANCE_VALUE] = HAS_ARG_FLAG | HAS_DEOPT_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_STORE_ATTR_WITH_HINT] = HAS_ARG_FLAG | HAS_NAME_FLAG | HAS_DEOPT_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_STORE_ATTR_SLOT] = HAS_ARG_FLAG | HAS_DEOPT_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_COMPARE_OP] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, @@ -292,7 +292,7 @@ const uint16_t _PyUop_Flags[MAX_UOP_ID+1] = { [_CALL_KW_NON_PY] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_MAKE_CALLARGS_A_TUPLE] = HAS_ERROR_FLAG | HAS_ERROR_NO_POP_FLAG | HAS_ESCAPES_FLAG, [_MAKE_FUNCTION] = HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, - [_SET_FUNCTION_ATTRIBUTE] = HAS_ARG_FLAG, + [_SET_FUNCTION_ATTRIBUTE] = HAS_ARG_FLAG | HAS_ERROR_FLAG, [_RETURN_GENERATOR] = HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_BUILD_SLICE] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, [_CONVERT_VALUE] = HAS_ARG_FLAG | HAS_ERROR_FLAG | HAS_ESCAPES_FLAG, diff --git a/Lib/test/test_regions/test_regressions.py b/Lib/test/test_regions/test_regressions.py new file mode 100644 index 00000000000000..bfc3461721ac80 --- /dev/null +++ b/Lib/test/test_regions/test_regressions.py @@ -0,0 +1,31 @@ +import unittest +from regions import Region +from immutable import freeze + +class TestCrashesObject(unittest.TestCase): + + def ignore_test_exception_and_crash_on_freeze(self): + r = Region() + class A: + def foo(self): + r + freeze(A) + + def test_barrier_in_optimized_opcode(self): + class A: pass + + def build_region(): + r = Region() + r.a = A() + r.a.b = A() + r.a = None + return r + + r1 = build_region() + # This second call will optimize the function to use + # `_STORE_ATTR_INSTANCE_VALUE` bytecode that can't + # handle region barriers correctly. We therefore need + # to de-opt again if the objects are in separate regions + # like in the example above. + r2 = build_region() + r3 = build_region() diff --git a/Objects/abstract.c b/Objects/abstract.c index ca3d013593388e..a148daa3146951 100644 --- a/Objects/abstract.c +++ b/Objects/abstract.c @@ -166,6 +166,7 @@ PyObject_GetItem(PyObject *o, PyObject *key) PyMappingMethods *m = Py_TYPE(o)->tp_as_mapping; if (m && m->mp_subscript) { + // TODO(regions): PyRegion_NotifyTypeUse(Py_TYPE(o)); PyObject *item = m->mp_subscript(o, key); assert(_Py_CheckSlotResult(o, "__getitem__", item != NULL)); return item; @@ -199,10 +200,10 @@ PyObject_GetItem(PyObject *o, PyObject *key) } if (meth && meth != Py_None) { result = PyObject_CallOneArg(meth, key); - Py_DECREF(meth); + PyRegion_CLEARLOCAL(meth); return result; } - Py_XDECREF(meth); + PyRegion_CLEARLOCAL(meth); PyErr_Format(PyExc_TypeError, "type '%.200s' is not subscriptable", ((PyTypeObject *)o)->tp_name); return NULL; @@ -2906,12 +2907,13 @@ PyObject_GetIter(PyObject *o) return type_error("'%.200s' object is not iterable", o); } else { + // FIXME(regions): xFrednet: PyRegion_NotifyTypeUse(t); PyObject *res = (*f)(o); if (res != NULL && !PyIter_Check(res)) { PyErr_Format(PyExc_TypeError, "%T.__iter__() must return an iterator, not %T", o, res); - Py_SETREF(res, NULL); + PyRegion_CLEARLOCAL(res); } return res; } @@ -2957,11 +2959,8 @@ static int iternext(PyObject *iter, PyObject **item) { iternextfunc tp_iternext = Py_TYPE(iter)->tp_iternext; - // Check if the type is Pyrona aware, otherwise, mark all open - // regions as dirty - // FIXME(regions): Enable this check, which currently almost always triggers - // PyRegion_NotifyTypeUse(Py_TYPE(iter)); + // FIXME(regions): PyRegion_NotifyTypeUse(Py_TYPE(iter)); if ((*item = tp_iternext(iter))) { return 1; } diff --git a/Objects/object.c b/Objects/object.c index d441e0a5cae541..96159a38229e9d 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -1595,7 +1595,7 @@ _PyObject_GetDictPtr(PyObject *obj) PyObject * PyObject_SelfIter(PyObject *obj) { - return Py_NewRef(obj); + return PyRegion_NewRef(obj); } /* Helper used when the __next__ method is removed from a type: diff --git a/Objects/sliceobject.c b/Objects/sliceobject.c index 5389d20ec3567d..ee2ab9efc15b43 100644 --- a/Objects/sliceobject.c +++ b/Objects/sliceobject.c @@ -128,15 +128,24 @@ _PyBuildSlice_Consume2(PyObject *start, PyObject *stop, PyObject *step) } } + // This LRC/RC bump is needed to use one `TakeRefs` for + // all three objects. + if (PyRegion_NewRef(step) == NULL) { + goto error; + } + if (PyRegion_TakeRefs(obj, start, stop, step)) { + PyRegion_CLEARLOCAL(step); + goto error; + } obj->start = start; obj->stop = stop; - obj->step = Py_NewRef(step); + obj->step = step; _PyObject_GC_TRACK(obj); return obj; error: - Py_DECREF(start); - Py_DECREF(stop); + PyRegion_CLEARLOCAL(start); + PyRegion_CLEARLOCAL(stop); return NULL; } @@ -152,6 +161,9 @@ PySlice_New(PyObject *start, PyObject *stop, PyObject *step) if (stop == NULL) { stop = Py_None; } + if (PyRegion_AddLocalRefs(start, stop)) { + return NULL; + } return (PyObject *)_PyBuildSlice_Consume2(Py_NewRef(start), Py_NewRef(stop), step); } diff --git a/Python/bytecodes.c b/Python/bytecodes.c index c4f815c554ffdf..b074f30a0debe4 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -831,6 +831,8 @@ dummy_func( assert(Py_REFCNT(left_o) >= 2 || !PyStackRef_IsHeapSafe(left)); PyStackRef_CLOSE_SPECIALIZED(left, _PyUnicode_ExactDealloc); DEAD(left); + // REGIONS: No write barrier needed, since these are all unicode + // objects and therefore immutable when observed PyObject *temp = PyStackRef_AsPyObjectSteal(*target_local); PyObject *right_o = PyStackRef_AsPyObjectSteal(right); PyUnicode_Append(&temp, right_o); @@ -882,6 +884,7 @@ dummy_func( } op(_BINARY_SLICE, (container, start, stop -- res)) { + // REGIONS: Write barrier called inside `_PyBuildSlice_ConsumeRefs` PyObject *slice = _PyBuildSlice_ConsumeRefs(PyStackRef_AsPyObjectSteal(start), PyStackRef_AsPyObjectSteal(stop)); PyObject *res_o; @@ -892,6 +895,8 @@ dummy_func( } else { res_o = PyObject_GetItem(PyStackRef_AsPyObjectBorrow(container), slice); + // REGIONS: No barrier needed, since slice is local + assert(PyRegion_IsLocal(slice)); Py_DECREF(slice); } PyStackRef_CLOSE(container); @@ -909,6 +914,7 @@ dummy_func( } op(_STORE_SLICE, (v, container, start, stop -- )) { + // Regions: Write barrier called inside the function PyObject *slice = _PyBuildSlice_ConsumeRefs(PyStackRef_AsPyObjectSteal(start), PyStackRef_AsPyObjectSteal(stop)); int err; @@ -917,6 +923,7 @@ dummy_func( } else { err = PyObject_SetItem(PyStackRef_AsPyObjectBorrow(container), slice, PyStackRef_AsPyObjectBorrow(v)); + assert(PyRegion_IsLocal(slice)); Py_DECREF(slice); } DECREF_INPUTS(); @@ -1088,12 +1095,14 @@ dummy_func( _PUSH_FRAME; inst(LIST_APPEND, (list, unused[oparg-1], v -- list, unused[oparg-1])) { + // REGIONS: The write barrier is called in the function int err = _PyList_AppendTakeRef((PyListObject *)PyStackRef_AsPyObjectBorrow(list), PyStackRef_AsPyObjectSteal(v)); ERROR_IF(err < 0); } inst(SET_ADD, (set, unused[oparg-1], v -- set, unused[oparg-1])) { + // REGIONS: The write barrier is called in the function int err = _PySet_AddTakeRef((PySetObject *)PyStackRef_AsPyObjectBorrow(set), PyStackRef_AsPyObjectSteal(v)); ERROR_IF(err); @@ -1138,6 +1147,7 @@ dummy_func( // Ensure nonnegative, zero-or-one-digit ints. DEOPT_IF(!_PyLong_IsNonNegativeCompact((PyLongObject *)sub)); Py_ssize_t index = ((PyLongObject*)sub)->long_value.ob_digit[0]; + DEOPT_IF(!PyRegion_IsLocal(list)); DEOPT_IF(!LOCK_OBJECT(list)); // Ensure index < len(list) if (index >= PyList_GET_SIZE(list)) { @@ -1154,6 +1164,7 @@ dummy_func( PyStackRef_CLOSE_SPECIALIZED(sub_st, _PyLong_ExactDealloc); DEAD(sub_st); PyStackRef_CLOSE(list_st); + PyRegion_RemoveLocalRef(old_value); Py_DECREF(old_value); } @@ -1165,6 +1176,7 @@ dummy_func( assert(PyDict_CheckExact(dict)); STAT_INC(STORE_SUBSCR, hit); + // REGIONS: The write barrier is called inside the function int err = _PyDict_SetItem_Take2((PyDictObject *)dict, PyStackRef_AsPyObjectSteal(sub), PyStackRef_AsPyObjectSteal(value)); @@ -1460,6 +1472,7 @@ dummy_func( inst(POP_EXCEPT, (exc_value -- )) { _PyErr_StackItem *exc_info = tstate->exc_info; + // Regions: `exc_info->exc_value` is local Py_XSETREF(exc_info->exc_value, PyStackRef_IsNone(exc_value) ? NULL : PyStackRef_AsPyObjectSteal(exc_value)); @@ -1467,6 +1480,7 @@ dummy_func( tier1 inst(RERAISE, (values[oparg], exc_st -- values[oparg])) { PyObject *exc = PyStackRef_AsPyObjectSteal(exc_st); + assert(PyRegion_IsLocal(exc)); assert(oparg >= 0 && oparg <= 2); if (oparg) { @@ -1605,8 +1619,10 @@ dummy_func( } op(_UNPACK_SEQUENCE, (seq -- unused[oparg], top[0])) { + // Regions: seq_o is local, the LRC is increased by the steal PyObject *seq_o = PyStackRef_AsPyObjectSteal(seq); int res = _PyEval_UnpackIterableStackRef(tstate, seq_o, oparg, -1, top); + PyRegion_RemoveLocalRef(seq_o); Py_DECREF(seq_o); ERROR_IF(res == 0); } @@ -1663,8 +1679,10 @@ dummy_func( } inst(UNPACK_EX, (seq -- unused[oparg & 0xFF], unused, unused[oparg >> 8], top[0])) { + // Regions: seq_o is local, the LRC is increased by the steal PyObject *seq_o = PyStackRef_AsPyObjectSteal(seq); int res = _PyEval_UnpackIterableStackRef(tstate, seq_o, oparg & 0xFF, oparg >> 8, top); + PyRegion_AddLocalRef(seq_o); Py_DECREF(seq_o); ERROR_IF(res == 0); } @@ -2640,16 +2658,29 @@ dummy_func( ERROR_IF(true); } assert(_PyObject_GetManagedDict(owner_o) == NULL); + PyObject *value_o = PyStackRef_AsPyObjectBorrow(value); + DEOPT_IF(!PyRegion_IsLocal(owner_o) && !PyRegion_SameRegion(owner_o, value_o)); + // This call can't escape, due to the DEOPT_IF guard above. + // As is, it should only update the LRC and or be a noop. In + // the worst case, it should only may mark a region as dirty. + if (PyRegion_AddRef(owner_o, value_o)) { + UNLOCK_OBJECT(owner_o); + DECREF_INPUTS(); + ERROR_IF(true); + } PyObject **value_ptr = (PyObject**)(((char *)owner_o) + offset); PyObject *old_value = *value_ptr; - FT_ATOMIC_STORE_PTR_RELEASE(*value_ptr, PyStackRef_AsPyObjectSteal(value)); + FT_ATOMIC_STORE_PTR_RELEASE(*value_ptr, Py_NewRef(value_o)); if (old_value == NULL) { PyDictValues *values = _PyObject_InlineValues(owner_o); Py_ssize_t index = value_ptr - values->values; _PyDictValues_AddToInsertionOrder(values, index); + } else { + PyRegion_RemoveRef(owner_o, old_value); } UNLOCK_OBJECT(owner_o); PyStackRef_CLOSE(owner); + PyStackRef_CLOSE(value); Py_XDECREF(old_value); } @@ -2664,6 +2695,7 @@ dummy_func( assert(Py_TYPE(owner_o)->tp_flags & Py_TPFLAGS_MANAGED_DICT); PyDictObject *dict = _PyObject_GetManagedDict(owner_o); DEOPT_IF(dict == NULL); + DEOPT_IF(!PyRegion_IsLocal(dict)); DEOPT_IF(!LOCK_OBJECT(dict)); if (!Py_CHECKWRITE(owner_o)) { @@ -2709,6 +2741,7 @@ dummy_func( op(_STORE_ATTR_SLOT, (index/1, value, owner --)) { PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner); + DEOPT_IF(!PyRegion_IsLocal(owner_o)); DEOPT_IF(!LOCK_OBJECT(owner_o)); // TODO(Immutable) If the dictionary object has been made and is immutable, then this should fail, // but we aren't finding the dictionary object here? Can we do this efficiently enough? @@ -4989,9 +5022,19 @@ dummy_func( inst(SET_FUNCTION_ATTRIBUTE, (attr_st, func_in -- func_out)) { PyObject *func = PyStackRef_AsPyObjectBorrow(func_in); + if (!Py_CHECKWRITE(func)) + { + _PyEval_FormatExcNotWriteable(tstate, _PyFrame_GetCode(frame), oparg); + DECREF_INPUTS(); + ERROR_IF(true); + } PyObject *attr = PyStackRef_AsPyObjectSteal(attr_st); func_out = func_in; DEAD(func_in); + if (PyRegion_AddRef(func, attr)) { + DECREF_INPUTS(); + ERROR_IF(true); + } assert(PyFunction_Check(func)); size_t offset = _Py_FunctionAttributeOffsets[oparg]; assert(offset != 0); diff --git a/Python/ceval.c b/Python/ceval.c index caaceb0fb13d2a..229311b8c7125e 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -2347,10 +2347,10 @@ _PyEval_UnpackIterableStackRef(PyThreadState *tstate, PyObject *v, if (w == NULL) { if (_PyErr_Occurred(tstate)) goto Error; - Py_DECREF(it); + PyRegion_CLEARLOCAL(it); return 1; } - Py_DECREF(w); + PyRegion_CLEARLOCAL(w); if (PyList_CheckExact(v) || PyTuple_CheckExact(v) || PyDict_CheckExact(v)) { @@ -2388,14 +2388,14 @@ _PyEval_UnpackIterableStackRef(PyThreadState *tstate, PyObject *v, } /* Resize the list. */ Py_SET_SIZE(l, ll - argcntafter); - Py_DECREF(it); + PyRegion_CLEARLOCAL(it); return 1; Error: for (; i > 0; i--, sp++) { PyStackRef_CLOSE(*sp); } - Py_XDECREF(it); + PyRegion_CLEARLOCAL(it); return 0; } diff --git a/Python/executor_cases.c.h b/Python/executor_cases.c.h index 9446bb18fcdee6..03d0c5bdf7c86e 100644 --- a/Python/executor_cases.c.h +++ b/Python/executor_cases.c.h @@ -1333,6 +1333,9 @@ assert(WITHIN_STACK_BOUNDS()); _PyFrame_SetStackPointer(frame, stack_pointer); res_o = PyObject_GetItem(PyStackRef_AsPyObjectBorrow(container), slice); + stack_pointer = _PyFrame_GetStackPointer(frame); + assert(PyRegion_IsLocal(slice)); + _PyFrame_SetStackPointer(frame, stack_pointer); Py_DECREF(slice); stack_pointer = _PyFrame_GetStackPointer(frame); stack_pointer += 2; @@ -1374,6 +1377,9 @@ assert(WITHIN_STACK_BOUNDS()); _PyFrame_SetStackPointer(frame, stack_pointer); err = PyObject_SetItem(PyStackRef_AsPyObjectBorrow(container), slice, PyStackRef_AsPyObjectBorrow(v)); + stack_pointer = _PyFrame_GetStackPointer(frame); + assert(PyRegion_IsLocal(slice)); + _PyFrame_SetStackPointer(frame, stack_pointer); Py_DECREF(slice); stack_pointer = _PyFrame_GetStackPointer(frame); stack_pointer += 2; @@ -1780,6 +1786,10 @@ JUMP_TO_JUMP_TARGET(); } Py_ssize_t index = ((PyLongObject*)sub)->long_value.ob_digit[0]; + if (!PyRegion_IsLocal(list)) { + UOP_STAT_INC(uopcode, miss); + JUMP_TO_JUMP_TARGET(); + } if (!LOCK_OBJECT(list)) { UOP_STAT_INC(uopcode, miss); JUMP_TO_JUMP_TARGET(); @@ -1802,6 +1812,7 @@ assert(WITHIN_STACK_BOUNDS()); _PyFrame_SetStackPointer(frame, stack_pointer); PyStackRef_CLOSE(list_st); + PyRegion_RemoveLocalRef(old_value); Py_DECREF(old_value); stack_pointer = _PyFrame_GetStackPointer(frame); break; @@ -2230,6 +2241,7 @@ assert(WITHIN_STACK_BOUNDS()); _PyFrame_SetStackPointer(frame, stack_pointer); int res = _PyEval_UnpackIterableStackRef(tstate, seq_o, oparg, -1, top); + PyRegion_RemoveLocalRef(seq_o); Py_DECREF(seq_o); stack_pointer = _PyFrame_GetStackPointer(frame); if (res == 0) { @@ -2335,6 +2347,7 @@ assert(WITHIN_STACK_BOUNDS()); _PyFrame_SetStackPointer(frame, stack_pointer); int res = _PyEval_UnpackIterableStackRef(tstate, seq_o, oparg & 0xFF, oparg >> 8, top); + PyRegion_AddLocalRef(seq_o); Py_DECREF(seq_o); stack_pointer = _PyFrame_GetStackPointer(frame); if (res == 0) { @@ -3700,19 +3713,49 @@ JUMP_TO_ERROR(); } assert(_PyObject_GetManagedDict(owner_o) == NULL); + PyObject *value_o = PyStackRef_AsPyObjectBorrow(value); + if (!PyRegion_IsLocal(owner_o) && !PyRegion_SameRegion(owner_o, value_o)) { + UOP_STAT_INC(uopcode, miss); + JUMP_TO_JUMP_TARGET(); + } + if (PyRegion_AddRef(owner_o, value_o)) { + UNLOCK_OBJECT(owner_o); + _PyFrame_SetStackPointer(frame, stack_pointer); + _PyStackRef tmp = owner; + owner = PyStackRef_NULL; + stack_pointer[-1] = owner; + PyStackRef_CLOSE(tmp); + tmp = value; + value = PyStackRef_NULL; + stack_pointer[-2] = value; + PyStackRef_CLOSE(tmp); + stack_pointer = _PyFrame_GetStackPointer(frame); + stack_pointer += -2; + assert(WITHIN_STACK_BOUNDS()); + JUMP_TO_ERROR(); + } PyObject **value_ptr = (PyObject**)(((char *)owner_o) + offset); PyObject *old_value = *value_ptr; - FT_ATOMIC_STORE_PTR_RELEASE(*value_ptr, PyStackRef_AsPyObjectSteal(value)); + FT_ATOMIC_STORE_PTR_RELEASE(*value_ptr, Py_NewRef(value_o)); if (old_value == NULL) { PyDictValues *values = _PyObject_InlineValues(owner_o); Py_ssize_t index = value_ptr - values->values; _PyDictValues_AddToInsertionOrder(values, index); + } else { + _PyFrame_SetStackPointer(frame, stack_pointer); + PyRegion_RemoveRef(owner_o, old_value); + stack_pointer = _PyFrame_GetStackPointer(frame); } UNLOCK_OBJECT(owner_o); - stack_pointer += -2; + stack_pointer += -1; assert(WITHIN_STACK_BOUNDS()); _PyFrame_SetStackPointer(frame, stack_pointer); PyStackRef_CLOSE(owner); + stack_pointer = _PyFrame_GetStackPointer(frame); + stack_pointer += -1; + assert(WITHIN_STACK_BOUNDS()); + _PyFrame_SetStackPointer(frame, stack_pointer); + PyStackRef_CLOSE(value); Py_XDECREF(old_value); stack_pointer = _PyFrame_GetStackPointer(frame); break; @@ -3732,6 +3775,10 @@ UOP_STAT_INC(uopcode, miss); JUMP_TO_JUMP_TARGET(); } + if (!PyRegion_IsLocal(dict)) { + UOP_STAT_INC(uopcode, miss); + JUMP_TO_JUMP_TARGET(); + } if (!LOCK_OBJECT(dict)) { UOP_STAT_INC(uopcode, miss); JUMP_TO_JUMP_TARGET(); @@ -3803,6 +3850,10 @@ value = stack_pointer[-2]; uint16_t index = (uint16_t)CURRENT_OPERAND0(); PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner); + if (!PyRegion_IsLocal(owner_o)) { + UOP_STAT_INC(uopcode, miss); + JUMP_TO_JUMP_TARGET(); + } if (!LOCK_OBJECT(owner_o)) { UOP_STAT_INC(uopcode, miss); JUMP_TO_JUMP_TARGET(); @@ -6786,8 +6837,31 @@ func_in = stack_pointer[-1]; attr_st = stack_pointer[-2]; PyObject *func = PyStackRef_AsPyObjectBorrow(func_in); + if (!Py_CHECKWRITE(func)) + { + _PyFrame_SetStackPointer(frame, stack_pointer); + _PyEval_FormatExcNotWriteable(tstate, _PyFrame_GetCode(frame), oparg); + _PyStackRef tmp = func_in; + func_in = PyStackRef_NULL; + stack_pointer[-1] = func_in; + PyStackRef_CLOSE(tmp); + tmp = attr_st; + attr_st = PyStackRef_NULL; + stack_pointer[-2] = attr_st; + PyStackRef_CLOSE(tmp); + stack_pointer = _PyFrame_GetStackPointer(frame); + stack_pointer += -2; + assert(WITHIN_STACK_BOUNDS()); + JUMP_TO_ERROR(); + } PyObject *attr = PyStackRef_AsPyObjectSteal(attr_st); func_out = func_in; + if (PyRegion_AddRef(func, attr)) { + stack_pointer[-2] = func_out; + stack_pointer += -1; + assert(WITHIN_STACK_BOUNDS()); + JUMP_TO_ERROR(); + } assert(PyFunction_Check(func)); size_t offset = _Py_FunctionAttributeOffsets[oparg]; assert(offset != 0); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 3766d628b67220..d2a8e71d5d303a 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -1141,6 +1141,9 @@ assert(WITHIN_STACK_BOUNDS()); _PyFrame_SetStackPointer(frame, stack_pointer); res_o = PyObject_GetItem(PyStackRef_AsPyObjectBorrow(container), slice); + stack_pointer = _PyFrame_GetStackPointer(frame); + assert(PyRegion_IsLocal(slice)); + _PyFrame_SetStackPointer(frame, stack_pointer); Py_DECREF(slice); stack_pointer = _PyFrame_GetStackPointer(frame); stack_pointer += 2; @@ -10211,6 +10214,7 @@ exc_st = stack_pointer[-1]; values = &stack_pointer[-1 - oparg]; PyObject *exc = PyStackRef_AsPyObjectSteal(exc_st); + assert(PyRegion_IsLocal(exc)); assert(oparg >= 0 && oparg <= 2); if (oparg) { frame->instr_ptr = _PyFrame_GetBytecode(frame) + PyStackRef_UntagInt(values[0]); @@ -10679,8 +10683,28 @@ func_in = stack_pointer[-1]; attr_st = stack_pointer[-2]; PyObject *func = PyStackRef_AsPyObjectBorrow(func_in); + if (!Py_CHECKWRITE(func)) + { + _PyFrame_SetStackPointer(frame, stack_pointer); + _PyEval_FormatExcNotWriteable(tstate, _PyFrame_GetCode(frame), oparg); + _PyStackRef tmp = func_in; + func_in = PyStackRef_NULL; + stack_pointer[-1] = func_in; + PyStackRef_CLOSE(tmp); + tmp = attr_st; + attr_st = PyStackRef_NULL; + stack_pointer[-2] = attr_st; + PyStackRef_CLOSE(tmp); + stack_pointer = _PyFrame_GetStackPointer(frame); + stack_pointer += -2; + assert(WITHIN_STACK_BOUNDS()); + JUMP_TO_LABEL(error); + } PyObject *attr = PyStackRef_AsPyObjectSteal(attr_st); func_out = func_in; + if (PyRegion_AddRef(func, attr)) { + JUMP_TO_LABEL(pop_2_error); + } assert(PyFunction_Check(func)); size_t offset = _Py_FunctionAttributeOffsets[oparg]; assert(offset != 0); @@ -10852,19 +10876,50 @@ JUMP_TO_LABEL(error); } assert(_PyObject_GetManagedDict(owner_o) == NULL); + PyObject *value_o = PyStackRef_AsPyObjectBorrow(value); + if (!PyRegion_IsLocal(owner_o) && !PyRegion_SameRegion(owner_o, value_o)) { + UPDATE_MISS_STATS(STORE_ATTR); + assert(_PyOpcode_Deopt[opcode] == (STORE_ATTR)); + JUMP_TO_PREDICTED(STORE_ATTR); + } + if (PyRegion_AddRef(owner_o, value_o)) { + UNLOCK_OBJECT(owner_o); + _PyFrame_SetStackPointer(frame, stack_pointer); + _PyStackRef tmp = owner; + owner = PyStackRef_NULL; + stack_pointer[-1] = owner; + PyStackRef_CLOSE(tmp); + tmp = value; + value = PyStackRef_NULL; + stack_pointer[-2] = value; + PyStackRef_CLOSE(tmp); + stack_pointer = _PyFrame_GetStackPointer(frame); + stack_pointer += -2; + assert(WITHIN_STACK_BOUNDS()); + JUMP_TO_LABEL(error); + } PyObject **value_ptr = (PyObject**)(((char *)owner_o) + offset); PyObject *old_value = *value_ptr; - FT_ATOMIC_STORE_PTR_RELEASE(*value_ptr, PyStackRef_AsPyObjectSteal(value)); + FT_ATOMIC_STORE_PTR_RELEASE(*value_ptr, Py_NewRef(value_o)); if (old_value == NULL) { PyDictValues *values = _PyObject_InlineValues(owner_o); Py_ssize_t index = value_ptr - values->values; _PyDictValues_AddToInsertionOrder(values, index); + } else { + _PyFrame_SetStackPointer(frame, stack_pointer); + PyRegion_RemoveRef(owner_o, old_value); + stack_pointer = _PyFrame_GetStackPointer(frame); } UNLOCK_OBJECT(owner_o); - stack_pointer += -2; + stack_pointer += -1; assert(WITHIN_STACK_BOUNDS()); _PyFrame_SetStackPointer(frame, stack_pointer); PyStackRef_CLOSE(owner); + stack_pointer = _PyFrame_GetStackPointer(frame); + stack_pointer += -1; + assert(WITHIN_STACK_BOUNDS()); + _PyFrame_SetStackPointer(frame, stack_pointer); + PyStackRef_CLOSE(value); Py_XDECREF(old_value); stack_pointer = _PyFrame_GetStackPointer(frame); } @@ -10902,6 +10957,11 @@ value = stack_pointer[-2]; uint16_t index = read_u16(&this_instr[4].cache); PyObject *owner_o = PyStackRef_AsPyObjectBorrow(owner); + if (!PyRegion_IsLocal(owner_o)) { + UPDATE_MISS_STATS(STORE_ATTR); + assert(_PyOpcode_Deopt[opcode] == (STORE_ATTR)); + JUMP_TO_PREDICTED(STORE_ATTR); + } if (!LOCK_OBJECT(owner_o)) { UPDATE_MISS_STATS(STORE_ATTR); assert(_PyOpcode_Deopt[opcode] == (STORE_ATTR)); @@ -10977,6 +11037,11 @@ assert(_PyOpcode_Deopt[opcode] == (STORE_ATTR)); JUMP_TO_PREDICTED(STORE_ATTR); } + if (!PyRegion_IsLocal(dict)) { + UPDATE_MISS_STATS(STORE_ATTR); + assert(_PyOpcode_Deopt[opcode] == (STORE_ATTR)); + JUMP_TO_PREDICTED(STORE_ATTR); + } if (!LOCK_OBJECT(dict)) { UPDATE_MISS_STATS(STORE_ATTR); assert(_PyOpcode_Deopt[opcode] == (STORE_ATTR)); @@ -11255,6 +11320,9 @@ assert(WITHIN_STACK_BOUNDS()); _PyFrame_SetStackPointer(frame, stack_pointer); err = PyObject_SetItem(PyStackRef_AsPyObjectBorrow(container), slice, PyStackRef_AsPyObjectBorrow(v)); + stack_pointer = _PyFrame_GetStackPointer(frame); + assert(PyRegion_IsLocal(slice)); + _PyFrame_SetStackPointer(frame, stack_pointer); Py_DECREF(slice); stack_pointer = _PyFrame_GetStackPointer(frame); stack_pointer += 2; @@ -11439,6 +11507,11 @@ JUMP_TO_PREDICTED(STORE_SUBSCR); } Py_ssize_t index = ((PyLongObject*)sub)->long_value.ob_digit[0]; + if (!PyRegion_IsLocal(list)) { + UPDATE_MISS_STATS(STORE_SUBSCR); + assert(_PyOpcode_Deopt[opcode] == (STORE_SUBSCR)); + JUMP_TO_PREDICTED(STORE_SUBSCR); + } if (!LOCK_OBJECT(list)) { UPDATE_MISS_STATS(STORE_SUBSCR); assert(_PyOpcode_Deopt[opcode] == (STORE_SUBSCR)); @@ -11463,6 +11536,7 @@ assert(WITHIN_STACK_BOUNDS()); _PyFrame_SetStackPointer(frame, stack_pointer); PyStackRef_CLOSE(list_st); + PyRegion_RemoveLocalRef(old_value); Py_DECREF(old_value); stack_pointer = _PyFrame_GetStackPointer(frame); } @@ -11859,6 +11933,7 @@ assert(WITHIN_STACK_BOUNDS()); _PyFrame_SetStackPointer(frame, stack_pointer); int res = _PyEval_UnpackIterableStackRef(tstate, seq_o, oparg & 0xFF, oparg >> 8, top); + PyRegion_AddLocalRef(seq_o); Py_DECREF(seq_o); stack_pointer = _PyFrame_GetStackPointer(frame); if (res == 0) { @@ -11909,6 +11984,7 @@ assert(WITHIN_STACK_BOUNDS()); _PyFrame_SetStackPointer(frame, stack_pointer); int res = _PyEval_UnpackIterableStackRef(tstate, seq_o, oparg, -1, top); + PyRegion_RemoveLocalRef(seq_o); Py_DECREF(seq_o); stack_pointer = _PyFrame_GetStackPointer(frame); if (res == 0) { diff --git a/Python/region.c b/Python/region.c index c3f7592aa109ba..909eb4b2631446 100644 --- a/Python/region.c +++ b/Python/region.c @@ -15,6 +15,23 @@ #include +// #define REGION_TRACING + +#ifdef REGION_TRACING +#define if_trace(...) __VA_ARGS__ +#define trace_arg(arg) , (Py_uintptr_t)(arg) +#define trace(msg, region, ...) \ + do { \ + printf(msg "\n", (Py_region_t)(region) __VA_OPT__(,) __VA_ARGS__); \ + } while(0) +#define trace_lrc(...) trace(__VA_ARGS__) +#else +#define if_trace(...) +#define trace_arg(...) +#define trace(...) +#define trace_lrc(...) +#endif + /* Macro that jumps to error, if the expression `x` does not succeed. */ #define SUCCEEDS(x) do { int r = (x); if (r != 0) goto error; } while (0) @@ -56,8 +73,8 @@ #define STAGED_AS_PTR(staged) (staged & STAGED_PTR_MASK) // Prototypes -static int regiondata_inc_osc(Py_region_t region); -static void regiondata_dec_osc(Py_region_t region); +static int regiondata_inc_osc(Py_region_t region if_trace(, Py_uintptr_t reason)); +static void regiondata_dec_osc(Py_region_t region if_trace(, Py_uintptr_t reason)); static int regiondata_is_open(Py_region_t data); static Py_region_t regiondata_get_parent(Py_region_t region); static Py_region_t regiondata_get_parent_follow_pending(Py_region_t region); @@ -490,7 +507,7 @@ static int regiondata_union_merge( bool cleanup_inc_osc = false; if (regiondata_is_open(target)) { // Inc OSC can't fail here, since `target` is already open - regiondata_inc_osc(target); + regiondata_inc_osc(target trace_arg(source)); cleanup_inc_osc = true; } @@ -534,6 +551,9 @@ static int regiondata_union_merge( // Merge stats into the `target` if (HAS_DATA(target)) { _Py_region_data *target_data = (_Py_region_data*)target; + trace(" - %lx: Merging %lx into %zx", source, source, target); + trace(" - %lx: Merge adjusts LRC += %ld", source, source_data->lrc); + trace(" - %lx: Merge adjusts OSC += %ld", source, source_data->osc); target_data->lrc += source_data->lrc; target_data->osc += source_data->osc; // Do a region merge, which keeps the bridge objects at the start @@ -547,7 +567,7 @@ static int regiondata_union_merge( // puts target into the right state. target_data->open_tick = source_data->open_tick; if (source_data->open_tick != OPEN_TICK_CLOSED) { - regiondata_inc_osc(regiondata_get_parent(target)); + regiondata_inc_osc(regiondata_get_parent(target) trace_arg(NULL)); } } else if (source_data->open_tick == OPEN_TICK_CLOSED) { // It's fine if the target is open but source is closed @@ -562,9 +582,12 @@ static int regiondata_union_merge( // Check if the region can be opened or closed. regiondata_check_status(target); } else if (IS_LOCAL_REGION(target)) { + trace("%lx: Dissolving %lx into the local region", source, source); // The function below also bumps the LRC of the sub-regions // meaning this should be all covered now. gc_region_list_dissolve(&(source_data->gc_list)); + } else { + trace("%lx: Merging %lx into %lx", source, source, target); } // Remove information from `source` @@ -579,13 +602,14 @@ static int regiondata_union_merge( goto cleanup; error: + trace("%lx: Merging %lx into %lx failed", source, source, target); result = 1; cleanup: // This returns the OSC which was acquired earlier to keep it open during // this merge. if (cleanup_inc_osc) { - regiondata_dec_osc(target); + regiondata_dec_osc(target trace_arg(NULL)); } // Decrement the `target` RC again @@ -619,6 +643,7 @@ static int regiondata_open(Py_region_t region) { // Mark the region as open. _Py_region_data *data = (_Py_region_data*)region; data->open_tick = _PyOwnership_get_open_region_tick(); + trace(" - %lx: is now open", data); // Check if opening the region was successful if (data->open_tick == OPEN_TICK_CLOSED) { @@ -641,13 +666,14 @@ static int regiondata_open(Py_region_t region) { } else if (regiondata_get_parent(region) != 0) { Py_region_t parent = regiondata_get_parent(region); SUCCEEDS(regiondata_open(parent)); - SUCCEEDS(regiondata_inc_osc(parent)); + SUCCEEDS(regiondata_inc_osc(parent trace_arg(region))); } // Check for failure, which would leave the region closed return 0; error: + trace(" - %lx: opening failed", region); // Mark the region as closed on failure. data->open_tick = OPEN_TICK_CLOSED; return 1; @@ -680,6 +706,7 @@ static void regiondata_mark_as_dirty(Py_region_t region) { // Mark region as dirty _Py_region_data* data = (_Py_region_data*)region; data->open_tick = OPEN_TICK_DIRTY; + trace(" - %lx: is now dirty", region); } static int regiondata_is_dirty(Py_region_t region) { @@ -740,6 +767,7 @@ static int regiondata_close(Py_region_t region) { // Mark the region as closed. _Py_region_data *data = (_Py_region_data*)region; data->open_tick = OPEN_TICK_CLOSED; + trace(" - %lx: is now closed", region); // Notify the owner if (HAS_OWNER_TAG(region, OWNER_TAG_COWN)) { @@ -748,14 +776,11 @@ static int regiondata_close(Py_region_t region) { // from error paths. } else if (regiondata_get_parent(region) != 0) { Py_region_t parent = regiondata_get_parent(region); - regiondata_dec_osc(parent); - SUCCEEDS(regiondata_check_status(parent)); + regiondata_dec_osc(parent trace_arg(region)); } // Check for failure, which would leave the region closed return 0; -error: - return -1; } static int regiondata_closes_after_lrc(Py_region_t region, Py_ssize_t lrc) { @@ -845,7 +870,7 @@ static int regiondata_check_status(Py_region_t region) { * This might open this and parent regions, which can fail. See * `regiondata_open` for possible failures. * */ -static int regiondata_inc_lrc(Py_region_t region) { +static int regiondata_inc_lrc(Py_region_t region if_trace(, Py_uintptr_t reason)) { // Invariant: ASSERT_IS_UNION_ROOT(region); @@ -862,6 +887,7 @@ static int regiondata_inc_lrc(Py_region_t region) { // Update the LRC, once the region is open _Py_region_data *data = (_Py_region_data*)region; data->lrc += 1; + trace_lrc("%lx: LRC++ for %lx", region, reason); return 0; } @@ -869,7 +895,7 @@ static int regiondata_inc_lrc(Py_region_t region) { /* This decreases the local reference count. * * */ -static void regiondata_dec_lrc(Py_region_t region) { +static void regiondata_dec_lrc(Py_region_t region if_trace(, Py_uintptr_t reason)) { // Invariant: ASSERT_IS_UNION_ROOT(region); @@ -881,6 +907,7 @@ static void regiondata_dec_lrc(Py_region_t region) { // Update the LRC _Py_region_data *data = (_Py_region_data*)region; if (data->lrc == 0) { + trace("%lx: LRC-- for %lx, but the LRC is 0", region, reason); // Try to open the region to mark it as dirty // // This can fail if the region is owned by a cown which @@ -896,6 +923,7 @@ static void regiondata_dec_lrc(Py_region_t region) { } } else { data->lrc -= 1; + trace_lrc("%lx: LRC-- for %lx", region, reason); // Check the region state to determine if it should be closed. SUCCEEDS(regiondata_check_close(region)); @@ -915,7 +943,7 @@ static void regiondata_dec_lrc(Py_region_t region) { * This might open this and parent regions, which can fail. See * `regiondata_open` for possible failures. * */ -static int regiondata_inc_osc(Py_region_t region) { +static int regiondata_inc_osc(Py_region_t region if_trace(, Py_uintptr_t reason)) { // Invariant: ASSERT_IS_UNION_ROOT(region); @@ -932,16 +960,14 @@ static int regiondata_inc_osc(Py_region_t region) { // Update the OSC, once the region is open _Py_region_data *data = (_Py_region_data*)region; data->osc += 1; + trace_lrc("%lx: OSC++ for %lx", region, reason); return 0; } /* This decreases the open-subregion count. (This does not update RC) - * - * This might close this and parent regions, which can fail. See - * `regiondata_close` for possible failures. * */ -static void regiondata_dec_osc(Py_region_t region) { +static void regiondata_dec_osc(Py_region_t region if_trace(, Py_uintptr_t reason)) { // Invariant: ASSERT_IS_UNION_ROOT(region); @@ -953,6 +979,7 @@ static void regiondata_dec_osc(Py_region_t region) { // Update the OSC _Py_region_data *data = (_Py_region_data*)region; data->osc -= 1; + trace_lrc("%lx: OSC-- for %lx", region, reason); // Check the region state to determine if it should be closed. SUCCEEDS(regiondata_check_close(region)); @@ -969,9 +996,6 @@ static void regiondata_dec_osc(Py_region_t region) { /* Setting the parent of an open region, might open the new parent region * and close the old parent region. - * - * This can fail, see `regiondata_open` and `regiondata_close` for possible - * failures. * */ static int regiondata_set_parent(Py_region_t region, Py_region_t new_parent) { // Check invariant: @@ -988,11 +1012,11 @@ static int regiondata_set_parent(Py_region_t region, Py_region_t new_parent) { // Notify the parents, if this region is open. if (regiondata_is_open(region)) { - if (regiondata_inc_osc(new_parent)) { + if (regiondata_inc_osc(new_parent trace_arg(region))) { return 1; } - regiondata_dec_osc(old_parent); + regiondata_dec_osc(old_parent trace_arg(region)); } // Make sure the sub-region is removed from the old parent and added to the @@ -1286,13 +1310,15 @@ typedef struct AddRegionState { static int _add_to_region_check_obj(PyObject *obj, void *state_void) { + trace(" - %lx: traversing %p", ((AddRegionState*)state_void)->merge_region, obj); + // Sanity Check, all objects given to this function should act like they're // in the subject region assert(_PyRegion_Get(obj) == ((AddRegionState*)state_void)->merge_region); assert(_PyRegion_GetFollowPending(obj) == ((AddRegionState*)state_void)->subject_region); // `_add_to_region_visit` already does the filtering and ensures that only - // new objects are traversed. This is therefore a no-op indicateing that + // new objects are traversed. This is therefore a no-op indicating that // the object should be traversed. return Py_OWNERSHIP_TRAVERSE_VISIT; } @@ -1343,16 +1369,20 @@ int _add_to_region_visit(PyObject *src, PyObject *tgt, void *state_void) { // Take ownership of local objects _Py_region_data *merge_data = (_Py_region_data*)state->merge_region; if (IS_LOCAL_REGION(tgt_region)) { - // Add incoming references to the LRC - // - // FIXME(regions): xFrednet: Handle weak references - merge_data->lrc += Py_REFCNT(tgt); + Py_ssize_t lrc_change = Py_REFCNT(tgt); // -1 if the RC accounts for a now intra-region reference if (!state->add_ref_target) { - merge_data->lrc -= 1; + lrc_change -= 1; } + trace(" - %lx: %p -> %p: Moving local target (LRC += %zd)", merge_data, src, tgt, lrc_change); + + // Add incoming references to the LRC + // + // FIXME(regions): xFrednet: Handle weak references + merge_data->lrc += lrc_change; + // Add the object to the merge region, this will also prevent it // from being traversed again. _PyRegion_SetMoveGC(tgt, state->merge_region); @@ -1365,6 +1395,7 @@ int _add_to_region_visit(PyObject *src, PyObject *tgt, void *state_void) { // added to the merge region by a previous iteration. This therefore only // adjusts the LRC if (tgt_region == state->subject_region) { + trace(" - %lx: %p -> %p: Intra-region reference (LRC--)", merge_data, src, tgt); // The LRC of the merge region can go negative by this operation as // this also includes references which should be subtract from the // LRC of the subject region. @@ -1471,6 +1502,7 @@ static PyRegion_staged_ref_t regiondata_stage_objects( _Py_region_data* merge_data = (_Py_region_data*)add_state.merge_region; regiondata_inc_rc(subject_region); merge_data->owner = (subject_region | OWNER_TAG_MERGE_PENDING); + trace("%lx: Staging %d references from %p in %zx", subject_region, tgt_count, src, add_state.merge_region); for (int tgt_i = 0; tgt_i < tgt_count; tgt_i += 1) { PyObject *tgt = targets[tgt_i]; @@ -1511,6 +1543,7 @@ static PyRegion_staged_ref_t regiondata_stage_objects( goto finally; error: + trace(" %lx: Staging failed", subject_region); if (HAS_OWNER_TAG(add_state.merge_region, OWNER_TAG_MERGE_PENDING)) { // Merge the region into local, to undo any ownership changes regiondata_union_merge(add_state.merge_region, _Py_LOCAL_REGION); @@ -1542,7 +1575,7 @@ static void staged_ref_reset(PyRegion_staged_ref_t staged_ref) { // and the one below is not the root of the union root? There is an // assert for this in `regiondata_dec_lrc` Py_region_t region = STAGED_AS_PTR(staged_ref); - regiondata_dec_lrc(region); + regiondata_dec_lrc(region trace_arg(staged_ref)); regiondata_dec_rc(region); return; } @@ -1557,7 +1590,7 @@ static void staged_ref_reset(PyRegion_staged_ref_t staged_ref) { Py_region_t region = regions[i]; // Decrement the LRC - regiondata_dec_lrc(region); + regiondata_dec_lrc(region trace_arg(staged_ref)); regiondata_dec_rc(region); i += 1; @@ -1663,7 +1696,7 @@ static int regiondata_clean(PyObject* bridge) { // Incrementing the RC of the bridge will ensure that we don't // accidentally release a cown early - if (regiondata_inc_lrc(_PyRegion_Get(bridge))) { + if (regiondata_inc_lrc(_PyRegion_Get(bridge) trace_arg(bridge))) { return -1; } Py_INCREF(bridge); @@ -1739,7 +1772,7 @@ static int regiondata_clean(PyObject* bridge) { // these would add a reference, expect cases when the region has been merged // into the local region. But then we should never have a reference to it. if (owner != 0) { - regiondata_dec_lrc(clean_region); + regiondata_dec_lrc(clean_region trace_arg(item)); } // The region should now be marked as clean @@ -1751,7 +1784,7 @@ static int regiondata_clean(PyObject* bridge) { clean_region_data->bridge = _PyRegionObject_CAST(item); clean_region_data->bridge->region = clean_region; // Move RC ownership if (!was_open && regiondata_is_open(clean_region)) { - regiondata_inc_osc(clean_region); + regiondata_inc_osc(clean_region trace_arg(clean_region)); } // Increase the number of regions which have been cleaned @@ -1766,7 +1799,7 @@ static int regiondata_clean(PyObject* bridge) { // Decrease the LRC, which was incremented at the start to keep the region // open. This shoudln't close the region, since the bridge object should // only be borrowed. - regiondata_dec_lrc(_PyRegion_Get(bridge)); + regiondata_dec_lrc(_PyRegion_Get(bridge) trace_arg(bridge)); Py_DECREF(bridge); Py_XDECREF(pending_list); // Resume invariant @@ -1850,7 +1883,7 @@ _Py_movable_status _PyRegion_GetMoveability(PyObject *obj) { if (PyFunction_Check(obj)) { return Py_MOVABLE_FREEZE; } - + // CWrappers can't really be owned, but need some special handling since // interpreters could still race on their RC. Solution, throw them in the // freezer @@ -1873,6 +1906,16 @@ _Py_movable_status _PyRegion_GetMoveability(PyObject *obj) { return Py_MOVABLE_NO; } + // Exceptions don't hold anything obviously problematic preventing them + // from being moved into a region. The actual problem is that the runtime + // stores references to them and that these are already emitted on an + // error path. Moving them into a region could add more problems. + // We should discuss how to handle these, maybe freezing is the correct + // approach? + if (PyExceptionInstance_Check(obj)) { + return Py_MOVABLE_NO; + } + // For now, we define all other objects as movable by default. (Surely // this will not backfire) return Py_MOVABLE_YES; @@ -1896,10 +1939,6 @@ int _PyRegion_New(_PyRegionObject *bridge) { bridge->region = region; assert(data->rc == 1); - // The region starts with an LRC of 1, due to the local reference to the - // bridge object - // regiondata_inc_lrc(region); - // This should never fail but might if the given bridge object has // some object which can't be moved. if (regiondata_add_object(region, NULL, _PyObject_CAST(bridge))) @@ -2127,7 +2166,7 @@ int _PyRegion_AddRef(PyObject *src, PyObject *tgt) { if (IS_LOCAL_REGION(src_region)) { // References from the local region are allowed, but need to be registered - return regiondata_inc_lrc(tgt_region); + return regiondata_inc_lrc(tgt_region trace_arg(tgt)); } // Attempt to slurp the target object into the source region @@ -2146,7 +2185,7 @@ static int _add_local_refs(PyObject *src, int tgt_count, PyObject **targets) { for (arg_i = 0; arg_i < tgt_count; arg_i += 1) { PyObject* tgt = targets[arg_i]; - result = regiondata_inc_lrc(_PyRegion_Get(tgt)); + result = regiondata_inc_lrc(_PyRegion_Get(tgt) trace_arg(tgt)); if (result != 0) { goto error; @@ -2158,7 +2197,7 @@ static int _add_local_refs(PyObject *src, int tgt_count, PyObject **targets) { error: for (int undo_i = 0; undo_i < arg_i; undo_i += 1) { PyObject* tgt = targets[undo_i]; - regiondata_dec_lrc(_PyRegion_Get(tgt)); + regiondata_dec_lrc(_PyRegion_Get(tgt) trace_arg(tgt)); } return result; } @@ -2167,7 +2206,7 @@ static PyRegion_staged_ref_t _stage_local_refs(PyObject *src, int argc, PyObject // Fast/No-allocation path for single local references if (argc == 1) { Py_region_t tgt_region = _PyRegion_Get(targets[0]); - if (regiondata_inc_lrc(tgt_region)) { + if (regiondata_inc_lrc(tgt_region trace_arg(targets[0]))) { return PyRegion_staged_ref_ERR; } regiondata_inc_rc(tgt_region); @@ -2190,7 +2229,7 @@ static PyRegion_staged_ref_t _stage_local_refs(PyObject *src, int argc, PyObject Py_region_t tgt_region = _PyRegion_Get(targets[i]); // LRC += 1 - if (regiondata_inc_lrc(tgt_region)) { + if (regiondata_inc_lrc(tgt_region trace_arg(targets[i]))) { goto error; } @@ -2350,7 +2389,7 @@ void _PyRegion_RemoveRef(PyObject *src, PyObject *tgt) { if (IS_LOCAL_REGION(src_region)) { // Decrease the target region LRC since this reference came from // the local region - regiondata_dec_lrc(_PyRegion_Get(tgt)); + regiondata_dec_lrc(_PyRegion_Get(tgt) trace_arg(tgt)); return; } @@ -2380,7 +2419,7 @@ void _PyRegion_RemoveRef(PyObject *src, PyObject *tgt) { } int _PyRegion_AddLocalRef(PyObject *tgt) { - return regiondata_inc_lrc(_PyRegion_Get(tgt)); + return regiondata_inc_lrc(_PyRegion_Get(tgt) trace_arg(tgt)); } int _PyRegion_AddLocalRefs(int argc, ...) { @@ -2415,7 +2454,7 @@ int _PyRegion_AddLocalRefs(int argc, ...) { } void _PyRegion_RemoveLocalRef(PyObject *tgt) { - regiondata_dec_lrc(_PyRegion_Get(tgt)); + regiondata_dec_lrc(_PyRegion_Get(tgt) trace_arg(tgt)); } int _PyRegion_TakeRefs(PyObject *src, int argc, ...) { diff --git a/Tools/cases_generator/analyzer.py b/Tools/cases_generator/analyzer.py index 9dd7e5dbfbae7b..a2414f6df39f7c 100644 --- a/Tools/cases_generator/analyzer.py +++ b/Tools/cases_generator/analyzer.py @@ -576,6 +576,8 @@ def has_error_without_pop(op: parser.CodeDef) -> bool: "PyLong_AsLong", "PyLong_FromLong", "PyLong_FromSsize_t", + "PyRegion_IsLocal", + "PyRegion_SameRegion", "PySlice_New", "PyStackRef_AsPyObjectBorrow", "PyStackRef_AsPyObjectNew",