Coverage.py test suite fails on 3.13 (since June?)

@godlygeek

Bug report

Bug description:

The coverage.py test suite is failing on Python3.13 using sys.settrace. Sorry it's taken so long to report. This is blocking me claiming 3.13 support, and running the test suite on nightly builds.

The test suite fails as if there are too many "return" events being traced, or not enough "calls". To see the failure, we can run just two tests from the suite. One fails:

% git clone https://github.com/nedbat/coveragepy
% cd coveragepy
% git show --oneline --no-patch     # Though the specific commit of coveragepy shouldn't matter
  75edefcd (HEAD -> master, origin/master, origin/HEAD) build: avoid an installation problem on Windows PyPy
% python3.13 -m venv venv
% source venv/bin/activate
% python3.13 -VV
Python 3.13.0a2 (main, Nov 24 2023, 14:53:47) [Clang 15.0.0 (clang-1500.0.40.1)]
% python3.13 -m pip install -r requirements/pytest.pip
% python3.13 -m pytest tests -vvv -k test_report_precision -n0

It should fail with:

            # Leaving this function, pop the filename stack.
            self.cur_file_data, self.cur_file_name, self.last_line, self.started_context = (
>               self.data_stack.pop()
            )
E           IndexError: pop from empty list

/private/tmp/bug/coveragepy/coverage/pytracer.py:263: IndexError

Just before that code in pytracer.py is some ugly opcode inspection which could now be wrong, but the symptoms aren't immediately pointing to that.

Matt Wozniski (@godlygeek) bisected CPython and found two commits of interest. This is his commentary:

04492cb is the bad commit. if you build cpython at that version, the coverage test suite fails with a SIGABRT due to a failed assertion. The first version after that where there's no SIGABRT is 9339d70 - that's the first version where you get the test suite failure we see today. You get the same error as we see today on main if you check out the older one, and then cherry-pick the fix for the assertion failure:

git checkout 04492cbc9aa45ac2c12d22083c406a0364c39f5b
git cherry-pick --no-commit 9339d70ac2d45743507e320e81e68acf77e366af

And if you instead check out the one that fixes the assertion error, and then revert the one that introduced the assertion error, the coverage test suite passes:

git checkout 9339d70ac2d45743507e320e81e68acf77e366af
git revert --no-commit 04492cbc9aa45ac2c12d22083c406a0364c39f5b

Unfortunately that doesn't revert cleanly on main, but at this point I'm very convinced 04492cb introduces the problem.

We know a commit where it fails (9339d70), and we know that from that point if you revert another commit (04492cb) it passes. And we know that it happens even with a pure Python tracer, not just with a C extension module, so it's not related to changes in the C API.

CPython versions tested on:

3.13, CPython main branch

Operating systems tested on:

macOS

Linked PRs

GH-113486: Do not emit spurious PY_UNWIND events for optimized calls to classes. #113680

@markshannon

@markshannon will likely be able to help.

Is it possible to trim the test case? Can we repro this with CPython only?

I don't have a simpler way to reproduce it. I'm hoping that a core dev familiar with those commits will understand more about what is happening.

@markshannon

MRE:

import sys

def trace(frame, event, arg):
    if event in ['call', 'return']:
        print(event, frame.f_code.co_name, frame.f_lineno)
    return trace

class B:
    def __init__(self):
        raise ValueError()

def f():
    try:
        return B()
    except ValueError:
        pass

for _ in range(100):
    f()

sys.settrace(trace)
f()
sys.settrace(None)

This is similar to #108390, but the fix for #108390 won't work here. The immediate cause is the shim frame sent PY_UNWIND event which was considered as a return. There's no "call" event for the shim frame so an extra return will be generated.

I don't think we want the "call" event for the shim frame, so obviously we need to somehow shield this PY_UNWIND event.

The naive way to solve this locally is to check frame->executable == &_Py_InitCleanup either before monitor_unwind in _PyEval_EvalFrameDefault, or somewhere in the legacy sys.settrace callback. It depends on whether we want the PY_UNWIND event in the shim frame for sys.monitoring. (We definitely do not want a "return" event for sys.settrace)

However, if we have more code objects like _Py_InitCleanup in the future, it would not be pretty to solve the problem like this. So circling back to what I've proposed in #108390 - should we create a flag for this kind of "invisible code object"? Even an internal one so monitoring system can recognize them and deal with them. @markshannon

GH-113486: Do not emit spurious PY_UNWIND events for optimized calls …

Triage: #113680 has been merged, can this be closed?

Yes, thanks. (I wasn't sure if the reporter or the fixer should close the issue...)

Either is fine, which means it often ends up being neither ;)

pythonGH-113486: Do not emit spurious PY_UNWIND events for optimized …

Navigation Menu

Repository navigation

Coverage.py test suite fails on 3.13 (since June?) #113486

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Coverage.py test suite fails on 3.13 (since June?) #113486

Description

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Activity

nedbat commented on Dec 26, 2023

gaogaotiantian commented on Dec 26, 2023

nedbat commented on Dec 26, 2023

gaogaotiantian commented on Dec 27, 2023

hugovk commented on Jan 11, 2024

nedbat commented on Jan 11, 2024

hugovk commented on Jan 11, 2024

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions