[3.7] bpo-17288: Prevent jumps from 'return' and 'exception' trace events by xdegaye · Pull Request #5928 · python/cpython (original) (raw)

Thank you @xdegaye! This is very nice PR. I like how it fixes the non-trivial bug. I have learned something new when reading it.

But I don't like the complexity of the new implementation of the trace function and the fact that it breaks one existing test. This code looks complicated and fragile. I afraid that it will be hard to modify it for supporting new tests or fix it if some details of frames or code objects will be changed. If in some implementation list.append() is implemented in Python this will break tests since the tracer will be confused by new frames.

I suggest to use the following implementation:

def trace(self, frame, event, arg):
    if self.done:
        return
    if (self.firstLine is None and frame.f_code == self.code and
            event == 'line'):
        self.firstLine = frame.f_lineno - 1
    if (event == self.event and self.firstLine and
            frame.f_lineno == self.firstLine + self.jumpFrom):
        f = frame
        while f is not None and f.f_code != self.code:
            f = f.f_back
        if f is not None:
            try:
                frame.f_lineno = self.firstLine + self.jumpTo
            except TypeError:
                frame.f_lineno = self.jumpTo
            self.done = True
    return self.trace

and in the constructor:

    self.firstLine = None if decorated else self.code.co_firstlineno

test_jump_forwards_out_of_with_block can be restored, and two new test should be written as:

@jump_test(2, 3, [1], event='call', error=(ValueError, "can't jump from"
           " the 'call' trace event of a new frame"))
def test_no_jump_from_call(output):
    output.append(1)
    def nested():
        output.append(3)
    nested()
    output.append(5)

@jump_test(3, 2, [2], event='return', error=(ValueError,
           "can't jump from a yield statement"))
def test_no_jump_from_yield(output):
    def gen():
        output.append(2)
        yield 3
    next(gen())
    output.append(5)

Line numbers are always relative to the first line of the testing function which is determined by the first executed line if decorated is true.