From eee0041cc2a996f95bb5ffc16dd45258f37e40e7 Mon Sep 17 00:00:00 2001 From: Xavier de Gaye Date: Tue, 27 Feb 2018 13:58:07 +0100 Subject: [PATCH 1/4] bpo-17288: Prevent jumps from 'return' and 'exception' trace events --- Lib/test/test_sys_settrace.py | 53 ++++++++++++++++--- .../2018-02-27-13-36-21.bpo-17288.Gdj24S.rst | 1 + Objects/frameobject.c | 38 ++++++++++++- 3 files changed, 84 insertions(+), 8 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2018-02-27-13-36-21.bpo-17288.Gdj24S.rst diff --git a/Lib/test/test_sys_settrace.py b/Lib/test/test_sys_settrace.py index e6eb80ad86a88d..439f01f78ccf8c 100644 --- a/Lib/test/test_sys_settrace.py +++ b/Lib/test/test_sys_settrace.py @@ -556,16 +556,21 @@ def g(frame, event, arg): class JumpTracer: """Defines a trace function that jumps from one place to another.""" - def __init__(self, function, jumpFrom, jumpTo): - self.function = function + def __init__(self, function, jumpFrom, jumpTo, event='line'): + self.code = function.__code__ self.jumpFrom = jumpFrom self.jumpTo = jumpTo + self.event = event self.done = False def trace(self, frame, event, arg): - if not self.done and frame.f_code == self.function.__code__: + # Jumps can also be traced in a nested function (e.g + # no_jump_from_call_event()). + if (not self.done and (frame.f_code == self.code or + (frame.f_back and frame.f_back.f_code == self.code))): firstLine = frame.f_code.co_firstlineno - if event == 'line' and frame.f_lineno == firstLine + self.jumpFrom: + if (event == self.event and + frame.f_lineno == firstLine + self.jumpFrom): # Cope with non-integer self.jumpTo (because of # no_jump_to_non_integers below). try: @@ -582,6 +587,25 @@ def no_jump_to_non_integers(output): except ValueError as e: output.append('integer' in str(e)) +def no_jump_from_call_event(output): + def nested(): + lineno = 1 + nested() + +def no_jump_from_return_event(output): + lineno = 1 + return + +def no_jump_from_yield(output): + def gen(): + lineno = 1 + yield 1 + next(gen()) + +def no_jump_from_exception_event(output): + lineno = 1 + 1 / 0 + # This verifies that you can't set f_lineno via _getframe or similar # trickery. def no_jump_without_trace_function(): @@ -609,8 +633,9 @@ def compare_jump_output(self, expected, received): "Expected: " + repr(expected) + "\n" + "Received: " + repr(received)) - def run_test(self, func, jumpFrom, jumpTo, expected, error=None): - tracer = JumpTracer(func, jumpFrom, jumpTo) + def run_test(self, func, jumpFrom, jumpTo, expected, error=None, + event='line'): + tracer = JumpTracer(func, jumpFrom, jumpTo, event) sys.settrace(tracer.trace) output = [] if error is None: @@ -1076,6 +1101,22 @@ class fake_function: sys.settrace(None) self.compare_jump_output([2, 3, 2, 3, 4], namespace["output"]) + def test_no_jump_from_call(self): + self.run_test(no_jump_from_call_event, 0, 1, [], event='call', + error=(ValueError, "can't jump from the 'call' trace" + " event of a new frame")) + + def test_no_jump_from_return_exception_events(self): + for func, event in ((no_jump_from_return_event, 'return'), + (no_jump_from_exception_event, 'exception')): + with self.subTest(event=event): + self.run_test(func, 2, 1, [], event=event, error=(ValueError, + "can only jump from a 'line' trace event")) + + def test_no_jump_from_yield(self): + self.run_test(no_jump_from_yield, 2, 1, [], event='return', + error=(ValueError, "can't jump from a yield statement")) + if __name__ == "__main__": unittest.main() diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-02-27-13-36-21.bpo-17288.Gdj24S.rst b/Misc/NEWS.d/next/Core and Builtins/2018-02-27-13-36-21.bpo-17288.Gdj24S.rst new file mode 100644 index 00000000000000..ce9e84c403135e --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2018-02-27-13-36-21.bpo-17288.Gdj24S.rst @@ -0,0 +1 @@ +Prevent jumps from 'return' and 'exception' trace events. diff --git a/Objects/frameobject.c b/Objects/frameobject.c index 3083e5b6445c29..e14222daee90e9 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -59,6 +59,9 @@ frame_getlineno(PyFrameObject *f, void *closure) * o 'try'/'for'/'while' blocks can't be jumped into because the blockstack * needs to be set up before their code runs, and for 'for' loops the * iterator needs to be on the stack. + * o Jumps cannot be made from within a trace function invoked with a + * 'return' or 'exception' event since the eval loop has been exited at + * that time. */ static int frame_setlineno(PyFrameObject *f, PyObject* p_new_lineno) @@ -94,13 +97,34 @@ frame_setlineno(PyFrameObject *f, PyObject* p_new_lineno) return -1; } + /* Upon the 'call' trace event of a new frame, f->f_lasti is -1 and + * f->f_trace is NULL, check first on the first condition. + * Forbidding jumps from the 'call' event of a new frame is a side effect + * of allowing to set f_lineno only from trace functions. */ + if (f->f_lasti == -1) + { + PyErr_Format(PyExc_ValueError, + "can't jump from the 'call' trace event of a new frame"); + return -1; + } + /* You can only do this from within a trace function, not via * _getframe or similar hackery. */ if (!f->f_trace) { PyErr_Format(PyExc_ValueError, - "f_lineno can only be set by a" - " line trace function"); + "f_lineno can only be set by a trace function"); + return -1; + } + + /* Forbid jumps upon a 'return' trace event (except after executing a + * YIELD_VALUE or YIELD_FROM opcode, f_stacktop is not NULL in that case) + * and upon an 'exception' trace event. + * Jumps from 'call' trace events have already been forbidden above for new + * frames, so this check does not change anything for 'call' events. */ + if (f->f_stacktop == NULL) { + PyErr_SetString(PyExc_ValueError, + "can only jump from a 'line' trace event"); return -1; } @@ -159,6 +183,16 @@ frame_setlineno(PyFrameObject *f, PyObject* p_new_lineno) /* We're now ready to look at the bytecode. */ PyBytes_AsStringAndSize(f->f_code->co_code, (char **)&code, &code_len); + + /* The trace function is called with a 'return' trace event after the + * execution of a yield statement. */ + assert(f->f_lasti != -1); + if (code[f->f_lasti] == YIELD_VALUE || code[f->f_lasti] == YIELD_FROM) { + PyErr_SetString(PyExc_ValueError, + "can't jump from a yield statement"); + return -1; + } + min_addr = Py_MIN(new_lasti, f->f_lasti); max_addr = Py_MAX(new_lasti, f->f_lasti); From 69c1bcbc5986f14fbcf7ee3399da3f6f8a932665 Mon Sep 17 00:00:00 2001 From: Xavier de Gaye Date: Sun, 11 Mar 2018 11:20:13 +0100 Subject: [PATCH 2/4] The new tests use now the jump_test() decorator Fix also a bug in the previous implementation of the PR where jumpFrom/jumpTo were systematically incremented by 1 by the jump_test() decorator without taking into account that JumpTracer allows now jumpFrom/jumpTo from within nested functions. --- Lib/test/test_sys_settrace.py | 110 ++++++++++++++++++++-------------- Objects/frameobject.c | 6 +- 2 files changed, 66 insertions(+), 50 deletions(-) diff --git a/Lib/test/test_sys_settrace.py b/Lib/test/test_sys_settrace.py index 439f01f78ccf8c..f3764e34e93e99 100644 --- a/Lib/test/test_sys_settrace.py +++ b/Lib/test/test_sys_settrace.py @@ -556,19 +556,43 @@ def g(frame, event, arg): class JumpTracer: """Defines a trace function that jumps from one place to another.""" - def __init__(self, function, jumpFrom, jumpTo, event='line'): + def __init__(self, function, jumpFrom, jumpTo, event='line', + decorated=False): self.code = function.__code__ self.jumpFrom = jumpFrom self.jumpTo = jumpTo self.event = event + self.decorated = decorated + self.function_firstLine = None self.done = False def trace(self, frame, event, arg): - # Jumps can also be traced in a nested function (e.g - # no_jump_from_call_event()). + # frame.f_code.co_firstlineno is the first line of the decorator when + # 'function' is decorated and the decorator may be written using + # multiple physical lines when it is too long. Use the first line + # trace event in 'function' to find the first line of 'function'. + if (self.decorated and frame.f_code == self.code and + event == 'line' and self.function_firstLine is None): + self.function_firstLine = frame.f_lineno - 1 + + # Jumps can also be traced in one level nested functions (e.g + # test_no_jump_from_call()). if (not self.done and (frame.f_code == self.code or (frame.f_back and frame.f_back.f_code == self.code))): firstLine = frame.f_code.co_firstlineno + if self.decorated and frame.f_code == self.code: + if self.function_firstLine is not None: + firstLine = self.function_firstLine + else: + assert event == 'call' + assert frame.f_lineno == firstLine + # The jump is done on a call event and it is not possible + # to know how many physical lines are used by the + # decorator, so just prevent the jump from the decorator. + # The jump has to be made from a nested function, see + # test_no_jump_from_call(). + if event == self.event and self.jumpFrom == 0: + firstLine += 1 if (event == self.event and frame.f_lineno == firstLine + self.jumpFrom): # Cope with non-integer self.jumpTo (because of @@ -587,25 +611,6 @@ def no_jump_to_non_integers(output): except ValueError as e: output.append('integer' in str(e)) -def no_jump_from_call_event(output): - def nested(): - lineno = 1 - nested() - -def no_jump_from_return_event(output): - lineno = 1 - return - -def no_jump_from_yield(output): - def gen(): - lineno = 1 - yield 1 - next(gen()) - -def no_jump_from_exception_event(output): - lineno = 1 - 1 / 0 - # This verifies that you can't set f_lineno via _getframe or similar # trickery. def no_jump_without_trace_function(): @@ -634,8 +639,8 @@ def compare_jump_output(self, expected, received): "Received: " + repr(received)) def run_test(self, func, jumpFrom, jumpTo, expected, error=None, - event='line'): - tracer = JumpTracer(func, jumpFrom, jumpTo, event) + event='line', decorated=False): + tracer = JumpTracer(func, jumpFrom, jumpTo, event, decorated) sys.settrace(tracer.trace) output = [] if error is None: @@ -646,15 +651,15 @@ def run_test(self, func, jumpFrom, jumpTo, expected, error=None, sys.settrace(None) self.compare_jump_output(expected, output) - def jump_test(jumpFrom, jumpTo, expected, error=None): + def jump_test(jumpFrom, jumpTo, expected, error=None, event='line'): """Decorator that creates a test that makes a jump from one place to another in the following code. """ def decorator(func): @wraps(func) def test(self): - # +1 to compensate a decorator line - self.run_test(func, jumpFrom+1, jumpTo+1, expected, error) + self.run_test(func, jumpFrom, jumpTo, expected, + error=error, event=event, decorated=True) return test return decorator @@ -747,11 +752,12 @@ def test_jump_infinite_while_loop(output): output.append(3) output.append(4) - @jump_test(2, 3, [1, 3]) + @jump_test(3, 4, [1, 2, 4]) def test_jump_forwards_out_of_with_block(output): - with tracecontext(output, 1): - output.append(2) - output.append(3) + output.append(1) + with tracecontext(output, 2): + output.append(3) + output.append(4) @jump_test(3, 1, [1, 2, 1, 2, 3, -2]) def test_jump_backwards_out_of_with_block(output): @@ -1101,21 +1107,33 @@ class fake_function: sys.settrace(None) self.compare_jump_output([2, 3, 2, 3, 4], namespace["output"]) - def test_no_jump_from_call(self): - self.run_test(no_jump_from_call_event, 0, 1, [], event='call', - error=(ValueError, "can't jump from the 'call' trace" - " event of a new frame")) - - def test_no_jump_from_return_exception_events(self): - for func, event in ((no_jump_from_return_event, 'return'), - (no_jump_from_exception_event, 'exception')): - with self.subTest(event=event): - self.run_test(func, 2, 1, [], event=event, error=(ValueError, - "can only jump from a 'line' trace event")) - - def test_no_jump_from_yield(self): - self.run_test(no_jump_from_yield, 2, 1, [], event='return', - error=(ValueError, "can't jump from a yield statement")) + @jump_test(0, 1, [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(2) + nested() + + @jump_test(2, 1, [1], event='return', error=(ValueError, + "can only jump from a 'line' trace event")) + def test_no_jump_from_return_event(output): + output.append(1) + return + + @jump_test(2, 1, [1], event='exception', error=(ValueError, + "can only jump from a 'line' trace event")) + def test_no_jump_from_exception_event(output): + output.append(1) + 1 / 0 + + @jump_test(2, 1, [1], event='return', error=(ValueError, + "can't jump from a yield statement")) + def test_no_jump_from_yield(output): + def gen(): + output.append(1) + yield 1 + next(gen()) if __name__ == "__main__": diff --git a/Objects/frameobject.c b/Objects/frameobject.c index e14222daee90e9..26de8a9b622cf4 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -101,8 +101,7 @@ frame_setlineno(PyFrameObject *f, PyObject* p_new_lineno) * f->f_trace is NULL, check first on the first condition. * Forbidding jumps from the 'call' event of a new frame is a side effect * of allowing to set f_lineno only from trace functions. */ - if (f->f_lasti == -1) - { + if (f->f_lasti == -1) { PyErr_Format(PyExc_ValueError, "can't jump from the 'call' trace event of a new frame"); return -1; @@ -110,8 +109,7 @@ frame_setlineno(PyFrameObject *f, PyObject* p_new_lineno) /* You can only do this from within a trace function, not via * _getframe or similar hackery. */ - if (!f->f_trace) - { + if (!f->f_trace) { PyErr_Format(PyExc_ValueError, "f_lineno can only be set by a trace function"); return -1; From 8cadc6ef5f0dc4bea1d8d5252bb51beba2c335a3 Mon Sep 17 00:00:00 2001 From: Xavier de Gaye Date: Mon, 12 Mar 2018 09:58:19 +0100 Subject: [PATCH 3/4] JumpTestCase line numbers are relative to the first line of the testing function --- Lib/test/test_sys_settrace.py | 67 +++++++++++++---------------------- 1 file changed, 24 insertions(+), 43 deletions(-) diff --git a/Lib/test/test_sys_settrace.py b/Lib/test/test_sys_settrace.py index f3764e34e93e99..37be520aab39c3 100644 --- a/Lib/test/test_sys_settrace.py +++ b/Lib/test/test_sys_settrace.py @@ -562,43 +562,23 @@ def __init__(self, function, jumpFrom, jumpTo, event='line', self.jumpFrom = jumpFrom self.jumpTo = jumpTo self.event = event - self.decorated = decorated - self.function_firstLine = None + self.firstLine = None if decorated else self.code.co_firstlineno self.done = False def trace(self, frame, event, arg): - # frame.f_code.co_firstlineno is the first line of the decorator when - # 'function' is decorated and the decorator may be written using - # multiple physical lines when it is too long. Use the first line - # trace event in 'function' to find the first line of 'function'. - if (self.decorated and frame.f_code == self.code and - event == 'line' and self.function_firstLine is None): - self.function_firstLine = frame.f_lineno - 1 - - # Jumps can also be traced in one level nested functions (e.g - # test_no_jump_from_call()). - if (not self.done and (frame.f_code == self.code or - (frame.f_back and frame.f_back.f_code == self.code))): - firstLine = frame.f_code.co_firstlineno - if self.decorated and frame.f_code == self.code: - if self.function_firstLine is not None: - firstLine = self.function_firstLine - else: - assert event == 'call' - assert frame.f_lineno == firstLine - # The jump is done on a call event and it is not possible - # to know how many physical lines are used by the - # decorator, so just prevent the jump from the decorator. - # The jump has to be made from a nested function, see - # test_no_jump_from_call(). - if event == self.event and self.jumpFrom == 0: - firstLine += 1 - if (event == self.event and - frame.f_lineno == firstLine + self.jumpFrom): - # Cope with non-integer self.jumpTo (because of - # no_jump_to_non_integers below). + 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 = firstLine + self.jumpTo + frame.f_lineno = self.firstLine + self.jumpTo except TypeError: frame.f_lineno = self.jumpTo self.done = True @@ -752,12 +732,11 @@ def test_jump_infinite_while_loop(output): output.append(3) output.append(4) - @jump_test(3, 4, [1, 2, 4]) + @jump_test(2, 3, [1, 3]) def test_jump_forwards_out_of_with_block(output): - output.append(1) - with tracecontext(output, 2): - output.append(3) - output.append(4) + with tracecontext(output, 1): + output.append(2) + output.append(3) @jump_test(3, 1, [1, 2, 1, 2, 3, -2]) def test_jump_backwards_out_of_with_block(output): @@ -1107,13 +1086,14 @@ class fake_function: sys.settrace(None) self.compare_jump_output([2, 3, 2, 3, 4], namespace["output"]) - @jump_test(0, 1, [1], event='call', error=(ValueError, "can't jump from" + @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(2) + output.append(3) nested() + output.append(5) @jump_test(2, 1, [1], event='return', error=(ValueError, "can only jump from a 'line' trace event")) @@ -1127,13 +1107,14 @@ def test_no_jump_from_exception_event(output): output.append(1) 1 / 0 - @jump_test(2, 1, [1], event='return', error=(ValueError, + @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(1) - yield 1 + output.append(2) + yield 3 next(gen()) + output.append(5) if __name__ == "__main__": From a6ec0e054182d08dac7bc7b938c54131d97cf817 Mon Sep 17 00:00:00 2001 From: Xavier de Gaye Date: Mon, 12 Mar 2018 23:05:48 +0100 Subject: [PATCH 4/4] Restore previous comments --- Lib/test/test_sys_settrace.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Lib/test/test_sys_settrace.py b/Lib/test/test_sys_settrace.py index 37be520aab39c3..1a1a3616f3523a 100644 --- a/Lib/test/test_sys_settrace.py +++ b/Lib/test/test_sys_settrace.py @@ -568,6 +568,10 @@ def __init__(self, function, jumpFrom, jumpTo, event='line', def trace(self, frame, event, arg): if self.done: return + # frame.f_code.co_firstlineno is the first line of the decorator when + # 'function' is decorated and the decorator may be written using + # multiple physical lines when it is too long. Use the first line + # trace event in 'function' to find the first line of 'function'. if (self.firstLine is None and frame.f_code == self.code and event == 'line'): self.firstLine = frame.f_lineno - 1 @@ -577,6 +581,8 @@ def trace(self, frame, event, arg): while f is not None and f.f_code != self.code: f = f.f_back if f is not None: + # Cope with non-integer self.jumpTo (because of + # no_jump_to_non_integers below). try: frame.f_lineno = self.firstLine + self.jumpTo except TypeError: