classification
Title: cannot jump from a 'return' or 'exception' trace event
Type: crash Stage: resolved
Components: Interpreter Core Versions: Python 3.8, Python 3.7, Python 3.6, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: benjamin.peterson, georg.brandl, jcea, miss-islington, serhiy.storchaka, vstinner, xdegaye
Priority: normal Keywords: patch

Created on 2013-02-24 17:26 by xdegaye, last changed 2018-03-13 21:11 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
return.py xdegaye, 2013-02-24 17:26
jump.py xdegaye, 2013-03-01 11:29
default.patch xdegaye, 2013-03-01 11:43 review
default_2.patch xdegaye, 2014-07-08 10:57 review
exception.py xdegaye, 2018-02-26 17:29
Pull Requests
URL Status Linked Edit
PR 5928 merged xdegaye, 2018-02-27 13:07
PR 6100 merged miss-islington, 2018-03-13 08:54
PR 6107 merged xdegaye, 2018-03-13 16:36
PR 6111 merged xdegaye, 2018-03-13 20:46
Messages (27)
msg182880 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2013-02-24 17:26
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)
msg182881 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2013-02-24 17:31
Oops, it occurs too with python 2.7.
msg183254 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2013-03-01 11:29
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
msg183255 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2013-03-01 11:43
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).
msg183264 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2013-03-01 16:17
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.
msg222555 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2014-07-08 10:57
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.
msg312797 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-02-25 10:28
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.
msg312808 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-02-25 12:49
Example in msg183254 doesn't crash Python 3.8.
msg312939 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2018-02-26 17:29
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.
msg313003 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2018-02-27 13:10
Added PR 5928 for the 3.7 branch.
msg313007 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-02-27 13:25
Are there any problems with rebasing it to master?
msg313009 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2018-02-27 14:09
Yes, rebase fails merging two changes in Objects/frameobject.c.
I can write another PR for master if you want.
msg313012 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-02-27 14:54
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.
msg313015 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2018-02-27 16:46
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
msg313051 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2018-02-28 07:03
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
msg313428 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-03-08 09:36
Thank you Xavier. I had pleasure from reviewing your patch. But please update tests for using the jump_test() decorator.
msg313432 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2018-03-08 10:09
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.
msg313594 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2018-03-11 10:37
Serhiy, I have updated the tests for using the jump_test() decorator and replied to your comment about RETURN_VALUE.
msg313730 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-03-13 08:52
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
msg313737 - (view) Author: miss-islington (miss-islington) Date: 2018-03-13 10:12
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
msg313761 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2018-03-13 15:28
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.
msg313765 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-03-13 17:31
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
msg313766 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-03-13 17:32
Thank you! Do you mind to make a backport to 2.7 (if this makes a sense)?
msg313775 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2018-03-13 20:50
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.
msg313776 - (view) Author: Xavier de Gaye (xdegaye) * (Python triager) Date: 2018-03-13 20:53
FWIW I now get github notifications, the problem was some bad configuration of my github profile (my fault).
msg313778 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-03-13 21:06
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
msg313779 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-03-13 21:11
Thank you Xavier!
History
Date User Action Args
2018-03-13 21:11:32serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg313779

stage: patch review -> resolved
2018-03-13 21:06:17serhiy.storchakasetmessages: + msg313778
2018-03-13 21:04:13serhiy.storchakasetversions: + Python 2.7, Python 3.6
2018-03-13 20:53:59xdegayesetmessages: + msg313776
2018-03-13 20:50:06xdegayesetmessages: + msg313775
2018-03-13 20:46:04xdegayesetpull_requests: + pull_request5874
2018-03-13 17:32:57serhiy.storchakasetmessages: + msg313766
2018-03-13 17:31:35serhiy.storchakasetmessages: + msg313765
2018-03-13 16:36:54xdegayesetpull_requests: + pull_request5870
2018-03-13 15:28:02xdegayesetmessages: + msg313761
2018-03-13 10:12:19miss-islingtonsetnosy: + miss-islington
messages: + msg313737
2018-03-13 08:56:31serhiy.storchakalinkissue33041 dependencies
2018-03-13 08:54:10miss-islingtonsetpull_requests: + pull_request5863
2018-03-13 08:52:38serhiy.storchakasetmessages: + msg313730
2018-03-11 10:37:13xdegayesetmessages: + msg313594
2018-03-08 10:09:57xdegayesetmessages: + msg313432
2018-03-08 09:36:20serhiy.storchakasetmessages: + msg313428
2018-02-28 07:03:34xdegayesetmessages: + msg313051
2018-02-27 16:46:08xdegayesetmessages: + msg313015
2018-02-27 14:54:31serhiy.storchakasetmessages: + msg313012
2018-02-27 14:09:12xdegayesetmessages: + msg313009
2018-02-27 13:25:35serhiy.storchakasetmessages: + msg313007
2018-02-27 13:10:04xdegayesetmessages: + msg313003
2018-02-27 13:07:15xdegayesetpull_requests: + pull_request5699
2018-02-26 17:29:59xdegayesetfiles: + exception.py

title: cannot jump from a return after setting f_lineno -> cannot jump from a 'return' or 'exception' trace event
messages: + msg312939
versions: + Python 3.7, Python 3.8, - Python 3.5
2018-02-25 12:49:05serhiy.storchakasetmessages: + msg312808
2018-02-25 10:28:57serhiy.storchakasetmessages: + msg312797
2016-06-05 16:32:02serhiy.storchakasetnosy: + serhiy.storchaka

stage: patch review
2014-07-08 10:59:16vstinnersetnosy: + vstinner
2014-07-08 10:58:00xdegayesetfiles: + default_2.patch
type: behavior -> crash
messages: + msg222555

versions: + Python 3.5, - Python 2.7, Python 3.3, Python 3.4
2013-03-01 16:17:57xdegayesetmessages: + msg183264
2013-03-01 15:23:51pitrousetnosy: + georg.brandl
2013-03-01 11:43:08xdegayesetfiles: + default.patch
keywords: + patch
messages: + msg183255
2013-03-01 11:29:09xdegayesetfiles: + jump.py
nosy: + jcea, benjamin.peterson
messages: + msg183254

2013-02-24 17:31:50xdegayesetmessages: + msg182881
versions: + Python 2.7
2013-02-24 17:26:53xdegayecreate