Add PyFrame for non-jitted frames during the stack walk

Summary: Currently exception information in logview could include sanitized data from locals (that is populated from frame objects). In order to switch to cinder powered async stacks we need to provide a way to obtain this information at least for non-jitted frames. This diff augments existing stack walking machinery to also pass in `PyFrameObject*` for Python frames or `NULL` for its JIT counterparts

Reviewed By: mpage

Differential Revision: D50098541

fbshipit-source-id: f7939d01307076be0dcde1a04ac80d07e1f48ae0

Cinder/Include/cinder/exports.h

@@ -126,7 +126,8 @@ typedef CiStackWalkDirective (*CiWalkStackCallback)(void *data,
 typedef CiStackWalkDirective (*CiWalkAsyncStackCallback)(void *data,
                                                     PyObject *fqname,
                                                     PyCodeObject *code,
-                                                    int lineno);
+                                                    int lineno,
+                                                    PyObject* pyFrame);
 
 /*
  * Walk the stack, invoking cb for each entry with the supplied data parameter

Jit/frame.cpp

@@ -542,7 +542,8 @@ void walkShadowStack(PyThreadState* tstate, FrameHandler handler) {
 
 // Called during stack walking for each item on the async stack. Returns false
 // to terminate stack walking.
-using AsyncFrameHandler = std::function<bool(PyObject*, const CodeObjLoc&)>;
+using AsyncFrameHandler =
+    std::function<bool(PyObject*, const CodeObjLoc&, PyObject*)>;
 
 // Invoke handler for each shadow frame on the async stack.
 void walkAsyncShadowStack(PyThreadState* tstate, AsyncFrameHandler handler) {
@@ -554,7 +555,8 @@ void walkAsyncShadowStack(PyThreadState* tstate, AsyncFrameHandler handler) {
     switch (owner) {
       case PYSF_INTERP: {
         PyFrameObject* py_frame = _PyShadowFrame_GetPyFrame(shadow_frame);
-        if (!handler(qualname.get(), CodeObjLoc(py_frame))) {
+        if (!handler(
+                qualname.get(), CodeObjLoc(py_frame), (PyObject*)py_frame)) {
           return;
         }
         break;
@@ -564,7 +566,7 @@ void walkAsyncShadowStack(PyThreadState* tstate, AsyncFrameHandler handler) {
         // single chunk, starting with innermost inlined frame.
         UnitState unit_state = getUnitState(shadow_frame);
         for (auto it = unit_state.rbegin(); it != unit_state.rend(); ++it) {
-          if (!handler(qualname.get(), it->loc)) {
+          if (!handler(qualname.get(), it->loc, nullptr)) {
             return;
           }
         }
@@ -797,13 +799,14 @@ int _PyShadowFrame_WalkAndPopulate(
   *sync_stack_len_out = 0;
 
   // First walk the async stack
-  jit::walkAsyncShadowStack(tstate, [&](PyObject*, const jit::CodeObjLoc& loc) {
-    int idx = *async_stack_len_out;
-    async_stack[idx] = loc.code;
-    async_linenos[idx] = loc.lineNo();
-    (*async_stack_len_out)++;
-    return *async_stack_len_out < array_capacity;
-  });
+  jit::walkAsyncShadowStack(
+      tstate, [&](PyObject*, const jit::CodeObjLoc& loc, PyObject* /*unused*/) {
+        int idx = *async_stack_len_out;
+        async_stack[idx] = loc.code;
+        async_linenos[idx] = loc.lineNo();
+        (*async_stack_len_out)++;
+        return *async_stack_len_out < array_capacity;
+      });
 
   // Next walk the sync stack
   jit::walkShadowStack(
@@ -830,8 +833,9 @@ void Ci_WalkAsyncStack(
     CiWalkAsyncStackCallback cb,
     void* data) {
   jit::walkAsyncShadowStack(
-      tstate, [&](PyObject* qualname, const jit::CodeObjLoc& loc) {
-        return cb(data, qualname, loc.code, loc.lineNo()) ==
+      tstate,
+      [&](PyObject* qualname, const jit::CodeObjLoc& loc, PyObject* pyFrame) {
+        return cb(data, qualname, loc.code, loc.lineNo(), pyFrame) ==
             CI_SWD_CONTINUE_STACK_WALK;
       });
 }

Lib/test/test_cinder.py

@@ -28,7 +28,7 @@
 from tempfile import TemporaryDirectory
 from textwrap import dedent
 
-from types import CodeType, FunctionType, GeneratorType, ModuleType
+from types import CodeType, FrameType, FunctionType, GeneratorType, ModuleType
 from typing import List, Tuple
 from unittest import skipIf
 from unittest.mock import patch
@@ -2107,6 +2107,7 @@ def test_get_stack_with_lineno_2(self):
 import asyncio
 
 result = None
+
 async def f1():
     await asyncio.sleep(0)
     return await f2()
@@ -2129,10 +2130,94 @@ async def g1():
         self.assertEqual(
             last_frames,
             [
-                ("module1:g0", 15),
-                ("module1:g1", 18),
-                ("module1:f1", 8),
-                ("module1:f2", 12),
+                ("module1:g0", 16),
+                ("module1:g1", 19),
+                ("module1:f1", 9),
+                ("module1:f2", 13),
+            ],
+        )
+
+    @unittest.skipIf(not cinderjit, "Tests functionality on cinderjit module")
+    def test_get_stack_with_lineno_3(self):
+        # use code string to have deterministic line numbers
+        code = """
+from cinder import _get_entire_call_stack_as_qualnames_with_lineno_and_frame
+from cinderjit import jit_suppress
+from test import cinder_support
+
+import asyncio
+
+result = None
+
+@cinder_support.failUnlessJITCompiled
+async def f1():
+    return await f2()
+
+@jit_suppress
+async def f2():
+    global result
+    result = _get_entire_call_stack_as_qualnames_with_lineno_and_frame()
+
+@cinder_support.failUnlessJITCompiled
+async def g0():
+    return await f1();
+
+asyncio.run(g0())
+        """
+        g = {"__name__": "module1"}
+        exec(code, g)
+        m, l, f = g["result"][-1]
+        self.assertEqual((m, l), ("module1:f2", 17))
+        self.assertIsInstance(f, FrameType)
+        self.assertEqual(f.f_code.co_name, "f2")
+        self.assertEqual(
+            g["result"][-3:-1], [("module1:g0", 21, None), ("module1:f1", 12, None)]
+        )
+
+    @unittest.skipIf(not cinderjit, "Tests functionality on cinderjit module")
+    def test_get_stack_with_lineno_4(self):
+        # use code string to have deterministic line numbers
+        code = """
+from cinder import _get_entire_call_stack_as_qualnames_with_lineno_and_frame
+from cinderjit import jit_suppress
+from test import cinder_support
+
+import asyncio
+
+result = None
+@cinder_support.failUnlessJITCompiled
+async def f1():
+    await asyncio.sleep(0)
+    return await f2()
+
+@jit_suppress
+async def f2():
+    global result
+    result = _get_entire_call_stack_as_qualnames_with_lineno_and_frame()
+
+@cinder_support.failUnlessJITCompiled
+async def g0():
+    return await g1();
+
+@cinder_support.failUnlessJITCompiled
+async def g1():
+    return await asyncio.gather(f1())
+
+asyncio.run(g0())
+        """
+        g = {"__name__": "module1"}
+        exec(code, g)
+        m, l, f = g["result"][-1]
+        self.assertEqual((m, l), ("module1:f2", 17))
+        self.assertIsInstance(f, FrameType)
+        self.assertEqual(f.f_code.co_name, "f2")
+
+        self.assertEqual(
+            g["result"][-4:-1],
+            [
+                ("module1:g0", 21, None),
+                ("module1:g1", 25, None),
+                ("module1:f1", 12, None),
             ],
         )
 

Modules/cinder.c

@@ -439,13 +439,15 @@ get_call_stack(PyObject *self, PyObject *args) {
 typedef struct {
     PyObject *list;
     int hasError;
+    int collectFrame;
 } StackWalkState;
 
 static CiStackWalkDirective frame_data_collector(
     void *data,
     PyObject *fqname,
     PyCodeObject *code,
-    int lineno)
+    int lineno,
+    PyObject *pyframe)
 {
     StackWalkState *state = (StackWalkState*)data;
     if (fqname == NULL) {
@@ -454,7 +456,7 @@ static CiStackWalkDirective frame_data_collector(
             fqname = ((PyCodeObject *)code)->co_name;
         }
     }
-    PyObject *t = PyTuple_New(2);
+    PyObject *t = PyTuple_New(2 + state->collectFrame);
     if (t == NULL) {
         goto fail;
     }
@@ -468,6 +470,15 @@ static CiStackWalkDirective frame_data_collector(
 
     // steals ref
     PyTuple_SET_ITEM(t, 1, lineNoObj);
+
+    if (state->collectFrame) {
+        PyObject *o = pyframe;
+        if (!o) {
+            o = Py_None;
+        }
+        PyTuple_SET_ITEM(t, 2, o);
+        Py_INCREF(o);
+    }
     int failed = PyList_Append(state->list, t);
     Py_DECREF(t);
     if (!failed) {
@@ -479,19 +490,29 @@ static CiStackWalkDirective frame_data_collector(
 }
 
 static PyObject*
-get_entire_call_stack_as_qualnames_with_lineno(PyObject *self, PyObject *Py_UNUSED(args)) {
+collect_stack(int collectFrame) {
     PyObject *stack = PyList_New(0);
     if (stack == NULL) {
         return NULL;
     }
-    StackWalkState state = { .list=stack, .hasError = 0 };
+    StackWalkState state = { .list=stack, .hasError = 0, .collectFrame = collectFrame };
     Ci_WalkAsyncStack(PyThreadState_GET(), frame_data_collector, &state);
     if (state.hasError || (PyList_Reverse(stack) != 0)) {
         Py_CLEAR(stack);
     }
     return stack;
 }
 
+static PyObject*
+get_entire_call_stack_as_qualnames_with_lineno(PyObject *self, PyObject *Py_UNUSED(args)) {
+    return collect_stack(0);
+}
+
+static PyObject*
+get_entire_call_stack_as_qualnames_with_lineno_and_frame(PyObject *self, PyObject *Py_UNUSED(args)) {
+    return collect_stack(1);
+}
+
 static PyObject*
 get_entire_call_stack_as_qualnames(PyObject *self, PyObject *Py_UNUSED(args)) {
   _PyShadowFrame *shadow_frame = PyThreadState_GET()->shadow_frame;
@@ -705,7 +726,7 @@ static struct PyMethodDef cinder_module_methods[] = {
      "Set the period, in bytecode instructions, for interpreter profiling."},
     {"get_and_clear_type_profiles",
      get_and_clear_type_profiles,
-     METH_NOARGS,
+    METH_NOARGS,
      "Get and clear accumulated interpreter type profiles."},
     {"get_and_clear_type_profiles_with_metadata",
      get_and_clear_type_profiles_with_metadata,
@@ -741,6 +762,10 @@ static struct PyMethodDef cinder_module_methods[] = {
         get_entire_call_stack_as_qualnames_with_lineno,
         METH_NOARGS,
         "Return the current stack as a list of tuples (qualname, lineno)."},
+    {"_get_entire_call_stack_as_qualnames_with_lineno_and_frame",
+        get_entire_call_stack_as_qualnames_with_lineno_and_frame,
+        METH_NOARGS,
+        "Return the current stack as a list of tuples (qualname, lineno, PyFrame | None)."},
     {"watch_sys_modules",
         watch_sys_modules,
         METH_NOARGS,