pythongh-107758: Improvements to lltrace feature (python#107757)

gvanrossum · web-flow · commit 328d92524451 · 2023-08-07T21:36:25.000-07:00
- The `dump_stack()` method could call a `__repr__` method implemented in Python,
  causing (infinite) recursion.
  I rewrote it to only print out the values for some fundamental types (`int`, `str`, etc.);
  for everything else it just prints `&lt;type_name @ 0xdeadbeef&gt;`.

- The lltrace-like feature for uops wrote to `stderr`, while the one in `ceval.c` writes to `stdout`;
  I changed the uops to write to stdout as well.
diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-08-08-02-46-46.gh-issue-107758.R5kyBI.rst b/Misc/NEWS.d/next/Core and Builtins/2023-08-08-02-46-46.gh-issue-107758.R5kyBI.rst
@@ -0,0 +1 @@
+Make the ``dump_stack()`` routine used by the ``lltrace`` feature (low-level interpreter debugging) robust against recursion by ensuring that it never calls a ``__repr__`` method implemented in Python. Also make the similar output for Tier-2 uops appear on ``stdout`` (instead of ``stderr``), to match the ``lltrace`` code in ceval.c.
diff --git a/Python/ceval.c b/Python/ceval.c
@@ -109,11 +109,24 @@ dump_stack(_PyInterpreterFrame *frame, PyObject **stack_pointer)
         if (ptr != stack_base) {
             printf(", ");
         }
-        if (PyObject_Print(*ptr, stdout, 0) != 0) {
+        if (*ptr == NULL) {
+            printf("<nil>");
+            continue;
+        }
+        if (
+            *ptr == Py_None
+            || PyBool_Check(*ptr)
+            || PyLong_CheckExact(*ptr)
+            || PyFloat_CheckExact(*ptr)
+            || PyUnicode_CheckExact(*ptr)
+        ) {
+            if (PyObject_Print(*ptr, stdout, 0) == 0) {
+                continue;
+            }
             PyErr_Clear();
-            printf("<%s object at %p>",
-                   Py_TYPE(*ptr)->tp_name, (void *)(*ptr));
         }
+        // Don't call __repr__(), it might recurse into the interpreter.
+        printf("<%s at %p>", Py_TYPE(*ptr)->tp_name, (void *)(*ptr));
     }
     printf("]\n");
     fflush(stdout);
@@ -128,9 +141,6 @@ lltrace_instruction(_PyInterpreterFrame *frame,
     if (frame->owner == FRAME_OWNED_BY_CSTACK) {
         return;
     }
-    /* This dump_stack() operation is risky, since the repr() of some
-       objects enters the interpreter recursively. It is also slow.
-       So you might want to comment it out. */
     dump_stack(frame, stack_pointer);
     int oparg = next_instr->op.arg;
     int opcode = next_instr->op.code;
@@ -729,6 +739,13 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
                 goto exit_unwind;
             }
             lltrace = r;
+            if (!lltrace) {
+                // When tracing executed uops, also trace bytecode
+                char *uop_debug = Py_GETENV("PYTHONUOPSDEBUG");
+                if (uop_debug != NULL && *uop_debug >= '0') {
+                    lltrace = (*uop_debug - '0') >= 4;  // TODO: Parse an int and all that
+                }
+            }
         }
         if (lltrace) {
             lltrace_resume_frame(frame);
@@ -896,6 +913,11 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
                 goto exception_unwind;
             }
             /* Resume normal execution */
+#ifdef LLTRACE
+            if (lltrace) {
+                lltrace_resume_frame(frame);
+            }
+#endif
             DISPATCH();
         }
     }
diff --git a/Python/executor.c b/Python/executor.c
@@ -41,7 +41,7 @@ _PyUopExecute(_PyExecutorObject *executor, _PyInterpreterFrame *frame, PyObject
         lltrace = *uop_debug - '0';  // TODO: Parse an int and all that
     }
     #define DPRINTF(level, ...) \
-        if (lltrace >= (level)) { fprintf(stderr, __VA_ARGS__); }
+        if (lltrace >= (level)) { printf(__VA_ARGS__); }
 #else
     #define DPRINTF(level, ...)
 #endif
diff --git a/Python/optimizer.c b/Python/optimizer.c
@@ -391,7 +391,7 @@ translate_bytecode_to_trace(
 
 #ifdef Py_DEBUG
 #define DPRINTF(level, ...) \
-    if (lltrace >= (level)) { fprintf(stderr, __VA_ARGS__); }
+    if (lltrace >= (level)) { printf(__VA_ARGS__); }
 #else
 #define DPRINTF(level, ...)
 #endif

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+Make the ``dump_stack()`` routine used by the ``lltrace`` feature (low-level interpreter debugging) robust against recursion by ensuring that it never calls a ``__repr__`` method implemented in Python. Also make the similar output for Tier-2 uops appear on ``stdout`` (instead of ``stderr``), to match the ``lltrace`` code in ceval.c.
Original file line number	Diff line number	Diff line change
`@@ -41,7 +41,7 @@ _PyUopExecute(_PyExecutorObject executor, _PyInterpreterFrame frame, PyObject`
`41`	`41`	`lltrace = *uop_debug - '0'; // TODO: Parse an int and all that`
`42`	`42`	`}`
`43`	`43`	`#define DPRINTF(level, ...) \`
`44`		`- if (lltrace >= (level)) { fprintf(stderr, __VA_ARGS__); }`
	`44`	`+ if (lltrace >= (level)) { printf(__VA_ARGS__); }`
`45`	`45`	`#else`
`46`	`46`	`#define DPRINTF(level, ...)`
`47`	`47`	`#endif`