Better mapping of `sys.monitoring` branches back to source code

[jaltmayerpizzorno]

Feature or enhancement

Proposal:

sys.monitoring allows us to record branches taken during execution, reporting them in terms of their source and destination offsets in bytecode. These offsets are meaningless to developers, however, unless they take the time to generate and study the bytecode disassembly. Code objects provide co_positions() to map bytecode locations to the source code that originated it, but the branches don't always map to locations that make sense to someone only looking at the source code.

For example, when executed,

def foo(x):
    if x: print(x)
foo(0)

shows a branch from offset 4, which maps to the if keyword, to offset 30, which also maps to that if.

  1           0 RESUME                   0

  2           2 LOAD_FAST                0 (x)
              4 POP_JUMP_IF_FALSE       12 (to 30)
              6 LOAD_GLOBAL              1 (NULL + print)
             16 LOAD_CONST               1 ('x')
             18 CALL                     1
             26 POP_TOP
             28 RETURN_CONST             0 (None)
        >>   30 RETURN_CONST             0 (None)

ex1.py 2:7-2:8 (foo@4) -> 2:7-2:8 (foo@30)

How can/should a coverage tool attempting to report this branch recognize that it is a branch out of the function?
The branch does go to a RETURN_... opcode, but is that reliable?

Branches from for loops also show a bit obfuscated. In the example

def foo():
    for i in range(3):
        print(i)

foo()

the branch taken into the block containing the print call actually shows as a branch to the i in line 2:

  1           0 RESUME                   0

  2           2 LOAD_GLOBAL              1 (NULL + range)
             12 LOAD_CONST               1 (3)
             14 CALL                     1
             22 GET_ITER
        >>   24 FOR_ITER                13 (to 54)
             28 STORE_FAST               0 (i)

  3          30 LOAD_GLOBAL              3 (NULL + print)
             40 LOAD_FAST                0 (i)
             42 CALL                     1
             50 POP_TOP
             52 JUMP_BACKWARD           15 (to 24)

  2     >>   54 END_FOR
             56 RETURN_CONST             0 (None)

ex2.py 2:4-3:16 (foo@24) -> 2:8-2:9 (foo@28)

The final branch once the loop is done also shows a bit funny, not to offset 54, as expected, but to offset 56:

ex2.py 2:4-3:16 (foo@24) -> 2:4-3:16 (foo@56)

I've seen unexpected behavior with while loops as well... I can probably recreate it if desired.

@markshannon, @nedbat and I spoke about this issue previously; Mark asked me to open this issue. It should arguably be a bug report rather than a feature request... it can be your call, Mark.

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

I spoke briefly about these issues in my PyCon'24 talk: https://www.youtube.com/watch?v=X9aXWeLC_C0

[markshannon]

Making locations "make sense to someone only looking at the source code" is hard as it is somewhat subjective, but I'll do my best 🙂

It seems that the locations that don't "make sense ..." are branches that exit the scope.
The False branch in:

def foo(x):
    if x: print(x)

and exiting the loop in:

def foo():
    for i in range(3):
        print(i)

The problem is that there is no location for the exit from the function, but we need to provide a location for the destination of the branch.

PEP 626 says that the line events must reflect the lines executed, so we cannot introduce spurious line changes.
PEP 657 refines lines to locations, but the same reasoning applies. Therefore any branch that exits the scope must have destination_location == source_location to avoid introducing extra events. Any branch that does not exit the scope should have destination_location != source_location. If you find either of statements to not be true, please submit an issue for that case.

Not all locations have been correct in earlier versions and the locations for some parts of compounds statements were overly broad, but that has been fixed now. I believe the locations are all good in 3.13 and 3.14 @iritkatriel is that correct?

The final branch once the loop is done also shows a bit funny, not to offset 54, as expected, but to offset 56

The actual offsets should be regarded as an implementation detail. It is the locations that matter. The runtime may skip instructions that it can tell it doesn't need to execute them.
It is probably best to treat an offset as just an index into co_positions for the actual location, not as anything meaningful in itself.

[iritkatriel]

I believe the locations are all good in 3.13 and 3.14 @iritkatriel is that correct?

3.13 and 3.14 are awesome.

The only open issue I am aware of re location is #96999 .

[jaltmayerpizzorno]

Thanks, Mark. Here's an example with while (where I added a return None just to avoid the branch-to-exit):

def foo(n):
    while n<3:
        print(n)
        n += 1

    return None

foo(0)

  1           0 RESUME                   0

  2           2 LOAD_FAST                0 (n)
              4 LOAD_CONST               1 (3)
              6 COMPARE_OP               2 (<)
             10 POP_JUMP_IF_FALSE       22 (to 56)

  3     >>   12 LOAD_GLOBAL              1 (NULL + print)
             22 LOAD_FAST                0 (n)
             24 CALL                     1
             32 POP_TOP

  4          34 LOAD_FAST                0 (n)
             36 LOAD_CONST               2 (1)
             38 BINARY_OP               13 (+=)
             42 STORE_FAST               0 (n)

  2          44 LOAD_FAST                0 (n)
             46 LOAD_CONST               1 (3)
             48 COMPARE_OP               2 (<)
             52 POP_JUMP_IF_FALSE        1 (to 56)
             54 JUMP_BACKWARD           22 (to 12)

  6     >>   56 RETURN_CONST             0 (None)

ex3.py 2:10-2:13 (foo@10) -> 3:8-3:13 (foo@12)
ex3.py 2:10-2:13 (foo@52) -> 2:10-2:13 (foo@54)
ex3.py 2:10-2:13 (foo@52) -> 2:10-2:13 (foo@54)
ex3.py 2:10-2:13 (foo@52) -> 6:11-6:15 (foo@56)

In the first iteration, the branch is shown as going into the block, but
the next two show as going from the condition to the condition.

[jaltmayerpizzorno]

Coverage tools need to be able to enumerate all possible branches... Is it reasonable to do so
by using dis to go through the bytecode, recording what every jump opcode's offsets and
their destinations map to? I.e., see if the dis.Instruction.opcode is in hasjrel or hasjabs
and interpret Instruction.arg as either a delta or absolute jump destination, then use co_positions
on those values?

[markshannon]

I've created an issue for the inconsistency in while statement locations.
@jaltmayerpizzorno could you make individual issues for each problem you find. That way we can track them more easily and make sure that none get overlooked.

[markshannon]

Coverage tools need to be able to enumerate all possible branches...

I'd be careful using dis output. It is mainly a tool for the curious and for debugging the bytecode compiler.
Having said that, what you suggest should work and I don't have a better suggestion that you can use immediately.

Maybe we should add something like co_branches() which would list all the branches. It should be straightforward to implement as we already need to be able to quickly find all branches for instrumentation.

[jaltmayerpizzorno]

Maybe we should add something like co_branches() which would list all the branches. It should be straightforward to implement as we already need to be able to quickly find all branches for instrumentation.

I would love that :) Coverage tools also need to know about all lines that contain code, which co_lines() provides.

[markshannon]

I've added co_branches() which returns an iterator of 3-tuples, (start, not_taken, taken) byte offsets to the draft PR

[jaltmayerpizzorno]

I've been experimenting with using the AST to identify what the branches do in the code (e.g., from an if to its block, or from one clause to another clause in an expression). This would allow us to filter out certain branches, such as those only relevant for clause coverage, or other branches (such as the one checking for a pending exception after a with context handler) that do not correspond to source code branches.

I am mapping each branch source and destination to the source code using co_positions() and comparing these to AST elements lineno, col_offset, end_lineno and end_col_offset. I then decide the function of the branch based on its source and destination, e.g.:

• if the source equals the destination, the branch is going out of scope (returning from the function, etc.);
• if both the source and destination of a branch lie in the test of an ast.If or ast.While, these are branches from one clause to another;
• if a branch source is in a ast.For's iter and its destination is in its target, then it's a branch into the block (and I can use the block's first node's line/column for the source code mapping);
• if it's ast.match_case, branches within its pattern and itself, or pattern and guard would be clause branches;

@markshannon @iritkatriel and possibly others: does this make sense? Does this way of interpreting the branches have a good chance of staying stable across Python releases?

Thank you!

[iritkatriel]

The structure of the AST is not stable, so something like ast.For may not even exist in the next version.

[nedbat]

The structure of the AST is not stable, so something like ast.For may not even exist in the next version.

It's hard to imagine the AST not having a For node while the language has a for statement.

But in any case, we'd really appreciate help with how to correctly interpret branches. co_branches() is very helpful, but what is the right way to determine that a branch is exiting the function?

def foo(x):
    if x: print(x)

produces this dis output:

1:0-1:0             0       RESUME                   0

2:7-2:8             2       LOAD_FAST                0 (x)
2:7-2:8             4       TO_BOOL
2:7-2:8            12       POP_JUMP_IF_FALSE       13 (to L1)
2:7-2:8            16       NOT_TAKEN
2:10-2:15          18       LOAD_GLOBAL              1 (print + NULL)
2:16-2:17          28       LOAD_FAST                0 (x)
2:10-2:18          30       CALL                     1
2:10-2:18          38       POP_TOP
2:10-2:18          40       RETURN_CONST             0 (None)
2:7-2:8      L1:   42       RETURN_CONST             0 (None)

and co_branches() says there is one branch starting from offset 12, with 18/42 as the taken/not-taken. What should we do to reliably say that a branch is exiting the function?