Title: continue and break in finally with return in try results with segfault
Type: crash Stage: resolved
Components: Interpreter Core Versions: Python 3.9, Python 3.8
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: BTaskaya, Mark.Shannon, eric.smith, ethan.furman, lukasz.langa, ncoghlan, pablogsal, ppperry, serhiy.storchaka, vstinner, xtreak
Priority: release blocker Keywords: 3.8regression, patch

Created on 2019-08-12 09:33 by BTaskaya, last changed 2019-08-24 10:57 by serhiy.storchaka. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 15221 closed BTaskaya, 2019-08-12 09:36
PR 15230 closed serhiy.storchaka, 2019-08-12 18:18
PR 15247 closed BTaskaya, 2019-08-13 06:51
PR 15320 merged serhiy.storchaka, 2019-08-17 16:13
PR 15456 merged serhiy.storchaka, 2019-08-24 10:16
Messages (20)
msg349450 - (view) Author: Batuhan (BTaskaya) * Date: 2019-08-12 09:33
When you try to put a return statement with the iterated value inside of try/finally and if you have continue in finally it will result with segfault.

def simple():
    for number in range(2):
            return number


My first debugs shows TOS is the next value int(1) when it comes to FOR_ITER, instead of the range instance. 
So when python try to call next() with (*iter->ob_type->tp_iternext)(iter) in FOR_ITER it gets a non iterator, int(1). 

Adding an assert can prove it, 
python: Python/ceval.c:3198: _PyEval_EvalFrameDefault: Assertion `PyIter_Check(iter)' failed.

For seeing how stack changed i enabled lltrace and formatted it little bit;

>>> STACK_SIZE          0                       
LOAD_GLOBAL             0                       
>>> STACK_SIZE          0                       
>>> push                <class 'range'>         
LOAD_CONST              1                       
>>> STACK_SIZE          1                       
>>> push                2                       
CALL_FUNCTION           1                       
>>> STACK_SIZE          2                       
>>> ext_pop             2                       
>>> ext_pop             <class 'range'>         
>>> push                range(0, 2)             
GET_ITER                None                    
>>> STACK_SIZE          1                       
FOR_ITER                24                      
>>> STACK_SIZE          1                       
>>> push                0                       
STORE_FAST              0                       
>>> STACK_SIZE          2                       
>>> pop                 0                       
SETUP_FINALLY           12                      
>>> STACK_SIZE          1                       
LOAD_FAST               0                       
>>> STACK_SIZE          1                       
>>> push                0                       
POP_BLOCK               None                    
>>> STACK_SIZE          2                       
CALL_FINALLY            6                       
>>> STACK_SIZE          2                       
>>> push                20                      
POP_FINALLY             0                       
>>> STACK_SIZE          3                       
>>> pop                 20                      
JUMP_ABSOLUTE           8                       
>>> STACK_SIZE          2                       
FOR_ITER                24                      
>>> STACK_SIZE          2

And the oddity is STACK_SIZE should be 1 before the FOR_ITER but it is 2, then i said why dont i try the SECOND() as iter, and it worked. It means an instruction is pushing the value of previous iteration.

There are 3 things we can do;
=> raise a RuntimeError if TOS isn't an iterator (IMHO we should do that)
=> check if try/finally created inside of a function and an iterator. then check inside of try if a return happens with the iterated value and if so set preserve_tos value to false. 
=> dont allow continue in finally

I want to fix this, and i prefer the first one. If you have any suggestion, i am open.
msg349451 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python triager) Date: 2019-08-12 10:34
Adding Serhiy since this change was introduced with issue32489.
msg349456 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-08-12 11:32
Thank you for your report Batuhan.

But PR 15221 is not proper way to fix it. It will not work when return an iterable. This issue should be fixed at compiler level. The generated code is:

Disassembly of <code object simple at 0x7f2195c7d450, file "<stdin>", line 1>:
  2           0 LOAD_GLOBAL              0 (range)
              2 LOAD_CONST               1 (2)
              4 CALL_FUNCTION            1
              6 GET_ITER
        >>    8 FOR_ITER                24 (to 34)
             10 STORE_FAST               0 (number)

  3          12 SETUP_FINALLY           12 (to 26)

  4          14 LOAD_FAST                0 (number)
             16 POP_BLOCK
             18 CALL_FINALLY             6 (to 26)
             20 ROT_TWO
             22 POP_TOP
             24 RETURN_VALUE

  6     >>   26 POP_FINALLY              0
             28 JUMP_ABSOLUTE            8
             30 END_FINALLY
             32 JUMP_ABSOLUTE            8
        >>   34 LOAD_CONST               0 (None)
             36 RETURN_VALUE

The return statement pushes a value at the stack (offset 14) and the continue statement jumps to the beginning of the loop (offset 28). The stack is not balanced here.
msg349458 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-08-12 11:56
It looks to me, that it is not possible to solve this with the current bytecode. Perhaps the only way to fix a crash is to revert issue32489.

Implementing Mark's idea about inlining the code of a "finally" block may help to solve the issue with "continue" in "finally".
msg349460 - (view) Author: Batuhan (BTaskaya) * Date: 2019-08-12 12:01
Raising an error can handle this because it is impossible to reach to return so we can declare this as an illegal syntax?
msg349462 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-08-12 12:35
> It looks to me, that it is not possible to solve this with the current bytecode. Perhaps the only way to fix a crash is to revert issue32489.

That sounds like a good compromise to unblock next Python 3.8 release. issue32489 can wait another release (Python 3.9), no?
msg349483 - (view) Author: Batuhan (BTaskaya) * Date: 2019-08-12 18:26
I closed the my PR in favor of Serhiy's PR.

On Mon, Aug 12, 2019, 9:18 PM Serhiy Storchaka <>

> Change by Serhiy Storchaka <>:
> ----------
> pull_requests: +14953
> pull_request:
> _______________________________________
> Python tracker <>
> <>
> _______________________________________
msg349513 - (view) Author: (ppperry) Date: 2019-08-13 01:22
Unfortunately, there's a similar bug for `break` in a finally inside two nested loops, so just re-banning `continue` won't fix the crash.
The code below segfaults:
def simple():
    for number in range(2):
    	for number in range(2):
            		return number
msg349528 - (view) Author: Batuhan (BTaskaya) * Date: 2019-08-13 06:52
serhiy can you review the solution i found?
msg349570 - (view) Author: Ethan Furman (ethan.furman) * (Python committer) Date: 2019-08-13 15:24
My apologies if I missed something, but do we have a consensus on the desired solution?

My understanding of `try/finally` is that whatever happens in the `finally` clause should:

- always happen
- win any conflicts with `try` clause

For example:

     a = 2
     a = 3
  # 3


  def f():
        return 5
        return 7
  # 7

So it seems like the ideal solution to:

  def mult(thing):
      return thing * 2

  def simple():
      for number in range(2):
              return mult(number)

would be:

msg349578 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-08-13 16:09
Batuhan, unfortunately your second solution does not work too, because continue and break can be conditional. For example:

def simple(x):
    for number in range(2):
            return number
            if x:


It should print:


Ethan, your understanding is correct.
msg349579 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-08-13 16:18
Note, that we have a regression in 3.8. There is a use case for "break" in "finally", and such code is even used in the stdlib. And who know in what third-party code it is used. In specific circumstances (see msg349513) it now can cause a crash. Other example:

import contextlib
def simple():
    with contextlib.nullcontext():
        for number in range(2):
                return number


It just raise an exception in 3.8, not crash:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 7, in simple
TypeError: 'range_iterator' object is not callable
msg349605 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-08-13 19:03
There may be a solution that does not require a significant change to the code generator, but small changes in many places with keeping the general structure. But it is a difficult task, so it takes some time. Don't worry, I'm working on it.
msg349693 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2019-08-14 13:02
Even though it doesn't fully resolve this crash, I'd still like us to consider reverting the change to allow "continue" in try/finally blocks.

It doesn't seem to have a compelling practical motivation behind it (beyond the fact that it's nice not to impose arbitrary restriction on users), and even if it's feasible in CPython now, it still creates a non-trivial amount of work for other Python implementations that are trying to remain consistent with what CPython allows.
msg349901 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-08-17 16:35
PR 15320 fixes a regression by changing the compiler. None is now always pushed on the stack before entering a try...finally block. The "return" statement with a non-constant value replaces it, so the stack is balanced.
msg350289 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2019-08-23 14:04
This is marked as a release blocker. The last beta is scheduled for Monday. Please decide how to proceed ASAP.
msg350300 - (view) Author: Pablo Galindo Salgado (pablogsal) * (Python committer) Date: 2019-08-23 14:41
I think PR 15320 should be merge as soon as possible at least to fix the segfault and stop the release blocker. Anything else can wait IMHO
msg350359 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-08-24 10:11
New changeset ef61c524ddeeb56da3858b86e349e7288d68178e by Serhiy Storchaka in branch 'master':
bpo-37830: Fix compilation of break and continue in finally. (GH-15320)
msg350364 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-08-24 10:41
New changeset ed146b52a3b6537689324e3bd9952055f9c6b43d by Serhiy Storchaka in branch '3.8':
[3.8] bpo-37830: Fix compilation of break and continue in finally. (GH-15320) (GH-15456)
msg350365 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-08-24 10:57
For forbidding 'break', 'continue' and 'return' in the 'finally' clause please open a topic on Python-Dev.
Date User Action Args
2019-08-30 11:07:08serhiy.storchakalinkissue37987 superseder
2019-08-24 10:57:26serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg350365

stage: patch review -> resolved
2019-08-24 10:41:56serhiy.storchakasetmessages: + msg350364
2019-08-24 10:16:26serhiy.storchakasetpull_requests: + pull_request15151
2019-08-24 10:11:55serhiy.storchakasetmessages: + msg350359
2019-08-23 14:41:03pablogsalsetmessages: + msg350300
2019-08-23 14:04:41lukasz.langasetmessages: + msg350289
2019-08-17 16:35:48serhiy.storchakasetmessages: + msg349901
2019-08-17 16:13:28serhiy.storchakasetkeywords: + patch
stage: patch review
pull_requests: + pull_request15039
2019-08-14 13:02:18ncoghlansetnosy: + ncoghlan
messages: + msg349693
2019-08-13 19:03:12serhiy.storchakasetmessages: + msg349605
2019-08-13 16:18:41serhiy.storchakasettitle: continue in finally with return in try results with segfault -> continue and break in finally with return in try results with segfault
messages: + msg349579
stage: patch review -> (no value)
2019-08-13 16:09:46serhiy.storchakasetkeywords: + 3.8regression, - patch

messages: + msg349578
2019-08-13 15:24:08ethan.furmansetnosy: + ethan.furman
messages: + msg349570
2019-08-13 06:52:51BTaskayasetmessages: + msg349528
2019-08-13 06:51:42BTaskayasetpull_requests: + pull_request14968
2019-08-13 01:22:41ppperrysetnosy: + ppperry
messages: + msg349513
2019-08-12 18:26:40BTaskayasetmessages: + msg349483
2019-08-12 18:18:36serhiy.storchakasetpull_requests: + pull_request14953
2019-08-12 12:35:11vstinnersetmessages: + msg349462
2019-08-12 12:25:51pablogsalsetnosy: + pablogsal
2019-08-12 12:01:26BTaskayasetmessages: + msg349460
2019-08-12 11:56:22serhiy.storchakasetmessages: + msg349458
2019-08-12 11:44:39vstinnersetnosy: + vstinner
2019-08-12 11:32:54serhiy.storchakasetpriority: normal -> release blocker
nosy: + lukasz.langa
2019-08-12 11:32:42serhiy.storchakasetassignee: serhiy.storchaka

messages: + msg349456
nosy: + Mark.Shannon
2019-08-12 10:34:52xtreaksetnosy: + serhiy.storchaka, xtreak
messages: + msg349451
2019-08-12 10:09:45BTaskayasetversions: + Python 3.8, Python 3.9
2019-08-12 09:57:51eric.smithsetnosy: + eric.smith
2019-08-12 09:57:35eric.smithsettype: crash
components: + Interpreter Core
2019-08-12 09:36:56BTaskayasetkeywords: + patch
stage: patch review
pull_requests: + pull_request14948
2019-08-12 09:33:33BTaskayacreate