[SOT][3.13] fix GetLocals free error in eval_custom_code and clean cpython_internals.c some _PyFrame_GetCode (#69267)

gouzil · web-flow · commit 6d4ce585d106 · 2024-11-12T21:35:40.000+08:00
diff --git a/paddle/fluid/pybind/sot/cpython_internals.c b/paddle/fluid/pybind/sot/cpython_internals.c
@@ -49,11 +49,7 @@ int Internal_PyUnstable_InterpreterFrame_GetLine(_PyInterpreterFrame *frame) {
 int Internal_PyInterpreterFrame_GetLine(_PyInterpreterFrame *frame) {
 #endif
   int addr = _PyInterpreterFrame_LASTI(frame) * sizeof(_Py_CODEUNIT);
-#if PY_3_13_PLUS
-  return PyCode_Addr2Line(_PyFrame_GetCode(frame), addr);
-#else
-  return PyCode_Addr2Line(frame->f_code, addr);
-#endif
+  return PyCode_Addr2Line(PyFrame_GET_CODE(frame), addr);
 }
 
 static int Internal_PyFrame_OpAlreadyRan(_PyInterpreterFrame *frame,
@@ -62,15 +58,14 @@ static int Internal_PyFrame_OpAlreadyRan(_PyInterpreterFrame *frame,
   // This only works when opcode is a non-quickened form:
   assert(_PyOpcode_Deopt[opcode] == opcode);
   int check_oparg = 0;
+  for (_Py_CODEUNIT *instruction = _PyCode_CODE(PyFrame_GET_CODE(frame));
 #if PY_3_13_PLUS
-  for (_Py_CODEUNIT *instruction = _PyCode_CODE(_PyFrame_GetCode(frame));
        instruction < frame->instr_ptr;
-       instruction++) {
 #else
-  for (_Py_CODEUNIT *instruction = _PyCode_CODE(frame->f_code);
        instruction < frame->prev_instr;
-       instruction++) {
 #endif
+       instruction++) {
+
 #if PY_3_12_PLUS
     int check_opcode = _PyOpcode_Deopt[instruction->op.code];
     check_oparg |= instruction->op.arg;
@@ -225,9 +220,9 @@ bool Internal_PyFrame_HasHiddenLocals(_PyInterpreterFrame *frame) {
 
   return false;
 }
-static PyObject *framelocalsproxy_new(PyTypeObject *type,
-                                      PyObject *args,
-                                      PyObject *kwds) {
+static PyObject *Internal_framelocalsproxy_new(PyTypeObject *type,
+                                               PyObject *args,
+                                               PyObject *kwds) {
   if (PyTuple_GET_SIZE(args) != 1) {
     PyErr_Format(PyExc_TypeError,
                  "FrameLocalsProxy expected 1 argument, got %zd",
@@ -265,8 +260,8 @@ PyObject *Internal_PyFrameLocalsProxy_New(PyFrameObject *frame) {
     return NULL;
   }
 
-  PyObject *proxy =
-      (PyObject *)framelocalsproxy_new(&PyFrameLocalsProxy_Type, args, NULL);
+  PyObject *proxy = (PyObject *)Internal_framelocalsproxy_new(
+      &PyFrameLocalsProxy_Type, args, NULL);
   Py_DECREF(args);
   return proxy;
 }
@@ -634,11 +629,7 @@ PyFrameObject *Internal_PyFrame_MakeAndSetFrameObject(
   PyErr_Fetch(&error_type, &error_value, &error_traceback);
 #endif
 
-#if PY_3_13_PLUS
-  PyFrameObject *f = Internal_PyFrame_New_NoTrack(_PyFrame_GetCode(frame));
-#else
-  PyFrameObject *f = Internal_PyFrame_New_NoTrack(frame->f_code);
-#endif
+  PyFrameObject *f = Internal_PyFrame_New_NoTrack(PyFrame_GET_CODE(frame));
   if (f == NULL) {
 #if PY_3_12_PLUS
     Py_XDECREF(exc);
@@ -712,24 +703,22 @@ static void Internal_take_ownership(PyFrameObject *f,
   Py_ssize_t size =
       ((char *)&frame->localsplus[frame->stacktop]) - (char *)frame;
 
-#if PY_3_13_PLUS
-  Py_INCREF(_PyFrame_GetCode(frame));
-#elif PY_3_12_PLUS
-  Py_INCREF(frame->f_code);
+#if PY_3_12_PLUS
+  Py_INCREF(PyFrame_GET_CODE(frame));
 #endif
 
   memcpy((_PyInterpreterFrame *)f->_f_frame_data, frame, size);
   frame = (_PyInterpreterFrame *)f->_f_frame_data;
   f->f_frame = frame;
   frame->owner = FRAME_OWNED_BY_FRAME_OBJECT;
   if (_PyFrame_IsIncomplete(frame)) {
-// This may be a newly-created generator or coroutine frame. Since it's
-// dead anyways, just pretend that the first RESUME ran:
+    // This may be a newly-created generator or coroutine frame. Since it's
+    // dead anyways, just pretend that the first RESUME ran:
+    PyCodeObject *code = PyFrame_GET_CODE(frame);
+
 #if PY_3_13_PLUS
-    PyCodeObject *code = _PyFrame_GetCode(frame);
     frame->instr_ptr = _PyCode_CODE(code) + code->_co_firsttraceable + 1;
 #else
-    PyCodeObject *code = frame->f_code;
     frame->prev_instr = _PyCode_CODE(code) + code->_co_firsttraceable;
 #endif
   }
@@ -792,8 +781,8 @@ void Internal_PyFrame_Clear(_PyInterpreterFrame *frame) {
    * to have cleared the enclosing generator, if any. */
   assert(frame->owner != FRAME_OWNED_BY_GENERATOR ||
          _PyFrame_GetGenerator(frame)->gi_frame_state == FRAME_CLEARED);
-// GH-99729: Clearing this frame can expose the stack (via finalizers). It's
-// crucial that this frame has been unlinked, and is no longer visible:
+  // GH-99729: Clearing this frame can expose the stack (via finalizers). It's
+  // crucial that this frame has been unlinked, and is no longer visible:
 #if PY_3_13_PLUS
   assert(PyThreadState_GET()->current_frame != frame);
 #else
diff --git a/paddle/fluid/pybind/sot/eval_frame.c b/paddle/fluid/pybind/sot/eval_frame.c
@@ -47,6 +47,9 @@ typedef _PyInterpreterFrame FrameObject;
 typedef struct PyInterpreterFrameProxy {
   PyObject_HEAD
   _PyInterpreterFrame *frame;
+  #if PY_3_13_PLUS
+  PyObject* locals;
+  #endif
 } PyInterpreterFrameProxy;
 // clang-format on
 
@@ -70,9 +73,8 @@ DECLARE_PROXY_PROPERTY(f_code)
 #if PY_3_13_PLUS
 static PyObject *PyInterpreterFrameProxy_property_f_locals(
     PyInterpreterFrameProxy *self, void *closure) {
-  PyObject *f_locals = Internal_PyFrame_GetLocals(self->frame);
-  Py_XINCREF(f_locals);
-  return f_locals;
+  Py_XINCREF(self->locals);
+  return self->locals;
 }
 #else
 DECLARE_PROXY_PROPERTY(f_locals)
@@ -134,6 +136,9 @@ PyInterpreterFrameProxy *PyInterpreterFrameProxy_New(
     return NULL;
   }
   self->frame = frame;
+#if PY_3_13_PLUS
+  self->locals = NULL;
+#endif
   return self;
 }
 
@@ -349,12 +354,13 @@ static PyObject *_custom_eval_frame(PyThreadState *tstate,
   // _PyFrame_FastToLocalsWithError directly. But this is an internal API, so we
   // copy many code from CPython project into our project.
 #if PY_3_13_PLUS
-  Internal_PyFrame_GetLocals(frame);
+  PyObject *f_locals = Internal_PyFrame_GetLocals(frame);
+  if (f_locals == NULL) {
 #else
   if (Internal_PyFrame_FastToLocalsWithError(frame) < 0) {
+#endif
     return NULL;
   }
-#endif
 #else
   if (frame->f_code->co_flags & 0x20) {
     out = eval_frame_default(tstate, frame, throw_flag);
@@ -394,7 +400,17 @@ static PyObject *_custom_eval_frame(PyThreadState *tstate,
   } else {
     /* should calculate guards here if we want */
 #if PY_3_11_PLUS
-    PyObject *args = Py_BuildValue("(O)", PyInterpreterFrameProxy_New(frame));
+    PyInterpreterFrameProxy *frame_proxy = PyInterpreterFrameProxy_New(frame);
+    if (frame_proxy == NULL) {
+#if PY_3_13_PLUS
+      Py_DECREF(f_locals);
+#endif
+      return NULL;
+    }
+#if PY_3_13_PLUS
+    frame_proxy->locals = f_locals;
+#endif
+    PyObject *args = Py_BuildValue("(O)", frame_proxy);
 #else
     PyObject *args = Py_BuildValue("(O)", frame);
 #endif
@@ -403,6 +419,7 @@ static PyObject *_custom_eval_frame(PyThreadState *tstate,
     if (result == NULL) {
 #if PY_3_12_PLUS
 #if PY_3_13_PLUS
+      Py_DECREF(f_locals);
       Internal_PyEval_FrameClearAndPop(tstate, frame);
 #else
       Internal_PyEvalFrameClearAndPop(tstate, frame);
@@ -415,6 +432,10 @@ static PyObject *_custom_eval_frame(PyThreadState *tstate,
     Py_DECREF(result);
   }
 
+#if PY_3_13_PLUS
+  Py_DECREF(f_locals);
+#endif
+
   // code status
   if (is_code_without_graph(code == Py_None ? PyFrame_GET_CODE(frame)
                                             : (PyCodeObject *)code) &&
diff --git a/test/sot/skip_files_py313 b/test/sot/skip_files_py313
@@ -23,8 +23,6 @@ test/sot/test_builtin_range.py
 test/sot/test_builtin_zip.py
 test/sot/test_call_object.py
 test/sot/test_constant_graph.py
-test/sot/test_dataclass.py
-test/sot/test_delete_fast.py
 test/sot/test_dtype.py
 test/sot/test_dup_top.py
 test/sot/test_enumerate.py
@@ -36,7 +34,6 @@ test/sot/test_numpy.py
 test/sot/test_numpy_var_if.py
 test/sot/test_side_effects.py
 test/sot/test_simulate_initialize.py
-test/sot/test_sir_rollback.py
 test/sot/test_sot_cost_model.py
 test/sot/test_sot_dynamic_shape.py
 test/sot/test_sot_exception.py
