[Patches] [ python-Patches-643835 ] Set Next Statement for Python debuggers
noreply@sourceforge.net
noreply@sourceforge.net
Thu, 19 Dec 2002 10:17:42 -0800
Patches item #643835, was opened at 2002-11-25 18:18
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=643835&group_id=5470
Category: Core (C code)
Group: Python 2.3
Status: Closed
Resolution: Accepted
Priority: 5
Submitted By: Richie Hindle (richiehindle)
Assigned to: Michael Hudson (mwh)
Summary: Set Next Statement for Python debuggers
Initial Comment:
"Set Next Statement", aka "Jump", for Python debuggers.
For Python 2.3a0 on all platforms (built and tested in Win98).
This patch adds the ability for Python debuggers (including
pdb) to control which line of code in a frame will be executed
next. This allows the user to jump back and execute code
again, or jump forward to skip code that he doesn't want to
run. A debugger (or anything else that installs a trace
function) does this by setting the value of frame.f_lineno
from within a trace function.
There are some situations where you can't jump because it
would break the stack - these are caught and a ValueError
is raised.
There are other cases where the jump *is* possible but the
stack needs fixing up, and the code does that.
The patch includes:
o A setter for frame.f_lineno, in frameobject.c
o A new command 'j(ump) <line number>' for pdb.
o Tests in test_trace.py for all the allowed and disallowed
types of jumps.
o Updated documentation for pdb and frame objects.
----------------------------------------------------------------------
>Comment By: Neal Norwitz (nnorwitz)
Date: 2002-12-19 13:17
Message:
Logged In: YES
user_id=33168
Thanks! That fixed the problem. Checked in as
Objects/frameobject.c 2.70
----------------------------------------------------------------------
Comment By: Richie Hindle (richiehindle)
Date: 2002-12-19 12:42
Message:
Logged In: YES
user_id=85414
OK, attached is a patch that fixes the bug and adds
the assertions that should have been there all along
to catch this kind of thing...
----------------------------------------------------------------------
Comment By: Richie Hindle (richiehindle)
Date: 2002-12-19 05:04
Message:
Logged In: YES
user_id=85414
Eeek! I'll look at this today.
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2002-12-18 18:39
Message:
Logged In: YES
user_id=33168
test_trace is now crashing on HPUX-11, line 200 in current cvs
case END_FINALLY:
setup_op = code[blockstack[blockstack_top-1]];
blockstack_top = 0, so it is accessing blockstack[-1]
(gdb) p blockstack
$1 = {0, 16, 0 <repeats 18 times>}
any ideas?
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2002-12-17 20:22
Message:
Logged In: YES
user_id=33168
Thanks Richie. Fixed.
----------------------------------------------------------------------
Comment By: Richie Hindle (richiehindle)
Date: 2002-12-17 12:41
Message:
Logged In: YES
user_id=85414
Great stuff!
One nit in your addition to the pdb help:
"for instance it it not possible..."
----------------------------------------------------------------------
Comment By: Michael Hudson (mwh)
Date: 2002-12-17 11:17
Message:
Logged In: YES
user_id=6656
OK, this is checked in with minor tweaks as
Doc/lib/libpdb.tex revision 1.34
Doc/ref/ref3.tex revision 1.96
Doc/whatsnew/whatsnew23.tex revision 1.89
Lib/pdb.py revision 1.59
Lib/test/test_trace.py revision 1.7
Misc/NEWS revision 1.565
Objects/frameobject.c revision 2.68
----------------------------------------------------------------------
Comment By: Michael Hudson (mwh)
Date: 2002-12-16 13:09
Message:
Logged In: YES
user_id=6656
I'll check this in pretty soon.
Re the signedness issues: you got the right bits of code,
but your fix went too far:
char op = codestr[blah]
switch (op) {
....
}
is *not* a good idea: bytecode is an array of unsigned chars
even if a Python string is not, and this would do the wrong
thing for bytecodes > 127 (we're not interested in any of
them, but still...).
I'll tweak that.
----------------------------------------------------------------------
Comment By: Richie Hindle (richiehindle)
Date: 2002-12-15 18:03
Message:
Logged In: YES
user_id=85414
I've attached a new patch, which implements most of
your suggestions:
o The Jump tests now have their own test case class
in test_trace.py.
o There's now a test for "lineno must be an integer".
o I've added your nested-finally-jump test.
o I've added a test for each of the four (four!! 8-)
types of 'except' line.
o I've clarified the docs. I don't have a machine
to build them on - are you in a position to check
that they build and look sensible?
o Jumps fail if the frame has no trace function set,
and there's a new test in test_trace.py for that.
o pdb complains if you try to do a jump in any frame
other than the bottom one.
I'd have liked frame_setlineno to police the fact
that you can only jump in the bottom frame of a trace,
but I couldn't figure out how to do that. It would
have implemented the final two points above in one go.
As far as I can see, you're right that you can't get
frame objects or set trace functions in restricted
mode. It would be trivial to reject writes to
f_lineno in restricted mode (frame objects have an
f_restricted flag that frame_setlineno could look at)
but given that it would be impossible to test, I
haven't implemented it.
I don't get any char / unsigned char warnings under
MSVC6 (even on the highest warning level). I've made
a guess at where they might be and 'fixed' them, but
I'd be grateful if you could check, and fix them if
they're still there.
I had a go at writing the finally-policing code in
pseudo-python, but it didn't come out significantly
simpler than the C - it has to work the same way as
the C in order to be useful, but that makes it pretty
much the same. Hopefully the comment explains the
workings sufficiently well.
----------------------------------------------------------------------
Comment By: Michael Hudson (mwh)
Date: 2002-12-11 06:11
Message:
Logged In: YES
user_id=6656
No problem, I just like to keep things ticking over lest I
start forgetting the issues.
re restricted mode: PyEval_GetRestricted() I think, but
a) I really hope you can't set a trace function in
restricted mode
b) I really hope you can't get hold of a frame object in
restricted mode
so I'm not sure you need to do anything.
----------------------------------------------------------------------
Comment By: Richie Hindle (richiehindle)
Date: 2002-12-11 03:54
Message:
Logged In: YES
user_id=85414
Sorry to need prodding! I agree with pretty much everything
you said. I was planning to just make the edits and submit
a new patch, but real life has got in the way.
I'll come up with a fresh patch soon. The only thing that
wasn't clear was "Restricted mode!?" Does this mean that
the code should behave differently in restricted mode
(probably it should fail with an explanatory exception).
Do you know off the top of your head how my C code
can find out whether it's in restricted mode?
----------------------------------------------------------------------
Comment By: Michael Hudson (mwh)
Date: 2002-12-10 12:56
Message:
Logged In: YES
user_id=6656
<prod>
Do you have comments on my comments? I can implement most
of my suggestions myself, but I thought it would be quicker
for you to do them...
----------------------------------------------------------------------
Comment By: Michael Hudson (mwh)
Date: 2002-12-06 10:35
Message:
Logged In: YES
user_id=6656
This is what happens when I download a patch and take it
home for comments:
Meta: Impressed! This is a bit long, so I suggest composing
your
reply in your mailer and pasting the result into the browser...
---------------------------------------------------
I get a couple of char** vs unsigned char** pointer
warnings. Trivial
stuff.
---------------------------------------------------
what should this:
def f(): # call this line 1
print 1, # 2
g() # 3
print 2, # 4
print 3 # 5
def g():
sys._getframe().f_back.f_lineno = 5
do? Three options:
1) barf
2) "work", i.e. print 1, 3
3) "fail silently", i.e. print 1, 2, 3
I think 1) is ideal. The patch gives 3). Could get slight
protection
by complaining in frame_setlineno if f_trace is not set.
OTOH, not
going to lose any sleep over this. The documentation should
be clear
on this, though.
Also unclear what to expect if you execute eg. "up" followed
by "j 10"
in pdb. Perhaps should complain?
---------------------------------------------------
Restricted mode!?
---------------------------------------------------
I'm not *convinced* that except: lines are the only ones
that start
with POP_TOP. OTOH:
1) it's not that significant if they're not
2) when I went looking for counter-examples in compile.c I found
somewhere that looked as if a POP_TOP could start the
line, went
"ooh!" and then noticed that it was compiling an except:
line :-)
---------------------------------------------------
Mondo fragile. What happens if a bus hits Richie?
Maybe add pseudo Python? E.g. how would the jump into/out
of finally
block detection look written using the bytecodehacks?
---------------------------------------------------
*Like* the testing framework. Very neat.
Could split the jump tests out into a new test case.
line 264 of test_trace: why x = 1, not output.append(4)?
Don't test the "must be an integer" part...
Perhaps should test all except line variants:
except:
except ClassName:
except ClassName, var:
(to make sure test fails if compilation details change).
Doesn't test a jump within a finally block.
Faintly worried about silly things like nested finally blocks.
Would like to see a test like this:
def mwh_nested_finally(output):
try:
output.append(2)
finally:
output.append(4)
try:
output.append(6)
finally:
output.append(8)
output.append(9)
mwh_nested_finally.jump = (4, 9)
mwh_nested_finally.output = [2, 9]
(which passes, but that's not the point)
---------------------------------------------------
For checkin:
1) clarify docs
2) add bytecodehacks-stylee code in comments
3) split out into separate test case
4) add a couple tests (all except styles, nested finallys).
----------------------------------------------------------------------
Comment By: Guido van Rossum (gvanrossum)
Date: 2002-11-26 08:44
Message:
Logged In: YES
user_id=6380
Yes, please review! The idea is cool, and it sounds like
he's thought about the consequences (I hope he also checks
the block stack).
----------------------------------------------------------------------
Comment By: Michael Hudson (mwh)
Date: 2002-11-26 05:59
Message:
Logged In: YES
user_id=6656
Er, wow :)
Guido, do you approve of the idea? If so, assign to me and
I'll review & test it.
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=643835&group_id=5470