Repository: python/cpython
Pull request: gh-105658: fix excess trace events for except block ending with a conditional block

This will remove the event with the wrong line number. It will still work because RERAISE doesn't need to have a line number attached. In exception_unwind, the instruction pointer on the frame is used to determine which exception handler should be invoked. The location of the reraise doesn't show up in the traceback.

Fixes #105658.

<!-- gh-issue-number: gh-109156 -->
* Issue: gh-105658
<!-- /gh-issue-number -->


Problem statement:
3.12 tracing regression: a conditional in a finally block will revisit the condition before exiting the block.
Python 3.12 introduced a change in tracing behavior.  Now a conditional in a finally block will revisit the condition before exiting the block.

Here is test.py:
```python
import sys
print(sys.version)

def example(raise_exc: bool, enter_finally_cond: bool):
    try:
        if raise_exc:
            print("raising exception")
            raise Exception
        print("not raising exception")
    finally:
        if enter_finally_cond:
            print("in finally conditional")

    print("after finally")

try:
    example(raise_exc=True, enter_finally_cond=True)
except:
    pass
```

When traced under 3.11:
```
% python3.11 -m trace --trace test.py
 --- modulename: test, funcname: <module>
test.py(1): import sys
test.py(2): print(sys.version)
3.11.4 (main, Jun  7 2023, 08:42:37) [Clang 14.0.3 (clang-1403.0.22.14.1)]
test.py(4): def example(raise_exc: bool, enter_finally_cond: bool):
test.py(16): try:
test.py(17):     example(raise_exc=True, enter_finally_cond=True)
 --- modulename: test, funcname: example
test.py(5):     try:
test.py(6):         if raise_exc:
test.py(7):             print("raising exception")
raising exception
test.py(8):             raise Exception
test.py(11):         if enter_finally_cond:
test.py(12):             print("in finally conditional")
in finally conditional
test.py(18): except:
test.py(19):     pass
```

When traced under 3.12:
```
% python3.12 -m trace --trace test.py
 --- modulename: test, funcname: <module>
test.py(1): import sys
test.py(2): print(sys.version)
3.12.0b2 (main, Jun  7 2023, 08:47:18) [Clang 14.0.3 (clang-1403.0.22.14.1)]
test.py(4): def example(raise_exc: bool, enter_finally_cond: bool):
test.py(16): try:
test.py(17):     example(raise_exc=True, enter_finally_cond=True)
 --- modulename: test, funcname: example
test.py(5):     try:
test.py(6):         if raise_exc:
test.py(7):             print("raising exception")
raising exception
test.py(8):             raise Exception
test.py(11):         if enter_finally_cond:
test.py(12):             print("in finally conditional")
in finally conditional
test.py(11):         if enter_finally_cond:             # <******
test.py(18): except:
test.py(19):     pass
```

The extra line is marked with `<******`.  There's no reason for the if statement to be traced again.


<!-- gh-linked-prs -->
### Linked PRs
* gh-109384
* gh-109411
<!-- /gh-linked-prs -->

Hints from issue discussion:
Confirmed on current main branch
Bisected to #98001, @nedbat can you confirm that?
Why change of location is needed here?
https://github.com/python/cpython/blob/20a56d8becba1a5a958b167fdb43b1a1b9228095/Python/compile.c#L3236
I've deleted it(same for `compiler_try_star_finally`) and seems this fixes the issue, but I don't sure that's correct solution.


Sorry, this dropped off my radar.  I made a PR to remove the line number from these instructions (which are virtual).

Commit message: Merge remote-tracking branch 'upstream/main' into lineno-try

Patch to review:
diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py
index 568200c9c86c7e..d104e5dd904999 100644
--- a/Lib/test/test_dis.py
+++ b/Lib/test/test_dis.py
@@ -642,7 +642,8 @@ def _tryfinallyconst(b):
             CALL                     0
             POP_TOP
             RERAISE                  0
-         >> COPY                     3
+
+None     >> COPY                     3
             POP_EXCEPT
             RERAISE                  1
 ExceptionTable:
@@ -674,7 +675,8 @@ def _tryfinallyconst(b):
             CALL                     0
             POP_TOP
             RERAISE                  0
-         >> COPY                     3
+
+None     >> COPY                     3
             POP_EXCEPT
             RERAISE                  1
 ExceptionTable:
@@ -1822,9 +1824,9 @@ def _prepare_test_cases():
   Instruction(opname='CALL', opcode=53, arg=1, argval=1, argrepr='', offset=414, start_offset=414, starts_line=False, line_number=28, is_jump_target=False, positions=None),
   Instruction(opname='POP_TOP', opcode=32, arg=None, argval=None, argrepr='', offset=422, start_offset=422, starts_line=False, line_number=28, is_jump_target=False, positions=None),
   Instruction(opname='RERAISE', opcode=102, arg=0, argval=0, argrepr='', offset=424, start_offset=424, starts_line=False, line_number=28, is_jump_target=False, positions=None),
-  Instruction(opname='COPY', opcode=61, arg=3, argval=3, argrepr='', offset=426, start_offset=426, starts_line=False, line_number=28, is_jump_target=False, positions=None),
-  Instruction(opname='POP_EXCEPT', opcode=31, arg=None, argval=None, argrepr='', offset=428, start_offset=428, starts_line=False, line_number=28, is_jump_target=False, positions=None),
-  Instruction(opname='RERAISE', opcode=102, arg=1, argval=1, argrepr='', offset=430, start_offset=430, starts_line=False, line_number=28, is_jump_target=False, positions=None),
+  Instruction(opname='COPY', opcode=61, arg=3, argval=3, argrepr='', offset=426, start_offset=426, starts_line=True, line_number=None, is_jump_target=False, positions=None),
+  Instruction(opname='POP_EXCEPT', opcode=31, arg=None, argval=None, argrepr='', offset=428, start_offset=428, starts_line=False, line_number=None, is_jump_target=False, positions=None),
+  Instruction(opname='RERAISE', opcode=102, arg=1, argval=1, argrepr='', offset=430, start_offset=430, starts_line=False, line_number=None, is_jump_target=False, positions=None),
 ]
 
 # One last piece of inspect fodder to check the default line number handling
diff --git a/Lib/test/test_sys_settrace.py b/Lib/test/test_sys_settrace.py
index 369a276ac33a12..f5da32597d39a2 100644
--- a/Lib/test/test_sys_settrace.py
+++ b/Lib/test/test_sys_settrace.py
@@ -2123,7 +2123,7 @@ def test_jump_in_nested_finally_3(output):
             output.append(11)
         output.append(12)
 
-    @jump_test(5, 11, [2, 4], (ValueError, 'exception'))
+    @jump_test(5, 11, [2, 4], (ValueError, 'comes after the current code block'))
     def test_no_jump_over_return_try_finally_in_finally_block(output):
         try:
             output.append(2)
diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-09-13-19-16-51.gh-issue-105658.z2nR2u.rst b/Misc/NEWS.d/next/Core and Builtins/2023-09-13-19-16-51.gh-issue-105658.z2nR2u.rst
new file mode 100644
index 00000000000000..2b4ca22f154678
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2023-09-13-19-16-51.gh-issue-105658.z2nR2u.rst	
@@ -0,0 +1,2 @@
+Fix bug where the line trace of an except block ending with a conditional
+includes an excess even with the line of the conditional expression.
diff --git a/Python/compile.c b/Python/compile.c
index b05c1ad96be803..1d9ae626677310 100644
--- a/Python/compile.c
+++ b/Python/compile.c
@@ -3261,18 +3261,6 @@ compiler_continue(struct compiler *c, location loc)
 }
 
 
-static location
-location_of_last_executing_statement(asdl_stmt_seq *stmts)
-{
-    for (Py_ssize_t i = asdl_seq_LEN(stmts) - 1; i >= 0; i++) {
-        location loc = LOC((stmt_ty)asdl_seq_GET(stmts, i));
-        if (loc.lineno > 0) {
-            return loc;
-        }
-    }
-    return NO_LOCATION;
-}
-
 /* Code generated for "try: <body> finally: <finalbody>" is as follows:
 
         SETUP_FINALLY           L
@@ -3341,9 +3329,9 @@ compiler_try_finally(struct compiler *c, stmt_ty s)
     RETURN_IF_ERROR(
         compiler_push_fblock(c, loc, FINALLY_END, end, NO_LABEL, NULL));
     VISIT_SEQ(c, stmt, s->v.Try.finalbody);
-    loc = location_of_last_executing_statement(s->v.Try.finalbody);
     compiler_pop_fblock(c, FINALLY_END, end);
 
+    loc = NO_LOCATION;
     ADDOP_I(c, loc, RERAISE, 0);
 
     USE_LABEL(c, cleanup);
@@ -3392,9 +3380,9 @@ compiler_try_star_finally(struct compiler *c, stmt_ty s)
         compiler_push_fblock(c, loc, FINALLY_END, end, NO_LABEL, NULL));
 
     VISIT_SEQ(c, stmt, s->v.TryStar.finalbody);
-    loc = location_of_last_executing_statement(s->v.Try.finalbody);
 
     compiler_pop_fblock(c, FINALLY_END, end);
+    loc = NO_LOCATION;
     ADDOP_I(c, loc, RERAISE, 0);
 
     USE_LABEL(c, cleanup);


Code review comments:
diff_hunk: @@ -0,0 +1,2 @@
+Fix bug where the line trace of an except block ending with a conditional
+includes an excess even with the line of the conditional expression.
path: Misc/NEWS.d/next/Core and Builtins/2023-09-13-19-16-51.gh-issue-105658.z2nR2u.rst
text: @user1:
```suggestion
includes an excess event with the line of the conditional expression.
```