GH-103488: Use return-offset, not yield-offset. (GH-103502)

* Use return-offset, not yield-offset, so that instruction pointer is correct when sending to a generator or coroutine.

Include/internal/pycore_frame.h

@@ -61,7 +61,13 @@ typedef struct _PyInterpreterFrame {
  // over, or (in the case of a newly-created frame) a totally invalid value:
  _Py_CODEUNIT *prev_instr;
  int stacktop; /* Offset of TOS from localsplus */
- uint16_t yield_offset;
+ /* 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;
  char owner;
  /* Locals and stack */
  PyObject *localsplus[1];
@@ -121,7 +127,7 @@ _PyFrame_Initialize(
  frame->stacktop = code->co_nlocalsplus;
  frame->frame_obj = NULL;
  frame->prev_instr = _PyCode_CODE(code) - 1;
- frame->yield_offset = 0;
+ frame->return_offset = 0;
  frame->owner = FRAME_OWNED_BY_THREAD;
 
  for (int i = null_locals_from; i < code->co_nlocalsplus; i++) {

Lib/test/test_generators.py

@@ -225,7 +225,22 @@ def f():
  gi = f()
  self.assertIsNone(gi.gi_frame.f_back)
 
+ def test_issue103488(self):
 
+ def gen_raises():
+ yield
+ raise ValueError()
+
+ def loop():
+ try:
+ for _ in gen_raises():
+ if True is False:
+ return
+ except ValueError:
+ pass
+
+ #This should not raise
+ loop()
 
 class ExceptionTest(unittest.TestCase):
  # Tests for the issue #23353: check that the currently handled exception

Misc/NEWS.d/next/Core and Builtins/2023-04-12-20-18-51.gh-issue-103488.vYvlHD.rst

@@ -0,0 +1,3 @@
+Change the internal offset distinguishing yield and return target addresses,
+so that the instruction pointer is correct for exception handling and other
+stack unwinding.

Python/bytecodes.c

@@ -506,6 +506,7 @@ dummy_func(
  new_frame->localsplus[0] = container;
  new_frame->localsplus[1] = sub;
  JUMPBY(INLINE_CACHE_ENTRIES_BINARY_SUBSCR);
+ frame->return_offset = 0;
  DISPATCH_INLINED(new_frame);
  }
 
@@ -637,6 +638,7 @@ dummy_func(
  _PyInterpreterFrame *dying = frame;
  frame = cframe.current_frame = dying->previous;
  _PyEvalFrameClearAndPop(tstate, dying);
+ frame->prev_instr += frame->return_offset;
  _PyFrame_StackPush(frame, retval);
  goto resume_frame;
  }
@@ -655,6 +657,7 @@ dummy_func(
  _PyInterpreterFrame *dying = frame;
  frame = cframe.current_frame = dying->previous;
  _PyEvalFrameClearAndPop(tstate, dying);
+ frame->prev_instr += frame->return_offset;
  _PyFrame_StackPush(frame, retval);
  goto resume_frame;
  }
@@ -670,6 +673,7 @@ dummy_func(
  _PyInterpreterFrame *dying = frame;
  frame = cframe.current_frame = dying->previous;
  _PyEvalFrameClearAndPop(tstate, dying);
+ frame->prev_instr += frame->return_offset;
  _PyFrame_StackPush(frame, retval);
  goto resume_frame;
  }
@@ -689,6 +693,7 @@ dummy_func(
  _PyInterpreterFrame *dying = frame;
  frame = cframe.current_frame = dying->previous;
  _PyEvalFrameClearAndPop(tstate, dying);
+ frame->prev_instr += frame->return_offset;
  _PyFrame_StackPush(frame, retval);
  goto resume_frame;
  }
@@ -823,13 +828,13 @@ dummy_func(
  {
  PyGenObject *gen = (PyGenObject *)receiver;
  _PyInterpreterFrame *gen_frame = (_PyInterpreterFrame *)gen->gi_iframe;
- frame->yield_offset = oparg;
+ frame->return_offset = oparg;
  STACK_SHRINK(1);
  _PyFrame_StackPush(gen_frame, v);
  gen->gi_frame_state = FRAME_EXECUTING;
  gen->gi_exc_state.previous_item = tstate->exc_info;
  tstate->exc_info = &gen->gi_exc_state;
- JUMPBY(INLINE_CACHE_ENTRIES_SEND + oparg);
+ JUMPBY(INLINE_CACHE_ENTRIES_SEND);
  DISPATCH_INLINED(gen_frame);
  }
  if (Py_IsNone(v) && PyIter_Check(receiver)) {
@@ -861,13 +866,13 @@ dummy_func(
  DEOPT_IF(gen->gi_frame_state >= FRAME_EXECUTING, SEND);
  STAT_INC(SEND, hit);
  _PyInterpreterFrame *gen_frame = (_PyInterpreterFrame *)gen->gi_iframe;
- frame->yield_offset = oparg;
+ frame->return_offset = oparg;
  STACK_SHRINK(1);
  _PyFrame_StackPush(gen_frame, v);
  gen->gi_frame_state = FRAME_EXECUTING;
  gen->gi_exc_state.previous_item = tstate->exc_info;
  tstate->exc_info = &gen->gi_exc_state;
- JUMPBY(INLINE_CACHE_ENTRIES_SEND + oparg);
+ JUMPBY(INLINE_CACHE_ENTRIES_SEND);
  DISPATCH_INLINED(gen_frame);
  }
 
@@ -886,7 +891,6 @@ dummy_func(
  _PyInterpreterFrame *gen_frame = frame;
  frame = cframe.current_frame = frame->previous;
  gen_frame->previous = NULL;
- frame->prev_instr -= frame->yield_offset;
  _PyFrame_StackPush(frame, retval);
  goto resume_frame;
  }
@@ -905,7 +909,6 @@ dummy_func(
  _PyInterpreterFrame *gen_frame = frame;
  frame = cframe.current_frame = frame->previous;
  gen_frame->previous = NULL;
- frame->prev_instr -= frame->yield_offset;
  _PyFrame_StackPush(frame, retval);
  goto resume_frame;
  }
@@ -1724,6 +1727,7 @@ dummy_func(
  STACK_SHRINK(shrink_stack);
  new_frame->localsplus[0] = owner;
  JUMPBY(INLINE_CACHE_ENTRIES_LOAD_ATTR);
+ frame->return_offset = 0;
  DISPATCH_INLINED(new_frame);
  }
 
@@ -1751,6 +1755,7 @@ dummy_func(
  new_frame->localsplus[0] = owner;
  new_frame->localsplus[1] = Py_NewRef(name);
  JUMPBY(INLINE_CACHE_ENTRIES_LOAD_ATTR);
+ frame->return_offset = 0;
  DISPATCH_INLINED(new_frame);
  }
 
@@ -2259,14 +2264,14 @@ dummy_func(
  DEOPT_IF(gen->gi_frame_state >= FRAME_EXECUTING, FOR_ITER);
  STAT_INC(FOR_ITER, hit);
  _PyInterpreterFrame *gen_frame = (_PyInterpreterFrame *)gen->gi_iframe;
- frame->yield_offset = oparg;
+ frame->return_offset = oparg;
  _PyFrame_StackPush(gen_frame, Py_NewRef(Py_None));
  gen->gi_frame_state = FRAME_EXECUTING;
  gen->gi_exc_state.previous_item = tstate->exc_info;
  tstate->exc_info = &gen->gi_exc_state;
- JUMPBY(INLINE_CACHE_ENTRIES_FOR_ITER + oparg);
- assert(next_instr->op.code == END_FOR ||
- next_instr->op.code == INSTRUMENTED_END_FOR);
+ JUMPBY(INLINE_CACHE_ENTRIES_FOR_ITER);
+ assert(next_instr[oparg].op.code == END_FOR ||
+ next_instr[oparg].op.code == INSTRUMENTED_END_FOR);
  DISPATCH_INLINED(gen_frame);
  }
 
@@ -2521,6 +2526,7 @@ dummy_func(
  goto error;
  }
  JUMPBY(INLINE_CACHE_ENTRIES_CALL);
+ frame->return_offset = 0;
  DISPATCH_INLINED(new_frame);
  }
  /* Callable is not a normal Python function */
@@ -2594,6 +2600,7 @@ dummy_func(
  // Manipulate stack directly since we leave using DISPATCH_INLINED().
  STACK_SHRINK(oparg + 2);
  JUMPBY(INLINE_CACHE_ENTRIES_CALL);
+ frame->return_offset = 0;
  DISPATCH_INLINED(new_frame);
  }
 
@@ -2631,6 +2638,7 @@ dummy_func(
  // Manipulate stack and cache directly since we leave using DISPATCH_INLINED().
  STACK_SHRINK(oparg + 2);
  JUMPBY(INLINE_CACHE_ENTRIES_CALL);
+ frame->return_offset = 0;
  DISPATCH_INLINED(new_frame);
  }
 

Python/ceval.c

@@ -672,7 +672,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
  _PyCode_CODE(tstate->interp->interpreter_trampoline);
  entry_frame.stacktop = 0;
  entry_frame.owner = FRAME_OWNED_BY_CSTACK;
- entry_frame.yield_offset = 0;
+ entry_frame.return_offset = 0;
  /* Push frame */
  entry_frame.previous = prev_cframe->current_frame;
  frame->previous = &entry_frame;
@@ -881,6 +881,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
  _PyInterpreterFrame *dying = frame;
  frame = cframe.current_frame = dying->previous;
  _PyEvalFrameClearAndPop(tstate, dying);
+ frame->return_offset = 0;
  if (frame == &entry_frame) {
  /* Restore previous cframe and exit */
  tstate->cframe = cframe.previous;