[issue37830] continue in finally with return in try results with segfault

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

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.

--
keywords: +3.8regression -patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37830] continue in finally with return in try results with segfault

[Ethan Furman]

Ethan Furman  added the comment:

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

--
nosy: +ethan.furman

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37830] continue in finally with return in try results with segfault

[Batuhan]

Batuhan  added the comment:

serhiy can you review the solution i found?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37830] continue in finally with return in try results with segfault

[Batuhan]

Change by Batuhan :


--
pull_requests: +14968
pull_request: https://github.com/python/cpython/pull/15247

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37830] continue in finally with return in try results with segfault

[ppperry]

ppperry  added the comment:

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()
```

--
nosy: +ppperry

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37830] continue in finally with return in try results with segfault

[Batuhan]

Batuhan  added the comment:

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

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

>
> Change by Serhiy Storchaka :
>
>
> --
> pull_requests: +14953
> pull_request: https://github.com/python/cpython/pull/15230
>
> ___
> Python tracker 
> 
> ___
>

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37830] continue in finally with return in try results with segfault

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
pull_requests: +14953
pull_request: https://github.com/python/cpython/pull/15230

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37830] continue in finally with return in try results with segfault

[STINNER Victor]

STINNER Victor  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37830] continue in finally with return in try results with segfault

[Pablo Galindo Salgado]

Change by Pablo Galindo Salgado :


--
nosy: +pablogsal

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37830] continue in finally with return in try results with segfault

[Batuhan]

Batuhan  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37830] continue in finally with return in try results with segfault

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37830] continue in finally with return in try results with segfault

[STINNER Victor]

Change by STINNER Victor :


--
nosy: +vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37830] continue in finally with return in try results with segfault

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
nosy: +lukasz.langa
priority: normal -> release blocker

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37830] continue in finally with return in try results with segfault

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

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 ", line 1>:
  2   0 LOAD_GLOBAL  0 (range)
  2 LOAD_CONST   1 (2)
  4 CALL_FUNCTION1
  6 GET_ITER
>>8 FOR_ITER24 (to 34)
 10 STORE_FAST   0 (number)

  3  12 SETUP_FINALLY   12 (to 26)

  4  14 LOAD_FAST0 (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_ABSOLUTE8
 30 END_FINALLY
 32 JUMP_ABSOLUTE8
>>   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.

--
assignee:  -> serhiy.storchaka
nosy: +Mark.Shannon

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37830] continue in finally with return in try results with segfault

[Karthikeyan Singaravelan]

Karthikeyan Singaravelan  added the comment:

Adding Serhiy since this change was introduced with issue32489.

--
nosy: +serhiy.storchaka, xtreak

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37830] continue in finally with return in try results with segfault

[Batuhan]

Change by Batuhan :


--
versions: +Python 3.8, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37830] continue in finally with return in try results with segfault

[Eric V. Smith]

Change by Eric V. Smith :


--
nosy: +eric.smith

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37830] continue in finally with return in try results with segfault

[Eric V. Smith]

Change by Eric V. Smith :


--
components: +Interpreter Core
type:  -> crash

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37830] continue in finally with return in try results with segfault

[Batuhan]

Change by Batuhan :


--
keywords: +patch
pull_requests: +14948
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/15221

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37830] continue in finally with return in try results with segfault

[Batuhan]

New submission from Batuhan :

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 
LOAD_CONST  1   
>>> STACK_SIZE  1   
>>> push2   
CALL_FUNCTION   1   
>>> STACK_SIZE  2   
>>> ext_pop 2   
>>> ext_pop  
>>> pushrange(0, 2) 
GET_ITERNone
>>> STACK_SIZE  1   
FOR_ITER24  
>>> STACK_SIZE  1   
>>> push0   
STORE_FAST  0   
>>> STACK_SIZE  2   
>>> pop 0   
SETUP_FINALLY   12  
>>> STACK_SIZE  1   
LOAD_FAST   0   
>>> STACK_SIZE  1   
>>> push0   
POP_BLOCK   None
>>> STACK_SIZE  2   
CALL_FINALLY6   
>>> STACK_SIZE  2   
>>> push20  
POP_FINALLY 0   
>>> STACK_SIZE  3   
>>> pop 20  
JUMP_ABSOLUTE   8   
>>> STACK_SIZE  2   
FOR_ITER24  
>>> 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.

--
messages: 349450
nosy: BTaskaya
priority: normal
severity: normal
status: open
title: continue in finally with return in try results with segfault

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com