Skip to content

bpo-24565: f->f_lineno is now -1 when tracing is not set #6233

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
wants to merge 2 commits into from
Closed

bpo-24565: f->f_lineno is now -1 when tracing is not set #6233

wants to merge 2 commits into from

Conversation

xdegaye
Copy link
Contributor

@xdegaye xdegaye commented Mar 25, 2018 •
edited by serhiy-storchaka
Loading

When tracing is set, f->f_lineno is valid for all the frames of the
stack except upon starting or resuming a frame where it is -1 until
the first call to maybe_call_line_trace().

Fix issues 7238, 16482, 17277 and 17697.

https://bugs.python.org/issue24565

@xdegaye
bpo-24565: f->f_lineno is now -1 when tracing is not set
8a49be2 
When tracing is set, f->f_lineno is valid for all the frames of the
stack except upon starting or resuming a frame where it is -1 until
the first call to maybe_call_line_trace().

Fix issues 7238, 16482, 17277 and 17697.
@xdegaye
Copy link
Contributor Author

xdegaye commented Mar 25, 2018 •
edited by serhiy-storchaka
Loading

The PR introduces a behavior change: upon resuming a generator frame, it is not possible anymore to jump from a call trace event. This is a minor change as the jump may be done from the first line event without changing anything, and actually it can be considered as a fix as it is more consistent to forbid jumps from all call events and not only for new frames as previously.

I have checked that all the faulty use cases in issues 7238, 16482, 17277 and 17697 are ok now with this PR, and that the tests added by the PR (except test_20_return_in_caller) fail as expected when run with python built from the master branch (without the PR changes).

xdegaye
xdegaye commented Mar 25, 2018
View reviewed changes
Python/ceval.c Outdated
@@ -5095,12 +5106,14 @@ maybe_dtrace_line(PyFrameObject *frame,
&bounds);
*instr_lb = bounds.ap_lower;
*instr_ub = bounds.ap_upper;
} else {
line = PyFrame_GetLineNumber(frame);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When dtrace is active and a trace function is unset, f->f_lineno is set to -1 on all the frames on the stack and this is why we cannot use directly f->f_lineno in maybe_dtrace_line() and why the current line number must be obtained from PyFrame_GetLineNumber() instead. However the call to PyFrame_GetLineNumber() is only needed when PyDTrace_LINE() is called.
I will fix that in the next commit.

@blueyed blueyed mentioned this pull request Mar 27, 2018
Closed
@blueyed
Copy link
Contributor

blueyed commented Mar 27, 2018

Thank you for working on this!

/cc @antocuni
(because I've noticed that pdbpp does some bookkeeping internally to address this (https://github.com/antocuni/pdb/blob/b7e789b836340ecbc131e26f8fd93d74fe652297/pdb.py#L234-L239))

@blueyed
Copy link
Contributor

blueyed commented May 4, 2018

Is there anything I could help to move this forward?
I've tried it myself and it's working great when using pdb with tests (e.g. pytest-dev/pytest#3237).

@serhiy-storchaka
Copy link
Member

Added links to BPO issues.

@serhiy-storchaka serhiy-storchaka self-requested a review May 5, 2018 11:29
@taleinat
Copy link
Contributor

@serhiy-storchaka, what would be needed to move this forward?

@blueyed
Copy link
Contributor

blueyed commented Jul 28, 2018

Yeah, it's a real pity to not have this in 3.7 really.
The bug(s) that this fixes get into your way always when using __import__('pdb').set_trace().

Do you think it could go into 3.7.x, or does it have to wait for 3.8? (which I assume)

@blueyed
Copy link
Contributor

blueyed commented Mar 18, 2019

Can you resolve the conflict, please?
It is a pity that this is so old already and appears to get no traction, although it is a fix for annoying, subtle issues.

@taleinat
Copy link
Contributor

Do you think it could go into 3.7.x, or does it have to wait for 3.8? (which I assume)

Since this is a change in behavior, it won't go into 3.7.x. (@blueyed has already mentioned a 3rd party library that this change would likely affect.)

@taleinat
Copy link
Contributor

@serhiy-storchaka, ping?

@xdegaye xdegaye mentioned this pull request Mar 18, 2019
Closed
@xdegaye
Copy link
Contributor Author

xdegaye commented Mar 18, 2019

Closing this PR as it is from unknown repository, as printed at the top of this page (my fault, deleted the branch on my fork).

PR #12419 is the continuation of this PR with the pending conflict resolved.

@xdegaye xdegaye closed this Mar 18, 2019
@xdegaye xdegaye mannequin mentioned this pull request Apr 10, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@serhiy-storchaka serhiy-storchaka Awaiting requested review from serhiy-storchaka

Assignees
No one assigned
Labels
awaiting review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@xdegaye @blueyed @serhiy-storchaka @taleinat @the-knights-who-say-ni @bedevere-bot