msg182880 - (view) |
Author: Xavier de Gaye (xdegaye) * |
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) * |
Date: 2013-02-24 17:31 |
Oops, it occurs too with python 2.7.
|
msg183254 - (view) |
Author: Xavier de Gaye (xdegaye) * |
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) * |
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) * |
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) * |
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) * |
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) * |
Date: 2018-02-25 12:49 |
Example in msg183254 doesn't crash Python 3.8.
|
msg312939 - (view) |
Author: Xavier de Gaye (xdegaye) * |
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) * |
Date: 2018-02-27 13:10 |
Added PR 5928 for the 3.7 branch.
|
msg313007 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2018-02-27 13:25 |
Are there any problems with rebasing it to master?
|
msg313009 - (view) |
Author: Xavier de Gaye (xdegaye) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
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) * |
Date: 2018-03-13 21:11 |
Thank you Xavier!
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:42 | admin | set | github: 61490 |
2018-03-13 21:11:32 | serhiy.storchaka | set | status: open -> closed resolution: fixed messages:
+ msg313779
stage: patch review -> resolved |
2018-03-13 21:06:17 | serhiy.storchaka | set | messages:
+ msg313778 |
2018-03-13 21:04:13 | serhiy.storchaka | set | versions:
+ Python 2.7, Python 3.6 |
2018-03-13 20:53:59 | xdegaye | set | messages:
+ msg313776 |
2018-03-13 20:50:06 | xdegaye | set | messages:
+ msg313775 |
2018-03-13 20:46:04 | xdegaye | set | pull_requests:
+ pull_request5874 |
2018-03-13 17:32:57 | serhiy.storchaka | set | messages:
+ msg313766 |
2018-03-13 17:31:35 | serhiy.storchaka | set | messages:
+ msg313765 |
2018-03-13 16:36:54 | xdegaye | set | pull_requests:
+ pull_request5870 |
2018-03-13 15:28:02 | xdegaye | set | messages:
+ msg313761 |
2018-03-13 10:12:19 | miss-islington | set | nosy:
+ miss-islington messages:
+ msg313737
|
2018-03-13 08:56:31 | serhiy.storchaka | link | issue33041 dependencies |
2018-03-13 08:54:10 | miss-islington | set | pull_requests:
+ pull_request5863 |
2018-03-13 08:52:38 | serhiy.storchaka | set | messages:
+ msg313730 |
2018-03-11 10:37:13 | xdegaye | set | messages:
+ msg313594 |
2018-03-08 10:09:57 | xdegaye | set | messages:
+ msg313432 |
2018-03-08 09:36:20 | serhiy.storchaka | set | messages:
+ msg313428 |
2018-02-28 07:03:34 | xdegaye | set | messages:
+ msg313051 |
2018-02-27 16:46:08 | xdegaye | set | messages:
+ msg313015 |
2018-02-27 14:54:31 | serhiy.storchaka | set | messages:
+ msg313012 |
2018-02-27 14:09:12 | xdegaye | set | messages:
+ msg313009 |
2018-02-27 13:25:35 | serhiy.storchaka | set | messages:
+ msg313007 |
2018-02-27 13:10:04 | xdegaye | set | messages:
+ msg313003 |
2018-02-27 13:07:15 | xdegaye | set | pull_requests:
+ pull_request5699 |
2018-02-26 17:29:59 | xdegaye | set | files:
+ 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:05 | serhiy.storchaka | set | messages:
+ msg312808 |
2018-02-25 10:28:57 | serhiy.storchaka | set | messages:
+ msg312797 |
2016-06-05 16:32:02 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka
stage: patch review |
2014-07-08 10:59:16 | vstinner | set | nosy:
+ vstinner
|
2014-07-08 10:58:00 | xdegaye | set | files:
+ 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:57 | xdegaye | set | messages:
+ msg183264 |
2013-03-01 15:23:51 | pitrou | set | nosy:
+ georg.brandl
|
2013-03-01 11:43:08 | xdegaye | set | files:
+ default.patch keywords:
+ patch messages:
+ msg183255
|
2013-03-01 11:29:09 | xdegaye | set | files:
+ jump.py nosy:
+ jcea, benjamin.peterson messages:
+ msg183254
|
2013-02-24 17:31:50 | xdegaye | set | messages:
+ msg182881 versions:
+ Python 2.7 |
2013-02-24 17:26:53 | xdegaye | create | |