fix: see through nop bytecodes to get the right arcs. #1999

Author: nedbat

CHANGES.rst

@@ -23,7 +23,11 @@ upgrading your version of coverage.py.
 Unreleased
 ----------
 
-Nothing yet.
+- Fix: some code with NOP bytecodes could report missing branches that are
+  actually executed. This is now fixed, closing `issue 1999`_. Python 3.9
+  still shows the problem.
+
+.. _issue 1999: https://github.com/nedbat/coveragepy/issues/1999
 
 
 .. start-releases

coverage/bytecode.py

@@ -43,7 +43,16 @@ def op_set(*op_names: str) -> set[int]:
 )
 
 # Opcodes that exit from a function.
-RETURNS = op_set("RETURN_VALUE", "RETURN_GENERATOR")
+RETURNS = op_set(
+    "RETURN_VALUE",
+    "RETURN_GENERATOR",
+)
+
+# Opcodes that do nothing.
+NOPS = op_set(
+    "NOP",
+    "NOT_TAKEN",
+)
 
 
 class InstructionWalker:
@@ -126,7 +135,7 @@ def walk_one_branch(start_at: TOffset) -> TBranchTrail:
             # pylint: disable=cell-var-from-loop
             inst_offsets: set[TOffset] = set()
             to_line = None
-            for inst2 in iwalker.walk(start_at=start_at):
+            for inst2 in iwalker.walk(start_at=start_at, follow_jumps=True):
                 inst_offsets.add(inst2.offset)
                 if inst2.line_number and inst2.line_number != from_line:
                     to_line = inst2.line_number
@@ -160,3 +169,18 @@ def walk_one_branch(start_at: TOffset) -> TBranchTrail:
                 the_trails[offset].append(trail)
 
     return the_trails
+
+
+def always_jumps(code: CodeType) -> dict[TOffset, TOffset]:
+    """Make a map of unconditional bytecodes jumping to others.
+
+    Only include bytecodes that do no work and go to another bytecode.
+    """
+    jumps = {}
+    iwalker = InstructionWalker(code)
+    for inst in iwalker.walk(follow_jumps=False):
+        if inst.opcode in ALWAYS_JUMPS:
+            jumps[inst.offset] = inst.jump_target
+        elif inst.opcode in NOPS:
+            jumps[inst.offset] = inst.offset + 2
+    return jumps

coverage/sysmon.py

@@ -24,7 +24,7 @@
 )
 
 from coverage import env
-from coverage.bytecode import TBranchTrails, branch_trails
+from coverage.bytecode import TBranchTrails, always_jumps, branch_trails
 from coverage.debug import short_filename, short_stack
 from coverage.misc import isolate_module
 from coverage.types import (
@@ -184,6 +184,10 @@ class CodeInfo:
     #   Two possible trails from the branch point, left and right.
     branch_trails: TBranchTrails
 
+    # Always-jumps are bytecode offsets that do no work but move
+    # to another offset.
+    always_jumps: dict[TOffset, TOffset]
+
 
 def bytes_to_lines(code: CodeType) -> dict[TOffset, TLineNo]:
     """Make a dict mapping byte code offsets to line numbers."""
@@ -348,6 +352,7 @@ def sysmon_py_start(
                 file_data=file_data,
                 byte_to_line=b2l,
                 branch_trails={},
+                always_jumps={},
             )
             self.code_infos[id(code)] = code_info
             self.code_objects.append(code)
@@ -431,15 +436,23 @@ def sysmon_branch_either(
             if self.stats is not None:
                 self.stats["branch_trails"] += 1
             code_info.branch_trails = branch_trails(code)
+            code_info.always_jumps = always_jumps(code)
             # log(f"branch_trails for {code}:\n    {code_info.branch_trails}")
         added_arc = False
         dest_info = code_info.branch_trails.get(instruction_offset)
+
+        # Re-map the destination offset through always-jumps to deal with NOP etc.
+        dests = {destination_offset}
+        while (dest := code_info.always_jumps.get(destination_offset)) is not None:
+            destination_offset = dest
+            dests.add(destination_offset)
+
         # log(f"{dest_info = }")
         if dest_info is not None:
             for offsets, arc in dest_info:
                 if arc is None:
                     continue
-                if destination_offset in offsets:
+                if dests & offsets:
                     code_info.file_data.add(arc)  # type: ignore
                     # log(f"adding {arc=}")
                     added_arc = True

tests/test_arcs.py

@@ -2129,6 +2129,29 @@ def func():
             branchz_missing="29 38 45 56 5. 9A 9.",
         )
 
+    @pytest.mark.skipif(env.PYVERSION[:2] == (3, 9), reason="not sure why it isn't fixed on 3.9")
+    def test_bug_1999(self) -> None:
+        self.check_coverage("""\
+            import asyncio
+
+            async def async_range(number):
+                for n in range(number):
+                    yield n
+
+            async def do_something(number):
+                try:
+                    async for n in async_range(number):
+                        print(n)
+                finally:
+                    print("Done.")
+
+            asyncio.run(do_something(3))
+            """,
+            branchz="4-3 45 9A 9C",
+            branchz_missing="",
+        )
+        assert self.stdout() == "0\n1\n2\nDone.\n"
+
 
 class AnnotationTest(CoverageTest):
     """Tests using type annotations."""