Skip to content

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

Closed
xdegaye wants to merge 2 commits intopython:masterfrom
xdegaye:bpo-24565
Closed

bpo-24565: f->f_lineno is now -1 when tracing is not set#6233
xdegaye wants to merge 2 commits intopython:masterfrom
xdegaye:bpo-24565

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
*instr_lb = bounds.ap_lower;
*instr_ub = bounds.ap_upper;
} else {
line = PyFrame_GetLineNumber(frame);
Copy link
Contributor Author

@xdegaye xdegaye Mar 25, 2018

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

serhiy-storchaka commented May 5, 2018

Added links to BPO issues.

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

taleinat commented Jul 25, 2018

@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

taleinat commented Mar 18, 2019

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

taleinat commented Mar 18, 2019

@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