8000 Incorrect locations for code following `case` blocks · Issue #103971 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

Incorrect locations for code following case blocks #103971

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.

Sign up for GitHub

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

Jump to bottom
Closed
marvinweitzel opened this issue Apr 28, 2023 · 7 comments
Closed

Incorrect locations for code following case blocks #103971

marvinweitzel opened this issue Apr 28, 2023 · 7 comments
Assignees
@gaogaotiantian @brandtbucher
Labels
3.11 only security fixes easy interpreter-core (Objects, Python, Grammar, and Parser dirs) triaged The issue has been accepted as valid by a triager. type-bug An unexpected behavior, bug, or error

Comments

@marvinweitzel
Copy link
marvinweitzel commented Apr 28, 2023 •
edited by bedevere-bot
Loading

Bug report

In the following example, the debugger hits a breakpoint that is set in the aVariable = ... line, which is in an if-statement whose condition is False and which should therefore not be executed. When I run the example with coverage (under PyCharm 2023.1), that line turns green. The print statement is not executed, which matches the expectation.

The assignment does not actually happen. It somehow just hits the line without really executing it.

Minimal reproducible example:

match 1:
    case 1:
        if False:
            print('this should not be executed')
            aVariable = 'somehow, we can hit a breakpoint here'

The same happens, if the last statement in the unreachable code is a pass. If I replace it with e.g. a print() statement, then everything behaves as expected.

If we extend the example a little bit, that behavior is reproducible for an unreachable else block, too:

match 1:
    case 1:
        if True:
            pass
        else:
            anotherVariable = 'somehow, we can hit a breakpoint here, too'

Your environment

python --version
Python 3.11.3

lsb_release -a
No LSB modules are available.
Distributor ID:	Ubuntu
Description:	Ubuntu 22.04.2 LTS
Release:	22.04
Codename:	jammy

I initially encountered that behavior in a 3.10 version. Due to the fact that I thought, I could fix it with an upgrade to 3.11, I don't know the exact minor version of 3.10.

I double-checked this with the first online Python debugger that I could find and it behaves the same way.

Linked PRs
@marvinweitzel marvinweitzel added the type-bug An unexpected behavior, bug, or error label Apr 28, 2023
@gaogaotiantian
Copy link
Member

Confirmed that this happens with pdb on 3.11 too. Not reproducible in main, bytecode is different.

3.11:

  0           0 RESUME                   0

  1           2 LOAD_CONST               0 (1)

  2           4 LOAD_CONST               0 (1)
              6 COMPARE_OP               2 (==)
             12 POP_JUMP_FORWARD_IF_FALSE     3 (to 20)

  3          14 NOP

  5          16 LOAD_CONST               4 (None)
             18 RETURN_VALUE

  2     >>   20 LOAD_CONST               4 (None)
             22 RETURN_VALUE

RETURN_VALUE was set at the last line(5) of the case block, so even though it's unreachable, it was hit.

3.12(main):

  0           0 RESUME                   0

  1           2 LOAD_CONST               0 (1)

  2           4 LOAD_CONST               0 (1)
              6 COMPARE_OP              40 (==)
             10 POP_JUMP_IF_FALSE        1 (to 14)

  3          12 RETURN_CONST             1 (None)

  2     >>   14 RETURN_CONST             1 (None)

RETURN_CONST is at line 3, which is correct.

Not 100% sure if this is a bug that we want to fix in 3.11? 3.10 is already closed for bug fix right? We need expertise from the compiler side.

@brandtbucher
Copy link
Member

Yeah, this is a bug, and should be fixed in 3.11. From a first glance, this should be as simple as saving and restoring the current location when entering/exiting case bodies in the compiler.

Any takers? Otherwise, I can pick it up.

@brandtbucher brandtbucher added interpreter-core (Objects, Python, Grammar, and Parser dirs) 3.11 only security fixes triaged The issue has been accepted as valid by a triager. labels Apr 28, 2023
@brandtbucher brandtbucher self-assigned this Apr 28, 2023
@brandtbucher
Copy link
Member

There are two places where VISIT_SEQ(c, stmt, m->body); is called in the 3.11 version of compile.c. I think that the fix should be as simple as calling UNSET_LOC(c); after each, to keep the last line number from leaking into the following control-flow.

@brandtbucher
Copy link
Member

This will also need one or more regression tests for the cases shown here. Thankfully, we already have good scaffolding for that in test.test_patma.TestTracing.

Even though the bug is only in 3.11, I'd like to see the tests forward-ported to 3.12 after.

@brandtbucher brandtbucher mentioned this issue Apr 28, 2023
Closed
@terryjreedy
Copy link
Member

I reproduced this in IDLE's debugger. Either 'Go' or 'Step, Step, Step' skips over line 4 to line 5, but a subsequent Go/Step does not execute line 5. Change line 5 to a print and is is skipped as expected. IDLE Debugger uses a subclass of bdb, so the bug is not in pdb.

@brandtbucher
Copy link
Member

Yep, this is a compiler bug (3.12 cleaned up how we handle locations to avoid issues like this, which is why it's silently fixed there).

@bedevere-bot bedevere-bot mentioned this issue Apr 28, 2023
Merged
gaogaotiantian added a commit to gaogaotiantian/cpython that referenced this issue Apr 28, 2023
@gaogaotiantian
Rename 2023-04-28-18-57-13.gh-issue-pythongh-103971.Q3U9lv.rst to 202…
8de15e5 
…3-04-28-18-57-13.gh-issue-103971.Q3U9lv.rst
@marvinweitzel
Copy link
Author

I just wanted to say how much I appreciate your responses and your extremely fast investigation of this issue.

Thanks a lot for taking this seriously!

@brandtbucher brandtbucher changed the title Match-case can hit a breakpoint in unreachable code and coverage turns green for that line Incorrect line numbers following case blocks Apr 28, 2023
@brandtbucher brandtbucher changed the title Incorrect line numbers following case blocks Incorrect locations for code following case blocks Apr 28, 2023
gaogaotiantian added a commit to gaogaotiantian/cpython that referenced this issue Apr 28, 2023
@gaogaotiantian
Add unreachable line test per pythongh-103971
5c5eafe
@bedevere-bot bedevere-bot mentioned this issue Apr 28, 2023
Merged
brandtbucher pushed a commit that referenced this issue Apr 28, 2023
@gaogaotiantian
[3.11] GH-103971: Fix incorrect locations for code following case blocks
fee3c91
brandtbucher pushed a commit that referenced this issue Apr 28, 2023
@gaogaotiantian
GH-103971: Forward-port test from GH-103980 (GH-103984)
79b9db9
@brandtbucher brandtbucher mentioned this issue May 31, 2023
Closed
@gaogaotiantian gaogaotiantian mentioned this issue Jul 26, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@gaogaotiantian gaogaotiantian

@brandtbucher brandtbucher

Labels
3.11 only security fixes easy interpreter-core (Objects, Python, Grammar, and Parser dirs) triaged The issue has been accepted as valid by a triager. type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@gaogaotiantian @terryjreedy @brandtbucher @marvinweitzel
0