From 2713a36b526f584483b7285f9a514d512e24f1ca Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Mon, 3 Jan 2022 13:24:52 -0800 Subject: [PATCH 1/6] Stop tracking the exact depth of each frame --- Include/internal/pycore_frame.h | 6 ++++-- Python/ceval.c | 16 ++++++++-------- Tools/gdb/libpython.py | 10 +++++----- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h index 8eca39d1ab2503..c21d7de91c576a 100644 --- a/Include/internal/pycore_frame.h +++ b/Include/internal/pycore_frame.h @@ -4,6 +4,8 @@ extern "C" { #endif +#include + /* runtime lifecycle */ @@ -44,7 +46,7 @@ typedef struct _interpreter_frame { int f_lasti; /* Last instruction if called */ int stacktop; /* Offset of TOS from localsplus */ PyFrameState f_state; /* What state the frame is in */ - int depth; /* Depth of the frame in a ceval loop */ + bool own_cframe; // Whether this is the root frame for the current cframe. PyObject *localsplus[1]; } InterpreterFrame; @@ -101,7 +103,7 @@ _PyFrame_InitializeSpecials( frame->generator = NULL; frame->f_lasti = -1; frame->f_state = FRAME_CREATED; - frame->depth = 0; + frame->own_cframe = true; } /* Gets the pointer to the locals array diff --git a/Python/ceval.c b/Python/ceval.c index 43925e6db269ce..288615c076bc72 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1683,7 +1683,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr cframe.previous = prev_cframe; tstate->cframe = &cframe; - assert(frame->depth == 0); + assert(frame->own_cframe); /* Push frame */ frame->previous = prev_cframe->current_frame; cframe.current_frame = frame; @@ -2310,7 +2310,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr _PyFrame_SetStackPointer(frame, stack_pointer); new_frame->previous = frame; frame = cframe.current_frame = new_frame; - new_frame->depth = frame->depth + 1; + frame->own_cframe = false; goto start_frame; } @@ -2475,7 +2475,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr TRACE_FUNCTION_EXIT(); DTRACE_FUNCTION_EXIT(); _Py_LeaveRecursiveCall(tstate); - if (frame->depth) { + if (!frame->own_cframe) { frame = cframe.current_frame = pop_frame(tstate, frame); _PyFrame_StackPush(frame, retval); goto resume_frame; @@ -2625,7 +2625,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr } TARGET(SEND) { - assert(frame->depth == 0); + assert(frame->own_cframe); assert(STACK_LEVEL() >= 2); PyObject *v = POP(); PyObject *receiver = TOP(); @@ -2684,7 +2684,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr } TARGET(YIELD_VALUE) { - assert(frame->depth == 0); + assert(frame->own_cframe); PyObject *retval = POP(); if (frame->f_code->co_flags & CO_ASYNC_GENERATOR) { @@ -4645,7 +4645,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr _PyFrame_SetStackPointer(frame, stack_pointer); new_frame->previous = frame; cframe.current_frame = frame = new_frame; - new_frame->depth = frame->depth + 1; + frame->own_cframe = false; goto start_frame; } } @@ -4739,7 +4739,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr _PyFrame_SetStackPointer(frame, stack_pointer); new_frame->previous = frame; frame = cframe.current_frame = new_frame; - new_frame->depth = frame->depth + 1; + frame->own_cframe = false; goto start_frame; } @@ -5415,7 +5415,7 @@ MISS_WITH_OPARG_COUNTER(STORE_SUBSCR) exit_unwind: assert(_PyErr_Occurred(tstate)); _Py_LeaveRecursiveCall(tstate); - if (frame->depth == 0) { + if (frame->own_cframe) { /* Restore previous cframe and exit */ tstate->cframe = cframe.previous; tstate->cframe->use_tracing = cframe.use_tracing; diff --git a/Tools/gdb/libpython.py b/Tools/gdb/libpython.py index a0a95e3fc63cba..5c3ce1cef6987d 100755 --- a/Tools/gdb/libpython.py +++ b/Tools/gdb/libpython.py @@ -1044,8 +1044,8 @@ def _f_nlocalsplus(self): def _f_lasti(self): return self._f_special("f_lasti", int_from_int) - def depth(self): - return self._f_special("depth", int_from_int) + def own_cframe(self): + return self._f_special("own_cframe", bool) def previous(self): return self._f_special("previous", PyFramePtr) @@ -1860,7 +1860,7 @@ def print_summary(self): line = interp_frame.current_line() if line is not None: sys.stdout.write(' %s\n' % line.strip()) - if interp_frame.depth() == 0: + if interp_frame.own_cframe(): break else: sys.stdout.write('#%i (unable to read python frame information)\n' % self.get_index()) @@ -1883,7 +1883,7 @@ def print_traceback(self): line = interp_frame.current_line() if line is not None: sys.stdout.write(' %s\n' % line.strip()) - if interp_frame.depth() == 0: + if interp_frame.own_cframe(): break else: sys.stdout.write(' (unable to read python frame information)\n') @@ -2147,7 +2147,7 @@ def invoke(self, args, from_tty): % (pyop_name.proxyval(set()), pyop_value.get_truncated_repr(MAX_OUTPUT_LEN))) - if pyop_frame.depth() == 0: + if pyop_frame.own_cframe(): break pyop_frame = pyop_frame.previous() From 439a0c23a3d8714306a9071985cdbcc192962124 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Mon, 3 Jan 2022 13:42:22 -0800 Subject: [PATCH 2/6] Clean up comment --- Include/internal/pycore_frame.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h index c21d7de91c576a..fa55fba7394a86 100644 --- a/Include/internal/pycore_frame.h +++ b/Include/internal/pycore_frame.h @@ -46,7 +46,7 @@ typedef struct _interpreter_frame { int f_lasti; /* Last instruction if called */ int stacktop; /* Offset of TOS from localsplus */ PyFrameState f_state; /* What state the frame is in */ - bool own_cframe; // Whether this is the root frame for the current cframe. + bool own_cframe; // Whether this is the "root" frame for the current CFrame PyObject *localsplus[1]; } InterpreterFrame; From 6037cba2db8318c90e1948b71f2a6100af4feae8 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Mon, 3 Jan 2022 15:06:19 -0800 Subject: [PATCH 3/6] Fix test for sizeof(frame) --- Lib/test/test_sys.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index 96075cf3b3473c..a34a257ce630e9 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -1323,7 +1323,7 @@ class C(object): pass def func(): return sys._getframe() x = func() - check(x, size('3Pi3c8P2iciP')) + check(x, size('3Pi3c8P2ic?P')) # function def func(): pass check(func, size('14Pi')) From 323da04925bb0729d1d7c53925dae5f9666596c5 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Mon, 3 Jan 2022 15:46:06 -0800 Subject: [PATCH 4/6] Fix test for sizeof(generator) --- Lib/test/test_sys.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index a34a257ce630e9..38771d427da7b1 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -1340,7 +1340,7 @@ def bar(cls): check(bar, size('PP')) # generator def get_gen(): yield 1 - check(get_gen(), size('P2P4P4c8P2iciP')) + check(get_gen(), size('P2P4P4c8P2ic?P')) # iterator check(iter('abc'), size('lP')) # callable-iterator From 98c58f3b8af7fd1a3ca630a5bf0e5da40998d462 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Tue, 4 Jan 2022 11:54:07 -0800 Subject: [PATCH 5/6] Apply suggestions from code review --- Include/internal/pycore_frame.h | 4 ++-- Python/ceval.c | 16 ++++++++-------- Tools/gdb/libpython.py | 10 +++++----- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h index fa55fba7394a86..42df51f635615d 100644 --- a/Include/internal/pycore_frame.h +++ b/Include/internal/pycore_frame.h @@ -46,7 +46,7 @@ typedef struct _interpreter_frame { int f_lasti; /* Last instruction if called */ int stacktop; /* Offset of TOS from localsplus */ PyFrameState f_state; /* What state the frame is in */ - bool own_cframe; // Whether this is the "root" frame for the current CFrame + bool is_entry; // Whether this is the "root" frame for the current CFrame. PyObject *localsplus[1]; } InterpreterFrame; @@ -103,7 +103,7 @@ _PyFrame_InitializeSpecials( frame->generator = NULL; frame->f_lasti = -1; frame->f_state = FRAME_CREATED; - frame->own_cframe = true; + frame->is_entry = false; } /* Gets the pointer to the locals array diff --git a/Python/ceval.c b/Python/ceval.c index 19b39f14fc7cd7..4e60bae4690102 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1683,7 +1683,8 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr cframe.previous = prev_cframe; tstate->cframe = &cframe; - assert(frame->own_cframe); + assert(!frame->is_entry); + frame->is_entry = true; /* Push frame */ frame->previous = prev_cframe->current_frame; cframe.current_frame = frame; @@ -2310,7 +2311,6 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr _PyFrame_SetStackPointer(frame, stack_pointer); new_frame->previous = frame; frame = cframe.current_frame = new_frame; - frame->own_cframe = false; goto start_frame; } @@ -2475,7 +2475,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr TRACE_FUNCTION_EXIT(); DTRACE_FUNCTION_EXIT(); _Py_LeaveRecursiveCall(tstate); - if (!frame->own_cframe) { + if (!frame->is_entry) { frame = cframe.current_frame = pop_frame(tstate, frame); _PyFrame_StackPush(frame, retval); goto resume_frame; @@ -2625,7 +2625,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr } TARGET(SEND) { - assert(frame->own_cframe); + assert(frame->is_entry); assert(STACK_LEVEL() >= 2); PyObject *v = POP(); PyObject *receiver = TOP(); @@ -2680,11 +2680,12 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr tstate->cframe->use_tracing = cframe.use_tracing; assert(tstate->cframe->current_frame == frame->previous); assert(!_PyErr_Occurred(tstate)); + frame->is_entry = false; return retval; } TARGET(YIELD_VALUE) { - assert(frame->own_cframe); + assert(frame->is_entry); PyObject *retval = POP(); if (frame->f_code->co_flags & CO_ASYNC_GENERATOR) { @@ -2706,6 +2707,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr tstate->cframe->use_tracing = cframe.use_tracing; assert(tstate->cframe->current_frame == frame->previous); assert(!_PyErr_Occurred(tstate)); + frame->is_entry = false; return retval; } @@ -4620,7 +4622,6 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr _PyFrame_SetStackPointer(frame, stack_pointer); new_frame->previous = frame; cframe.current_frame = frame = new_frame; - frame->own_cframe = false; goto start_frame; } } @@ -4714,7 +4715,6 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr _PyFrame_SetStackPointer(frame, stack_pointer); new_frame->previous = frame; frame = cframe.current_frame = new_frame; - frame->own_cframe = false; goto start_frame; } @@ -5390,7 +5390,7 @@ MISS_WITH_OPARG_COUNTER(STORE_SUBSCR) exit_unwind: assert(_PyErr_Occurred(tstate)); _Py_LeaveRecursiveCall(tstate); - if (frame->own_cframe) { + if (frame->is_entry) { /* Restore previous cframe and exit */ tstate->cframe = cframe.previous; tstate->cframe->use_tracing = cframe.use_tracing; diff --git a/Tools/gdb/libpython.py b/Tools/gdb/libpython.py index 5c3ce1cef6987d..e3d73bce6cfe5a 100755 --- a/Tools/gdb/libpython.py +++ b/Tools/gdb/libpython.py @@ -1044,8 +1044,8 @@ def _f_nlocalsplus(self): def _f_lasti(self): return self._f_special("f_lasti", int_from_int) - def own_cframe(self): - return self._f_special("own_cframe", bool) + def is_entry(self): + return self._f_special("is_entry", bool) def previous(self): return self._f_special("previous", PyFramePtr) @@ -1860,7 +1860,7 @@ def print_summary(self): line = interp_frame.current_line() if line is not None: sys.stdout.write(' %s\n' % line.strip()) - if interp_frame.own_cframe(): + if interp_frame.is_entry(): break else: sys.stdout.write('#%i (unable to read python frame information)\n' % self.get_index()) @@ -1883,7 +1883,7 @@ def print_traceback(self): line = interp_frame.current_line() if line is not None: sys.stdout.write(' %s\n' % line.strip()) - if interp_frame.own_cframe(): + if interp_frame.is_entry(): break else: sys.stdout.write(' (unable to read python frame information)\n') @@ -2147,7 +2147,7 @@ def invoke(self, args, from_tty): % (pyop_name.proxyval(set()), pyop_value.get_truncated_repr(MAX_OUTPUT_LEN))) - if pyop_frame.own_cframe(): + if pyop_frame.is_entry(): break pyop_frame = pyop_frame.previous() From 369983b9a3e3f3774c6e5f49ee3e1eb42a1af569 Mon Sep 17 00:00:00 2001 From: Brandt Bucher Date: Tue, 4 Jan 2022 12:50:17 -0800 Subject: [PATCH 6/6] Don't bother unsetting frame->is_entry --- Python/ceval.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/Python/ceval.c b/Python/ceval.c index 4e60bae4690102..2f4701d2738622 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1683,7 +1683,6 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr cframe.previous = prev_cframe; tstate->cframe = &cframe; - assert(!frame->is_entry); frame->is_entry = true; /* Push frame */ frame->previous = prev_cframe->current_frame; @@ -2680,7 +2679,6 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr tstate->cframe->use_tracing = cframe.use_tracing; assert(tstate->cframe->current_frame == frame->previous); assert(!_PyErr_Occurred(tstate)); - frame->is_entry = false; return retval; } @@ -2707,7 +2705,6 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr tstate->cframe->use_tracing = cframe.use_tracing; assert(tstate->cframe->current_frame == frame->previous); assert(!_PyErr_Occurred(tstate)); - frame->is_entry = false; return retval; }