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):
        try:
            return number
        finally:
            continue

simple()
SEGV


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
[SEGV]

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.

Adding Serhiy since this change was introduced with issue32489.

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.

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

Raising an error can handle this because it is impossible to reach to return so we can declare this as an illegal syntax?

> 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?

I closed the my PR in favor of Serhiy's PR.

On Mon, Aug 12, 2019, 9:18 PM Serhiy Storchaka <report@bugs.python.org>
wrote:

>
> Change by Serhiy Storchaka <storchaka+cpython@gmail.com>:
>
>
> ----------
> pull_requests: +14953
> pull_request: https://github.com/python/cpython/pull/15230
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue37830>
> _______________________________________
>

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):
        	try:
            		return number
        	finally:
            		break
simple()
```

serhiy can you review the solution i found?

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:

  try:
     a = 2
  finally:
     a = 3
  print(a)
  # 3

and

  def f():
     try:
        return 5
     finally:
        return 7
  print(f())
  # 7

So it seems like the ideal solution to:

  def mult(thing):
      print(thing*2)
      return thing * 2

  def simple():
      for number in range(2):
          try:
              return mult(number)
          finally:
              continue
  print(simple())

would be:

  0
  2
  None

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):
        try:
            return number
        finally:
            if x:
                continue

print(simple(0))
print(simple(1))

It should print:

0
None


Ethan, your understanding is correct.

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):
            try:
                return number
            finally:
                break

simple()

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

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.

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.

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.

This is marked as a release blocker. The last beta is scheduled for Monday. Please decide how to proceed ASAP.

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

New changeset ef61c524ddeeb56da3858b86e349e7288d68178e by Serhiy Storchaka in branch 'master':
bpo-37830: Fix compilation of break and continue in finally. (GH-15320)
https://github.com/python/cpython/commit/ef61c524ddeeb56da3858b86e349e7288d68178e

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)
https://github.com/python/cpython/commit/ed146b52a3b6537689324e3bd9952055f9c6b43d

For forbidding 'break', 'continue' and 'return' in the 'finally' clause please open a topic on Python-Dev.