Skip to content

Exception handling ignored in minimal while loop #108214

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
2 tasks done
pan324 opened this issue Aug 21, 2023 · 11 comments
Open
2 tasks done

Exception handling ignored in minimal while loop #108214

pan324 opened this issue Aug 21, 2023 · 11 comments
Labels
3.11 only security fixes 3.12 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@pan324
Copy link
Contributor

pan324 commented Aug 21, 2023

Bug report

Checklist

  • I am confident this is a bug in CPython, not a bug in a third-party project
  • I have searched the CPython issue tracker,
    and am confident this bug has not been reported before

CPython versions tested on:

3.11, 3.12

Operating systems tested on:

Linux, Windows

Output from running 'python -VV' on the command line:

No response

A clear and concise description of the bug:

Consider the following codeblock that works correctly when interrupted by ctrl+c.

try:
    while 1: 
        pass
except KeyboardInterrupt:
    print("Exception handling works!")
Exception handling works!

Now we push the pass into the same line and suddenly our try-except is ignored entirely:

try:
    while 1: pass
except KeyboardInterrupt:
    print("Exception handling works!")
Traceback (most recent call last):
  File "/home/panta/issue.py", line 2, in <module>
    while 1: pass
KeyboardInterrupt

This was quite a surprise to me because I naively expected both approaches to emit the same bytecode (except with different line info).

@pan324 pan324 added the type-bug An unexpected behavior, bug, or error label Aug 21, 2023
@iritkatriel
Copy link
Member

I can't reproduce:

iritkatriel@Irits-MBP python % cat yy.py 
try:
    while 1: pass
except KeyboardInterrupt:
    print("Exception handling works!")
iritkatriel@Irits-MBP cpython % ./python.exe yy.py
^CException handling works!
iritkatriel@Irits-MBP cpython% 

@ericvsmith
Copy link
Member

I can't reproduce either, on Windows using 3.11.4. And dis.dis shows the same output for both.

@pan324
Copy link
Contributor Author

pan324 commented Aug 21, 2023

Well this is weird:

Python 3.11.4 (tags/v3.11.4:d2340ef, Jun  7 2023, 05:45:37) [MSC v.1934 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import dis
>>> dis.dis("while 1: pass")
  0           0 RESUME                   0
  1     >>    2 JUMP_BACKWARD            1 (to 2)
>>> dis.dis("while 1:\n pass")
  0           0 RESUME                   0
  1           2 NOP
  2     >>    4 NOP
  1           6 JUMP_BACKWARD            2 (to 4)

NB: I have checked 3.13 and it does not seem to have the interrupt issue anymore (even on my system). The interrupt issue also does not exist in 3.10 or 3.9. However the bytecode has NOPs starting in Python 3.10, including 3.13.

@hugovk hugovk added the pending The issue will be closed if no feedback is provided label Aug 21, 2023
@chgnrdv
Copy link
Contributor

chgnrdv commented Aug 21, 2023

I can reproduce it on my Debian machine with 3.11.4.

$ python -VV
Python 3.11.4 (main, Jun 18 2023, 21:24:38) [GCC 10.2.1 20210110]
$ uname -a
Linux machine 5.10.0-25-amd64 #1 SMP Debian 5.10.191-1 (2023-08-16) x86_64 GNU/Linux

Disassembly:

$ python -m dis repro0.py
  0           0 RESUME                   0

  1           2 NOP

  2           4 NOP

  3     >>    6 NOP

  2           8 JUMP_BACKWARD            2 (to 6)
        >>   10 PUSH_EXC_INFO

  4          12 LOAD_NAME                0 (KeyboardInterrupt)
             14 CHECK_EXC_MATCH
             16 POP_JUMP_FORWARD_IF_FALSE    15 (to 48)
             18 POP_TOP

  5          20 PUSH_NULL
             22 LOAD_NAME                1 (print)
             24 LOAD_CONST               1 ('Exception handling works!')
             26 PRECALL                  1
             30 CALL                     1
             40 POP_TOP
             42 POP_EXCEPT
             44 LOAD_CONST               2 (None)
             46 RETURN_VALUE

  4     >>   48 RERAISE                  0
        >>   50 COPY                     3
             52 POP_EXCEPT
             54 RERAISE                  1
ExceptionTable:
  4 to 8 -> 10 [0]
  10 to 40 -> 50 [1] lasti
  48 to 48 -> 50 [1] lasti
$ python -m dis repro1.py
  0           0 RESUME                   0

  1           2 NOP

  2     >>    4 JUMP_BACKWARD            1 (to 4)
        >>    6 PUSH_EXC_INFO

  3           8 LOAD_NAME                0 (KeyboardInterrupt)
             10 CHECK_EXC_MATCH
             12 POP_JUMP_FORWARD_IF_FALSE    15 (to 44)
             14 POP_TOP

  4          16 PUSH_NULL
             18 LOAD_NAME                1 (print)
             20 LOAD_CONST               1 ('Exception handling works!')
             22 PRECALL                  1
             26 CALL                     1
             36 POP_TOP
             38 POP_EXCEPT
             40 LOAD_CONST               2 (None)
             42 RETURN_VALUE

  3     >>   44 RERAISE                  0
        >>   46 COPY                     3
             48 POP_EXCEPT
             50 RERAISE                  1
ExceptionTable:
  4 to 4 -> 6 [0]
  6 to 36 -> 46 [1] lasti
  44 to 44 -> 46 [1] lasti

@carljm
Copy link
Member

carljm commented Aug 21, 2023

The extra NOPs are expected, as they are necessary to preserve the line info correctly. They shouldn't result in the try/except being ignored. So a "repro" here should be "can (consistently, and even if waiting some time first) Ctrl-C without the except handler running," not just a difference in bytecode.

The bytecode looks correct in both cases: the bytecodes included in the loop (covered by the JUMP_BACKWARD) are within the range covered by the handler in the exception table in both cases. So the only way to skip the exception handler should be to hit Ctrl-C "too soon" (before the loop is even entered.)

@sweeneyde
Copy link
Member

I reproduced the bug (consistently never printing Exception handling works!) at the tag v3.11.0a7 on Windows debug builds, bisected forward to see it fixed only recently by e586211 (which I notice moved CHECK_EVAL_BREAKER() from the bottom to the top of JUMP_BACKWARD). Bisecting backward, it appears that this was introduced way back at adcd220.

@ericvsmith
Copy link
Member

@markshannon

@carljm
Copy link
Member

carljm commented Aug 21, 2023

I can also repro on Linux debug build, latest 3.12 branch, (47f60c3). Because JUMP_BACKWARD checks the eval breaker after doing the JUMPBY which subtracts from next_instr, in the exception handler next_instr is pointing to the JUMP_BACKWARD instruction itself. The exception table lookup uses next_instr-1 to get the "current instruction," so signal exception handling working correctly for JUMP_BACKWARD depends on the instruction prior to the target of the JUMP_BACKWARD also being part of the same exception handler block, which is probably usually the case, but not in this particular repro.

I think the correct fix in 3.11 and 3.12 is to move CHECK_EVAL_BREAKER() before JUMPBY in JUMP_BACKWARD, but this isn't as easy as just moving it, since the current implementation of CHECK_EVAL_BREAKER() in 3.11 and 3.12 also does DISPATCH() after handling a signal, so the JUMPBY would never occur. So a bit more of e586211 (the change to CHECK_EVAL_BREAKER() implementation) would also have to be backported to make this work correctly.

@carljm
Copy link
Member

carljm commented Aug 21, 2023

cc @gvanrossum since a partial backport of #104584 looks to be the right direction to fix this bug, and cc @Yhg1s since this is a fix that we probably ideally want in 3.12 final (though since the bug also exists in 3.11, it may not be strictly a blocker.)

(EDIT: I messed up the link above, it's #106141 that may need a partial backport, and that's a @markshannon PR so I tagged the wrong person too :) )

@sweeneyde sweeneyde removed the pending The issue will be closed if no feedback is provided label Aug 21, 2023
@AlexWaygood AlexWaygood added 3.11 only security fixes 3.12 only security fixes labels Aug 21, 2023
@gvanrossum
Copy link
Member

Thinking more about it, I think this is not a release blocker. We can fix it in 3.12.1 and backport that fix to 3.11. @Yhg1s Agreed?

(And yes, I can repro for 3.11 and 3.12 on macOS.)

@umair-gulbaz

This comment was marked as duplicate.

@iritkatriel iritkatriel added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.11 only security fixes 3.12 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

10 participants