bpo-22577: in pdb, prevent changes to locals from being lost after a jump

[scotchka]

The Pdb.setup method creates a curframe_locals attribute to prevent subsequent .f_locals access from undoing changes made during the debugging session. However, the existing implementation binds curframe_locals to .f_locals without making a copy, so changes are lost after a jump because it invokes the Bdb.format_stack_entry method which does contain an .f_locals access.

The proposed fix is to assign to curframe_locals a copy of .f_locals.

I've added a test to test_pdb that reproduces the original issue.

https://bugs.python.org/issue22577

[the-knights-who-say-ni]

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for your contribution, we look forward to reviewing it!

[rhettinger]

Wouldn't this leave the actual locals used in downstream code inconsistent with that reported by pdb?

FWIW, pdb is consistent with the way assignments to locals() are handled in regular functions:

>>> def f(x):
	x += 1
	locals()['x'] = 20
	print(locals())
	return x

>>> f(5)
{'x': 6}

>>> help(locals)
Help on built-in function locals in module builtins:

locals()
    Return a dictionary containing the current scope's local variables.
    
    NOTE: Whether or not updates to this dictionary will affect name lookups in
    the local scope and vice-versa is *implementation dependent* and not
    covered by any backwards compatibility guarantees.

It is the nature of locals to not be guaranteed to be updatable.

[xdegaye]

LGTM

@rhettinger this problem is not related to the locals() function and is specific to pdb:
the frame locals are updated by the call_trampoline()
funtion that calls PyFrame_LocalsToFast() upon returning from the trace function.

The example in bpo-22577, that this PR is referring to, illustrates the problem: the value of x, assigned to within a pdb session, is different whether the pdb jump command is used or not.

[xdegaye]

LGTM

Oops ! This change introduces a regression (missed because it seems there is no pdb test on the change of a local variable).

With the change, pdb prints 1 instead 123 as expected and as occuring in the latest python versions:

>>> def f(x):
...   from pdb import set_trace; set_trace()
...   print(x)
... 
>>> f(1)
> <stdin>(3)f()
(Pdb) x = 123
(Pdb) step
1

The test added by the PR succeeds because the following pdb commands occur in the same interaction, i.e. within the same python trace function, and do not test the changes occuring in frame->f_locals:

x = 123
jump 4
x

[scotchka]

Thanks for the catch! I will add the regression test to the PR and work on a solution. Any insight on approach would be appreciated.

[xdegaye]

IMHO the copy of self.curframe.f_locals should be done before the self._cmdloop() statement in interaction instead of in setup() as this method is used elsewhere. And self.curframe.f_locals should be updated with the changes that occured in self.curframe_locals after this statement (i.e. before resuming execution of the program).

However self.curframe_locals may have been changed meanwhile by do_up() and do_down() (see bpo-9633) so it is a little bit more involved than that, and not fixing this last problem at the same time may introduce an annnoying regression.

[scotchka]

I confirmed that the proposed implementation of PEP 558 fixes the bug. Hence I'm closing this PR.