gh-109094: replace frame->prev_instr by frame->instr_ptr

Include/internal/pycore_frame.h

@@ -58,26 +58,16 @@ typedef struct _PyInterpreterFrame{
  PyObject *f_builtins; /* Borrowed reference. Only valid if not on C stack */
  PyObject *f_locals; /* Strong reference, may be NULL. Only valid if not on C stack */
  PyFrameObject *frame_obj; /* Strong reference, may be NULL. Only valid if not on C stack */
- // NOTE: This is not necessarily the last instruction started in the given
- // frame. Rather, it is the code unit *prior to* the *next* instruction. For
- // example, it may be an inline CACHE entry, an instruction we just jumped
- // over, or (in the case of a newly-created frame) a totally invalid value:
- _Py_CODEUNIT *prev_instr;
+ _Py_CODEUNIT *instr_ptr; /* Instruction currently executing (or about to begin) */
  int stacktop; /* Offset of TOS from localsplus */
- /* The return_offset determines where a `RETURN` should go in the caller,
- * relative to `prev_instr`.
- * It is only meaningful to the callee,
- * so it needs to be set in any CALL (to a Python function)
- * or SEND (to a coroutine or generator).
- * If there is no callee, then it is meaningless. */
- uint16_t return_offset;
+ uint16_t return_offset; /* Only relevant during a function call */
  char owner;
  /* Locals and stack */
  PyObject *localsplus[1];
 } _PyInterpreterFrame;
 
 #define _PyInterpreterFrame_LASTI(IF) \
- ((int)((IF)->prev_instr - _PyCode_CODE(_PyFrame_GetCode(IF))))
+ ((int)((IF)->instr_ptr - _PyCode_CODE(_PyFrame_GetCode(IF))))
 
 static inline PyCodeObject *_PyFrame_GetCode(_PyInterpreterFrame *f){
  assert(PyCode_Check(f->f_executable));
@@ -134,7 +124,7 @@ _PyFrame_Initialize(
  frame->f_locals = locals;
  frame->stacktop = code->co_nlocalsplus;
  frame->frame_obj = NULL;
- frame->prev_instr = _PyCode_CODE(code) - 1;
+ frame->instr_ptr = _PyCode_CODE(code);
  frame->return_offset = 0;
  frame->owner = FRAME_OWNED_BY_THREAD;
 
@@ -185,7 +175,7 @@ _PyFrame_IsIncomplete(_PyInterpreterFrame *frame)
  return true;
  }
  return frame->owner != FRAME_OWNED_BY_GENERATOR &&
- frame->prev_instr < _PyCode_CODE(_PyFrame_GetCode(frame)) + _PyFrame_GetCode(frame)->_co_firsttraceable;
+frame->instr_ptr < _PyCode_CODE(_PyFrame_GetCode(frame)) + _PyFrame_GetCode(frame)->_co_firsttraceable;
 }
 
 static inline _PyInterpreterFrame *
@@ -297,7 +287,7 @@ _PyFrame_PushTrampolineUnchecked(PyThreadState *tstate, PyCodeObject *code, int
  frame->f_locals = NULL;
  frame->stacktop = code->co_nlocalsplus + stackdepth;
  frame->frame_obj = NULL;
- frame->prev_instr = _PyCode_CODE(code);
+ frame->instr_ptr = _PyCode_CODE(code);
  frame->owner = FRAME_OWNED_BY_THREAD;
  frame->return_offset = 0;
  return frame;

Include/internal/pycore_runtime.h

@@ -87,7 +87,7 @@ typedef struct _Py_DebugOffsets{
  struct _interpreter_frame{
  off_t previous;
  off_t executable;
- off_t prev_instr;
+ off_t instr_ptr;
  off_t localsplus;
  off_t owner;
  } interpreter_frame;

Include/internal/pycore_runtime_init.h

@@ -58,7 +58,7 @@ extern PyTypeObject _PyExc_MemoryError;
  .interpreter_frame ={\
  .previous = offsetof(_PyInterpreterFrame, previous), \
  .executable = offsetof(_PyInterpreterFrame, f_executable), \
- .prev_instr = offsetof(_PyInterpreterFrame, prev_instr), \
+ .instr_ptr = offsetof(_PyInterpreterFrame, instr_ptr), \
  .localsplus = offsetof(_PyInterpreterFrame, localsplus), \
  .owner = offsetof(_PyInterpreterFrame, owner), \
  }, \

Misc/NEWS.d/next/Core and Builtins/2023-10-13-16-55-55.gh-issue-109094.ziL4cJ.rst

@@ -0,0 +1,3 @@
+Replace ``prev_instr`` on the interpreter frame by ``instr_ptr`` which
+points to the beginning of the instruction that is currently executing (or
+will execute once the frame resumes).

Objects/frame_layout.md

@@ -130,3 +130,28 @@ returns. This extra frame is inserted so that `RETURN_VALUE`, `YIELD_VALUE`, and
 `RETURN_GENERATOR` do not need to check whether the current frame is the entry frame.
 The shim frame points to a special code object containing the `INTERPRETER_EXIT`
 instruction which cleans up the shim frame and returns.
+
+
+### The Instruction Pointer
+
+`_PyInterpreterFrame` has two fields which are used to maintain the instruction
+pointer: `instr_ptr` and `return_offset`.
+
+When a frame is executing, `instr_ptr` points to the instruction currently being
+executed. In a suspended frame, it points to the instruction that would execute
+if the frame were to resume. After `frame.f_lineno` is set, `instr_ptr` points to
+the next instruction to be executed. During a call to a python function,
+`instr_ptr` points to the call instruction, because this is what we would expect
+to see in an exception traceback.
+
+The `return_offset` field determines where a `RETURN` should go in the caller,
+relative to `instr_ptr`. It is only meaningful to the callee, so it needs to
+be set in any instruction that implements a call (to a Python function),
+including CALL, SEND and BINARY_SUBSCR_GETITEM, among others. If there is no
+callee, then return_offset is meaningless. It is necessary to have a separate
+field for the return offset because (1) if we apply this offset to `instr_ptr`
+while executing the `RETURN`, this is too early and would lose us information
+about the previous instruction which we could need for introspecting and
+debugging. (2) `SEND` needs to pass two offsets to the generator: one for
+`RETURN` and one for `YIELD`. It uses the `oparg` for one, and the
+`return_offset` for the other.

Objects/frameobject.c

@@ -821,7 +821,7 @@ frame_setlineno(PyFrameObject *f, PyObject* p_new_lineno, void *Py_UNUSED(ignore
  }
  /* Finally set the new lasti and return OK. */
  f->f_lineno = 0;
- f->f_frame->prev_instr = _PyCode_CODE(code) + best_addr;
+ f->f_frame->instr_ptr = _PyCode_CODE(code) + best_addr;
  return 0;
 }
 
@@ -1079,7 +1079,7 @@ PyFrame_New(PyThreadState *tstate, PyCodeObject *code,
  f->f_frame = (_PyInterpreterFrame *)f->_f_frame_data;
  f->f_frame->owner = FRAME_OWNED_BY_FRAME_OBJECT;
  // This frame needs to be "complete", so pretend that the first RESUME ran:
- f->f_frame->prev_instr = _PyCode_CODE(code) + code->_co_firsttraceable;
+ f->f_frame->instr_ptr = _PyCode_CODE(code) + code->_co_firsttraceable + 1;
  assert(!_PyFrame_IsIncomplete(f->f_frame));
  Py_DECREF(func);
  _PyObject_GC_TRACK(f);
@@ -1093,7 +1093,7 @@ _PyFrame_OpAlreadyRan(_PyInterpreterFrame *frame, int opcode, int oparg)
  assert(_PyOpcode_Deopt[opcode] == opcode);
  int check_oparg = 0;
  for (_Py_CODEUNIT *instruction = _PyCode_CODE(_PyFrame_GetCode(frame));
- instruction < frame->prev_instr; instruction++)
+ instruction < frame->instr_ptr; instruction++)
 {
  int check_opcode = _PyOpcode_Deopt[instruction->op.code];
  check_oparg |= instruction->op.arg;
@@ -1135,7 +1135,7 @@ frame_init_get_vars(_PyInterpreterFrame *frame)
  frame->localsplus[offset + i] = Py_NewRef(o);
  }
  // COPY_FREE_VARS doesn't have inline CACHEs, either:
- frame->prev_instr = _PyCode_CODE(_PyFrame_GetCode(frame));
+ frame->instr_ptr = _PyCode_CODE(_PyFrame_GetCode(frame));
 }
 
 

Objects/genobject.c

@@ -9,6 +9,7 @@
 #include "pycore_genobject.h" // struct _Py_async_gen_state
 #include "pycore_modsupport.h" // _PyArg_CheckPositional()
 #include "pycore_object.h" // _PyObject_GC_UNTRACK()
+#include "pycore_opcode_metadata.h" // _PyOpcode_Caches
 #include "pycore_pyerrors.h" // _PyErr_ClearExcState()
 #include "pycore_pystate.h" // _PyThreadState_GET()
 
@@ -362,8 +363,7 @@ _PyGen_yf(PyGenObject *gen)
  assert(_PyCode_CODE(_PyGen_GetCode(gen))[0].op.code != SEND);
  return NULL;
  }
- _Py_CODEUNIT next = frame->prev_instr[1];
- if (!is_resume(&next) || next.op.arg < 2)
+ if (!is_resume(frame->instr_ptr) || frame->instr_ptr->op.arg < 2)
 {
  /* Not in a yield from */
  return NULL;
@@ -398,9 +398,12 @@ gen_close(PyGenObject *gen, PyObject *args)
  _PyInterpreterFrame *frame = (_PyInterpreterFrame *)gen->gi_iframe;
  /* It is possible for the previous instruction to not be a
  * YIELD_VALUE if the debugger has changed the lineno. */
- if (err == 0 && is_yield(frame->prev_instr)){
- assert(is_resume(frame->prev_instr + 1));
- int exception_handler_depth = frame->prev_instr[0].op.arg;
+ assert(_PyOpcode_Caches[YIELD_VALUE] == 0);
+ assert(_PyOpcode_Caches[INSTRUMENTED_YIELD_VALUE] == 0);
+ if (err == 0 && is_yield(frame->instr_ptr - 1)){
+ _Py_CODEUNIT *yield_instr = frame->instr_ptr - 1;
+ assert(is_resume(frame->instr_ptr));
+ int exception_handler_depth = yield_instr->op.arg;
  assert(exception_handler_depth > 0);
  /* We can safely ignore the outermost try block
  * as it automatically generated to handle