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.
This commit is contained in:
Mark Shannon 2023-04-13 16:19:07 +01:00 committed by GitHub
parent 4307feaddc
commit efb8a2553c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 455 additions and 414 deletions

View File

@ -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++) {

View File

@ -225,7 +225,22 @@ class GeneratorTest(unittest.TestCase):
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

View File

@ -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.

View File

@ -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);
}

View File

@ -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 @@ exit_unwind:
_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;

File diff suppressed because it is too large Load Diff