Fixes after review (PY-26101)

Fix stepping after error jump, fix tests, move code modification to an appropriate function and make it more general
This commit is contained in:
Elizaveta Shashkova
2018-03-13 12:54:48 +03:00
parent e9af010573
commit 5dc1df3c0d
8 changed files with 102 additions and 57 deletions
@@ -560,7 +560,7 @@ class PyDBFrame:
breakpoint = breakpoints_for_file[line]
new_frame = frame
stop = True
if (step_cmd == CMD_STEP_OVER or step_cmd == CMD_SET_NEXT_STATEMENT) and stop_frame is frame and (is_line or is_return):
if step_cmd == CMD_STEP_OVER and stop_frame is frame and (is_line or is_return):
stop = False #we don't stop on breakpoint if we have to stop by step-over (it will be processed later)
elif plugin_manager is not None and main_debugger.has_plugin_line_breaks:
result = plugin_manager.get_breakpoint(main_debugger, self, frame, event, self._args)
@@ -3,7 +3,7 @@ from _pydev_imps._pydev_saved_modules import threading
from _pydevd_bundle.pydevd_additional_thread_info import PyDBAdditionalThreadInfo
from _pydevd_bundle.pydevd_comm import get_global_debugger
from _pydevd_bundle.pydevd_dont_trace_files import DONT_TRACE
from _pydevd_frame_eval.pydevd_frame_tracing import create_code_wrapper, update_globals_dict, dummy_tracing_holder
from _pydevd_frame_eval.pydevd_frame_tracing import pydev_trace_code_wrapper, update_globals_dict, dummy_tracing_holder
from _pydevd_frame_eval.pydevd_modify_bytecode import insert_code
from pydevd_file_utils import get_abs_path_real_path_and_base_from_frame, NORM_PATHS_AND_BASE_CONTAINER
@@ -107,16 +107,13 @@ cdef PyObject* get_bytecode_while_frame_eval(PyFrameObject *frame_obj, int exc):
code_object = frame.f_code
if breakpoints:
breakpoints_to_update = []
injected_code_size = 0
for offset, line in dis.findlinestarts(code_object):
if line in breakpoints:
breakpoint = breakpoints[line]
if code_object not in breakpoint.code_objects:
# This check is needed for generator functions, because after each yield the new frame is created
# but the former code object is used
injected_code = create_code_wrapper(offset + injected_code_size)
success, new_code = insert_code(frame.f_code, injected_code , line)
injected_code_size += len(injected_code.co_code)
success, new_code = insert_code(frame.f_code, pydev_trace_code_wrapper.__code__, line)
if success:
breakpoints_to_update.append(breakpoint)
Py_INCREF(new_code)
@@ -1,5 +1,3 @@
from types import CodeType
import sys
from _pydev_bundle import pydev_log
@@ -8,7 +6,7 @@ from _pydevd_bundle.pydevd_comm import get_global_debugger, CMD_SET_BREAK, CMD_S
from pydevd_file_utils import get_abs_path_real_path_and_base_from_frame, NORM_PATHS_AND_BASE_CONTAINER
from _pydevd_bundle.pydevd_frame import handle_breakpoint_condition, handle_breakpoint_expression
import opcode
class DummyTracingHolder:
dummy_trace_func = None
@@ -90,26 +88,3 @@ def pydev_trace_code_wrapper():
# import this module again, because it's inserted inside user's code
global _pydev_stop_at_break
return _pydev_stop_at_break()
def create_code_wrapper(offset):
co = pydev_trace_code_wrapper.__code__
# 0 + offset LOAD_GLOBAL 0 (_pydev_stop_at_break)
# 2 + offset CALL_FUNCTION 0
# 4 + offset POP_JUMP_IF_TRUE offset
byte_code = [116, 0, 131, 0]
if offset > 0xFF:
byte_code += [opcode.EXTENDED_ARG, offset >> 8]
byte_code += [115, offset & 0xFF]
#below code is just function trailer and gets removed
byte_code += [100, 0, 83, 0]
return CodeType(
co.co_argcount, co.co_kwonlyargcount, co.co_nlocals,
co.co_stacksize,
co.co_flags,
bytes(byte_code),
co.co_consts, co.co_names, co.co_varnames, co.co_filename,
co.co_name, co.co_firstlineno, co.co_lnotab, co.co_freevars,
co.co_cellvars)
@@ -4,9 +4,10 @@ from opcode import opmap, EXTENDED_ARG, HAVE_ARGUMENT
from types import CodeType
MAX_BYTE = 255
RETURN_VALUE_SIZE = 2
def _add_attr_values_from_insert_to_original(original_code, insert_code, insert_code_obj, attribute_name, op_list):
def _add_attr_values_from_insert_to_original(original_code, insert_code, insert_code_list, attribute_name, op_list):
"""
This function appends values of the attribute `attribute_name` of the inserted code to the original values,
and changes indexes inside inserted code. If some bytecode instruction in the inserted code used to call argument
@@ -24,7 +25,7 @@ def _add_attr_values_from_insert_to_original(original_code, insert_code, insert_
orig_value = getattr(original_code, attribute_name)
insert_value = getattr(insert_code, attribute_name)
orig_names_len = len(orig_value)
code_with_new_values = list(insert_code_obj)
code_with_new_values = list(insert_code_list)
offset = 0
while offset < len(code_with_new_values):
op = code_with_new_values[offset]
@@ -155,6 +156,23 @@ def _return_none_fun():
return None
def add_jump_instruction(jump_arg, code_to_insert):
"""
Add additional instruction POP_JUMP_IF_TRUE to implement a proper jump for "set next statement" action
Jump should be done to the beginning of the inserted fragment
:param jump_arg: argument for jump instruction
:param code_to_insert: code to insert
:return: a code to insert with properly added jump instruction
"""
extended_arg_list = []
if jump_arg > MAX_BYTE:
extended_arg_list += [EXTENDED_ARG, jump_arg >> 8]
jump_arg = jump_arg & MAX_BYTE
# remove 'RETURN_VALUE' instruction and add 'POP_JUMP_IF_TRUE' with (if needed) 'EXTENDED_ARG'
return list(code_to_insert.co_code[:-RETURN_VALUE_SIZE]) + extended_arg_list + [opmap['POP_JUMP_IF_TRUE'], jump_arg]
def insert_code(code_to_modify, code_to_insert, before_line):
"""
Insert piece of code `code_to_insert` to `code_to_modify` right inside the line `before_line` before the
@@ -173,19 +191,18 @@ def insert_code(code_to_modify, code_to_insert, before_line):
if line_no == before_line:
offset = off
return_none_size = len(_return_none_fun.__code__.co_code)
code_to_insert_obj = code_to_insert.co_code[:-return_none_size]
code_to_insert_list = add_jump_instruction(offset, code_to_insert)
try:
code_to_insert_obj, new_names = \
_add_attr_values_from_insert_to_original(code_to_modify, code_to_insert, code_to_insert_obj, 'co_names',
code_to_insert_list, new_names = \
_add_attr_values_from_insert_to_original(code_to_modify, code_to_insert, code_to_insert_list, 'co_names',
dis.hasname)
code_to_insert_obj, new_consts = \
_add_attr_values_from_insert_to_original(code_to_modify, code_to_insert, code_to_insert_obj, 'co_consts',
code_to_insert_list, new_consts = \
_add_attr_values_from_insert_to_original(code_to_modify, code_to_insert, code_to_insert_list, 'co_consts',
[opmap['LOAD_CONST']])
code_to_insert_obj, new_vars = \
_add_attr_values_from_insert_to_original(code_to_modify, code_to_insert, code_to_insert_obj, 'co_varnames',
code_to_insert_list, new_vars = \
_add_attr_values_from_insert_to_original(code_to_modify, code_to_insert, code_to_insert_list, 'co_varnames',
dis.haslocal)
new_bytes, all_inserted_code = _update_label_offsets(code_to_modify.co_code, offset, list(code_to_insert_obj))
new_bytes, all_inserted_code = _update_label_offsets(code_to_modify.co_code, offset, list(code_to_insert_list))
new_lnotab = _modify_new_lines(code_to_modify, all_inserted_code)
except ValueError:
+2 -3
View File
@@ -750,8 +750,7 @@ class PyDB:
if curr_func_name == func_name:
line = next_line
if frame.f_trace is None:
frame.f_trace = self.trace_dispatch
frame.f_trace = self.trace_dispatch
frame.f_lineno = line
stop = True
else:
@@ -866,7 +865,7 @@ class PyDB:
info.pydev_state = STATE_SUSPEND
thread.stop_reason = CMD_THREAD_SUSPEND
# return to the suspend state and wait for other command
self.do_wait_suspend(thread, frame, event, arg, "trace", send_suspend_message=False)
self.do_wait_suspend(thread, frame, event, arg, suspend_type, send_suspend_message=False)
return
elif info.pydev_step_cmd == CMD_STEP_RETURN:
@@ -4,6 +4,7 @@ import unittest
from io import StringIO
from _pydevd_frame_eval.pydevd_modify_bytecode import insert_code
from opcode import EXTENDED_ARG
TRACE_MESSAGE = "Trace called"
@@ -12,7 +13,7 @@ def tracing():
def call_tracing():
tracing()
return tracing()
def bar(a, b):
@@ -44,15 +45,36 @@ class TestInsertCode(unittest.TestCase):
code_orig = func_to_modify.__code__
code_to_insert = func_to_insert.__code__
success, result = insert_code(code_orig, code_to_insert, line_number)
self.compare_bytes_sequence(list(result.co_code), list(code_for_check.co_code))
self.compare_bytes_sequence(list(result.co_code), list(code_for_check.co_code), len(code_to_insert.co_code))
def compare_bytes_sequence(self, code1, code2):
def compare_bytes_sequence(self, code1, code2, inserted_code_size):
"""
Compare code after modification and the real code
Since we add POP_JUMP_IF_TRUE instruction, we can't compare modified code and the real code. That's why we
allow some inaccuracies while code comparison
:param code1: result code after modification
:param code2: a real code for checking
:param inserted_code_size: size of inserted code
"""
seq1 = [(offset, op, arg) for offset, op, arg in dis._unpack_opargs(code1)]
seq2 = [(offset, op, arg) for offset, op, arg in dis._unpack_opargs(code2)]
self.assertTrue(len(seq1) == len(seq2), "Bytes sequences have different lengths")
for i in range(len(seq1)):
of, op1, arg1 = seq1[i]
_, op2, arg2 = seq2[i]
if op1 != op2:
if op1 == 115 and op2 == 1:
# it's ok, because we added POP_JUMP_IF_TRUE manually, but it's POP_TOP in the real code
# inserted code - 2 (removed return instruction) - real code inserted
# Jump should be done to the beginning of inserted fragment
self.assertEqual(arg1, of - (inserted_code_size - 2))
continue
elif op1 == EXTENDED_ARG and op2 == 12:
# we added a real UNARY_NOT to balance EXTENDED_ARG added by new jump instruction
# i.e. inserted code size was increased as well
inserted_code_size += 2
continue
self.assertEqual(op1, op2, "Different operators at offset {}".format(of))
if arg1 != arg2:
if op1 in (100, 101, 106, 116):
@@ -488,12 +510,44 @@ class TestInsertCode(unittest.TestCase):
a = a + 1
return a
def foo_check_2():
a = 1
b = 2
if b > 0:
d = a + b
d += 1
b = b - 1 if a > 0 else b + 1
b = b - 1 if a > 0 else b + 1
b = b - 1 if a > 0 else b + 1
b = b - 1 if a > 0 else b + 1
b = b - 1 if a > 0 else b + 1
b = b - 1 if a > 0 else b + 1
b = b - 1 if a > 0 else b + 1
b = b - 1 if a > 0 else b + 1
b = b - 1 if a > 0 else b + 1
b = b - 1 if a > 0 else b + 1
b = b - 1 if a > 0 else b + 1
b = b - 1 if a > 0 else b + 1
b = b - 1 if a > 0 else b + 1
b = b - 1 if a > 0 else b + 1
b = b - 1 if a > 0 else b + 1
not tracing() # add 'not' to balance EXTENDED_ARG when jumping
b = b - 1 if a > 0 else b + 1
b = b - 1 if a > 0 else b + 1
b = b - 1 if a > 0 else b + 1
b = b - 1 if a > 0 else b + 1
a = a + 1
return a
self.check_insert_to_line_with_exec(foo, tracing, foo.__code__.co_firstlineno + 2)
sys.stdout = self.original_stdout
self.check_insert_to_line_by_symbols(foo, call_tracing, foo.__code__.co_firstlineno + 3,
foo_check.__code__)
self.check_insert_to_line_by_symbols(foo, call_tracing, foo.__code__.co_firstlineno + 21,
foo_check_2.__code__)
finally:
sys.stdout = self.original_stdout
sys.stdout = self.original_stdout
@@ -121,7 +121,7 @@ public class PyCythonExtensionWarning {
throw new ExecutionException("Python Run Configuration should be selected");
}
AbstractPythonRunConfiguration runConfiguration = (AbstractPythonRunConfiguration)configuration;
final String sdkPath = runConfiguration.getSdkHome();
final String interpreterPath = runConfiguration.getInterpreterPath();
final String helpersPath = PythonHelpersLocator.getHelpersRoot().getPath();
final String cythonExtensionsDir = PyDebugRunner.CYTHON_EXTENSIONS_DIR;
@@ -129,7 +129,7 @@ public class PyCythonExtensionWarning {
{"build_ext", "--build-lib", cythonExtensionsDir, "--build-temp", String.format("%s%sbuild", cythonExtensionsDir, File.separator)};
final List<String> cmdline = new ArrayList<>();
cmdline.add(sdkPath);
cmdline.add(interpreterPath);
cmdline.add(FileUtil.join(helpersPath, FileUtil.toSystemDependentName(SETUP_CYTHON_PATH)));
cmdline.addAll(Arrays.asList(cythonArgs));
LOG.info("Compile Cython Extensions " + StringUtil.join(cmdline, " "));
@@ -138,8 +138,8 @@ public class PyCythonExtensionWarning {
PythonEnvUtil.addToPythonPath(environment, cythonExtensionsDir);
PythonEnvUtil.setPythonUnbuffered(environment);
PythonEnvUtil.setPythonDontWriteBytecode(environment);
if (sdkPath != null) {
PythonEnvUtil.resetHomePathChanges(sdkPath, environment);
if (interpreterPath != null) {
PythonEnvUtil.resetHomePathChanges(interpreterPath, environment);
}
GeneralCommandLine commandLine = new GeneralCommandLine(cmdline).withEnvironment(environment);
@@ -1295,20 +1295,23 @@ public class PythonDebuggerTest extends PyEnvTestCase {
Pair<Boolean, String> pair = setNextStatement(7);
waitForPause();
assertTrue(pair.first);
eval("x").hasValue("1");
eval("x").hasValue("0");
// try to jump into a loop
pair = setNextStatement(9);
// do not wait for pause here, because we don't refresh suspension for incorrect jumps
assertFalse(pair.first);
assertTrue(pair.second.startsWith("Error:"));
stepOver();
waitForPause();
eval("x").hasValue("2");
resume();
waitForPause();
eval("a").hasValue("3");
eval("a").hasValue("2");
// jump inside a function
pair = setNextStatement(2);
waitForPause();
assertTrue(pair.first);
eval("a").hasValue("6");
eval("a").hasValue("2");
resume();
}
});