homepage

On python 3.3 and the default branch, the jump from a 'return' fails although
the change to f_lineno is validated, see below.
This problem does not occur with python 2.7.
$ python return.py
> /tmp/return.py(8)<module>()
-> foo()
(Pdb) break 5
Breakpoint 1 at /tmp/return.py:5
(Pdb) continue
> /tmp/return.py(5)foo()
-> lineno = 5
(Pdb) step
--Return--
> /tmp/return.py(5)foo()->None
-> lineno = 5
(Pdb) jump 4
> /tmp/return.py(4)foo()->None
-> lineno = 4
(Pdb) where
 /tmp/return.py(8)<module>()
-> foo()
> /tmp/return.py(4)foo()->None
-> lineno = 4
(Pdb) step
--Return--
> /tmp/return.py(8)<module>()->None
-> foo()
(Pdb)

Oops, it occurs too with python 2.7.

Nosying Benjamin Peterson who knows frame_setlineno (issue 14612) and nosying
Jesús Cea Avión. Hoping they don't mind.
This problem occurs also when setting f_lineno from an exception debug event.
One may crash the interpreter (or get a "SystemError: unknown opcode") when the
return debug event is one that handles a yield statement. For example the
following test causes a segmentation fault on python 3.3.0:
$ python /tmp/jump.py
> /tmp/jump.py(7)<module>()
-> for i in gen():
(Pdb) step
--Call--
> /tmp/jump.py(1)gen()
-> def gen():
(Pdb) step
> /tmp/jump.py(2)gen()
-> for i in range(1):
(Pdb) step
> /tmp/jump.py(3)gen()
-> yield i
(Pdb) step
--Return--
> /tmp/jump.py(3)gen()->0
-> yield i
(Pdb) jump 2
> /tmp/jump.py(2)gen()->0
-> for i in range(1):
(Pdb) step
> /tmp/jump.py(8)<module>()
-> lineno = 8
(Pdb) step
> /tmp/jump.py(7)<module>()
-> for i in gen():
(Pdb) step
--Call--
> /tmp/jump.py(2)gen()->0
-> for i in range(1):
(Pdb) step
Segmentation fault

The proposed patch fixes the problem:
* f_lineno cannot be set now from an exception trace function or from a return
 trace function.
* The broken arithmetic involving a null pointer (f->f_stacktop, at the end of
 frame_setlineno when popping blocks that are being jumped from and the
 function is invoked from an exception trace function or a return trace
 function), is fixed now.
* Blocks cannot be popped now in frame_setlineno (the cause of the crash) when
 tracing a yield.
To summarize the proposed fixes on f_lineno accessors:
* setter: f_lineno can only be set in the frame that is being traced (issue
 17277) and from within a line trace function (current issue).
* getter: f_lineno is valid in the frame being traced (issue 17277) and:
 + not from within a call trace function (i.e. when f->f_trace == NULL)
 + from within a line trace function
 + and from within an exception or return trace function. There is a corner
 case here when returning to a frame that was not being traced previously
 (its f_lineno is not valid) and that has had its local trace function
 f_trace set from within a trace function in one of its callees (the fix of
 issue 13183 does that) and when the first trace function invoked on
 returning to that frame is an exception or a return trace function: this is
 correctly handled by the f_trace setter that makes sure that f_lineno is
 accurate when setting f_trace (and an extension module implementing tracing,
 must ensure that f_lineno is accurate when setting f_trace).

Must add for completeness that f_lineno must be valid within a return trace
function for the reasons explained in the last paragraph of
Objects/lnotab_notes.txt.

Python 3.5 is still crashing with this test:
$ python jump.py
> jump.py(7)<module>()
-> for i in gen():
(Pdb) break 3
Breakpoint 1 at jump.py:3
(Pdb) continue
> jump.py(3)gen()
-> yield i
(Pdb) step
--Return--
> jump.py(3)gen()->0
-> yield i
(Pdb) jump 2
> jump.py(2)gen()->0
-> for i in range(1):
(Pdb) continue
Segmentation fault (core dumped)
It is true that frame_setlineno() assumes incorrectly that f->f_stacktop is not NULL, but the reason of the crash or of the "SystemError: unknown opcode" is that PyEval_EvalFrameEx() expects on its invocation f->f_lasti to refer to the index of the last instruction executed and sets (and assumes this instruction does not have argument) 'next_instr' accordingly to the next byte, while by jumping to line 2 we set f->f_lasti to zero and 'SETUP_LOOP', the opcode at this index, has an argument.
The attached patch is a slight improvement over the last one.

Do you mind to create a PR from your patch Xavier? The code of the f_lineno setter was changed in the master branch, so that it is better to create a PR for the 3.7 branch and later port it to master.

Example in msg183254 doesn't crash Python 3.8.

Those are the changes with the current behavior from the behavior in 3.5 observed at the time of the initial bug report:
 python 3.7:
 return.py Unchanged.
 exception.py After a jump from the 'exception' event into the previous statement, the ensuing 'step' command triggers a trace 'return' event.
 jump.py The sequence of trace events is unchanged and Python aborts now at the last 'step' command with:
 python: Python/ceval.c:1083: _PyEval_EvalFrameDefault: Assertion `STACK_LEVEL() <= co->co_stacksize' failed.
 python 3.8:
 return.py Unchanged.
 exception.py Unchanged from 3.7.
 jump.py The sequence of trace events is unchanged and the last 'step' command prints now the cryptic error msg:
 TypeError: 'NoneType' object is not callable
 > /path/to/jump.py(2)gen()->0
 -> for i in range(1):
Applying the last patch fixes both 3.7 and 3.8. I can build a PR from this patch. An explanation should be given for the behavior of 3.7 and 3.8 in the jump.py case.

Added PR 5928 for the 3.7 branch.

Are there any problems with rebasing it to master?

Yes, rebase fails merging two changes in Objects/frameobject.c.
I can write another PR for master if you want.

Usually we create a PR for master (unless the issue is gone here) and backport it to other branches. In this case it may be better to start from 3.7, but it is worth to look at the patch against master for the case if the change can written in the form that minimizes difference between 3.7 and master.

Actually
 $ git rebase --onto master 3.7 bpo-17288
fails with one single change in Objects/frameobject.c and one simply needs to (as I have tested it)
remove all the three confict markers in this file and remove the following two lines that were enclosed by these markers:
 min_addr = Py_MIN(new_lasti, f->f_lasti);
 max_addr = Py_MAX(new_lasti, f->f_lasti);
then run:
 $ git add Objects/frameobject.c
 $ git rebase --continue
 $ git co master; git merge bpo-17288 

xdegaye wrote:
> An explanation should be given for the behavior of 3.7 and 3.8 in the jump.py case.
In 3.7 when running jump.py as in msg183254, Python aborts with a frame stack overflow. The reason is that when the generator is resumed after the jump, its send() method is called with an argument and pushes the argument on the frame stack (i.e. f->f_stacktop - f->f_valuestack == 1 in gen_send_ex() before the call to PyEval_EvalFrameEx()). This argument is supposed to be popped by the first instruction executed by the generator which is expected to be YIELD_VALUE but, because of the jump, f->f_lasti is now 0 and the send() argument is not popped. Hence the stack overflow.
When LLTRACE is undefined in ceval.c, stack oveflow checking is disabled. I have checked with gdb that, in that case, when YIELD_VALUE is about to be executed then STACK_LEVEL() is 3 instead of 1 and therefore YIELD_VALUE does not pop the right value from the stack. The stack is indeed corrupted.
So there are two reasons for forbiddig to jump from a yield statement:
* the ceval loop has exited and the jump is not allowing the user to jump to another line
* after the jump, resuming a generator corrupts the frame stack

Thank you Xavier. I had pleasure from reviewing your patch. But please update tests for using the jump_test() decorator.

Sorry, for some reason github did not send me the emails of your review and I did not think about checking the PR :-(
Thanks for your review, I will work on it shortly.

Serhiy, I have updated the tests for using the jump_test() decorator and replied to your comment about RETURN_VALUE.

New changeset e32bbaf376a09c149fa7c7f2919d7c9ce4e2a055 by Serhiy Storchaka (xdegaye) in branch '3.7':
[3.7] bpo-17288: Prevent jumps from 'return' and 'exception' trace events. (GH-5928)
https://github.com/python/cpython/commit/e32bbaf376a09c149fa7c7f2919d7c9ce4e2a055

New changeset cf61a81f1d600064be6dd43896afcf5f976de9b0 by Miss Islington (bot) in branch '3.6':
[3.7] bpo-17288: Prevent jumps from 'return' and 'exception' trace events. (GH-5928)
https://github.com/python/cpython/commit/cf61a81f1d600064be6dd43896afcf5f976de9b0

In PR 5928 Serhiy Storchaka wrote:
> Great! Could you please create a backport to master?
Yes.
I prefer to answer in the issue as I am still not getting any mail from github.

New changeset b8e9d6c5cd44ebc9c462fea9ad1bc5d0b970e28a by Serhiy Storchaka (xdegaye) in branch 'master':
bpo-17288: Prevent jumps from 'return' and 'exception' trace events. (GH-6107)
https://github.com/python/cpython/commit/b8e9d6c5cd44ebc9c462fea9ad1bc5d0b970e28a

Thank you! Do you mind to make a backport to 2.7 (if this makes a sense)?

The conditions used for the fix are still the same in 2.7: f->f_trace == NULL on a call event, f->f_lasti == -1 in a new frame and f->f_stacktop == NULL in a trace function with a non-line event except for yield statements.
There are minor differences: the YIELD_FROM opcode does not exist and function.__code__ is written as function.func_code.
Added a PR for 2.7.

FWIW I now get github notifications, the problem was some bad configuration of my github profile (my fault).

New changeset baca85fcc7cdf70af4a76ea0966d4842c173de1a by Serhiy Storchaka (xdegaye) in branch '2.7':
[2.7] bpo-17288: Prevent jumps from 'return' and 'exception' trace events. (GH-6111)
https://github.com/python/cpython/commit/baca85fcc7cdf70af4a76ea0966d4842c173de1a

Thank you Xavier!