Fixes issues in frame eval (#1082, #1083). (#1085)

src/ptvsd/_vendored/pydevd/_pydevd_frame_eval/pydevd_frame_evaluator.pyx

@@ -74,9 +74,6 @@ cdef class FuncCodeInfo:
     cdef public bint breakpoint_found
     cdef public object new_code
     
-    # Lines with the breakpoints which were actually added to this function.
-    cdef public set breakpoints_created
-    
     # When breakpoints_mtime != PyDb.mtime the validity of breakpoints have
     # to be re-evaluated (if invalid a new FuncCodeInfo must be created and
     # tracing can't be disabled for the related frames).
@@ -92,7 +89,6 @@ cdef class FuncCodeInfo:
         # where needed, so, fallback to tracing.
         self.breakpoint_found = False
         self.new_code = None
-        self.breakpoints_created = set()
         self.breakpoints_mtime = -1
 
 
@@ -184,6 +180,7 @@ cdef FuncCodeInfo get_func_code_info(PyCodeObject * code_obj):
 
     cdef str co_filename = <str> code_obj.co_filename
     cdef str co_name = <str> code_obj.co_name
+    cdef set break_at_lines
 
     func_code_info = FuncCodeInfo()
     func_code_info.breakpoints_mtime = main_debugger.mtime
@@ -214,23 +211,27 @@ cdef FuncCodeInfo get_func_code_info(PyCodeObject * code_obj):
             if breakpoints:
                 # if DEBUG:
                 #    print('found breakpoints', code_obj_py.co_name, breakpoints)
+                break_at_lines = set()
+                new_code = None
                 for offset, line in dis.findlinestarts(code_obj_py):
                     if line in breakpoints:
-                        breakpoint = breakpoints[line]
+                        # breakpoint = breakpoints[line]
                         # if DEBUG:
                         #    print('created breakpoint', code_obj_py.co_name, line)
-                        func_code_info.breakpoints_created.add(line)
                         func_code_info.breakpoint_found = True
+                        break_at_lines.add(line)
     
                         success, new_code = insert_code(
-                            code_obj_py, create_pydev_trace_code_wrapper(line), line)
+                            code_obj_py, create_pydev_trace_code_wrapper(line), line, tuple(break_at_lines))
+                        code_obj_py = new_code
     
-                        if success:
-                            func_code_info.new_code = new_code
-                            code_obj_py = new_code
-                        else:
+                        if not success:
                             func_code_info.new_code = None
                             break
+                else:
+                    # Ok, all succeeded, set to generated code object.
+                    func_code_info.new_code = new_code
+                    
 
     Py_INCREF(func_code_info)
     _PyCode_SetExtra(<PyObject *> code_obj, _code_extra_index, <PyObject *> func_code_info)
@@ -279,7 +280,7 @@ cdef PyObject * get_bytecode_while_frame_eval(PyFrameObject * frame_obj, int exc
         return _PyEval_EvalFrameDefault(frame_obj, exc)
 
     # frame = <object> frame_obj
-    # DEBUG = frame.f_code.co_filename.endswith('_debugger_case_multiprocessing.py')
+    # DEBUG = frame.f_code.co_filename.endswith('_debugger_case_tracing.py')
     # if DEBUG:
     #     print('get_bytecode_while_frame_eval', frame.f_lineno, frame.f_code.co_name, frame.f_code.co_filename)
     
@@ -302,6 +303,8 @@ cdef PyObject * get_bytecode_while_frame_eval(PyFrameObject * frame_obj, int exc
                 main_debugger.signature_factory or \
                 additional_info.pydev_step_cmd == CMD_STEP_OVER and main_debugger.show_return_values and frame.f_back is additional_info.pydev_step_stop:
             
+            # if DEBUG:
+            #     print('get_bytecode_while_frame_eval enabled trace')
             if thread_info.thread_trace_func is not None:
                 frame.f_trace = thread_info.thread_trace_func
             else:

src/ptvsd/_vendored/pydevd/_pydevd_frame_eval/pydevd_modify_bytecode.py

@@ -51,12 +51,6 @@ def _modify_new_lines(code_to_modify, offset, code_to_insert):
     # There's a nice overview of co_lnotab in
     # https://github.com/python/cpython/blob/3.6/Objects/lnotab_notes.txt
 
-    if code_to_modify.co_firstlineno == 1 and offset == 0 and code_to_modify.co_name == '<module>':
-        # There's a peculiarity here: if a breakpoint is added in the first line of a module, we
-        # can't replace the code because we require a line event to stop and the live event
-        # was already generated, so, fallback to tracing.
-        return None
-
     new_list = list(code_to_modify.co_lnotab)
     if not new_list:
         # Could happen on a lambda (in this case, a breakpoint in the lambda should fallback to
@@ -192,24 +186,38 @@ def add_jump_instruction(jump_arg, code_to_insert):
 _created = {}
 
 
-def insert_code(code_to_modify, code_to_insert, before_line):
-    # This check is needed for generator functions, because after each yield a new frame is created
-    # but the former code object is used.
+def insert_code(code_to_modify, code_to_insert, before_line, all_lines_with_breaks=()):
+    '''
+    :param all_lines_with_breaks:
+        tuple(int) a tuple with all the breaks in the given code object (this method is expected
+        to be called multiple times with different lines to add multiple breakpoints, so, the
+        variable `before_line` should have the current breakpoint an the all_lines_with_breaks
+        should have all the breakpoints added so far (including the `before_line`).
+    '''
+    if not all_lines_with_breaks:
+        # Backward-compatibility with signature which received only one line.
+        all_lines_with_breaks = (before_line,)
 
-    ok_and_curr_before_line = _created.get(code_to_modify)
-    if ok_and_curr_before_line is not None:
-        ok, curr_before_line = ok_and_curr_before_line
-        if not ok:
-            return False, code_to_modify
+    # The cache is needed for generator functions, because after each yield a new frame
+    # is created but the former code object is used (so, check if code_to_modify is
+    # already there and if not cache based on the new code generated).
 
-        if curr_before_line == before_line:
-            return True, code_to_modify
+    # print('inserting code', before_line, all_lines_with_breaks)
+    # dis.dis(code_to_modify)
 
-        return False, code_to_modify
+    ok_and_new_code = _created.get((code_to_modify, all_lines_with_breaks))
+    if ok_and_new_code is not None:
+        return ok_and_new_code
 
     ok, new_code = _insert_code(code_to_modify, code_to_insert, before_line)
-    _created[new_code] = ok, before_line
-    return ok, new_code
+
+    # print('insert code ok', ok)
+    # dis.dis(new_code)
+
+    # Note: caching with new code!
+    cache_key = new_code, all_lines_with_breaks
+    _created[cache_key] = (ok, new_code)
+    return _created[cache_key]
 
 
 def _insert_code(code_to_modify, code_to_insert, before_line):
@@ -223,6 +231,16 @@ def _insert_code(code_to_modify, code_to_insert, before_line):
     :return: boolean flag whether insertion was successful, modified code
     """
     linestarts = dict(dis.findlinestarts(code_to_modify))
+    if not linestarts:
+        return False, code_to_modify
+
+    if code_to_modify.co_name == '<module>':
+        # There's a peculiarity here: if a breakpoint is added in the first line of a module, we
+        # can't replace the code because we require a line event to stop and the line event
+        # was already generated, so, fallback to tracing.
+        if before_line == min(linestarts.values()):
+            return False, code_to_modify
+
     if before_line not in linestarts.values():
         return False, code_to_modify
 

src/ptvsd/_vendored/pydevd/tests_python/resources/_debugger_case_tracing.py

@@ -0,0 +1,14 @@
+
+a = 1
+b = 2
+c = 3
+
+
+def foo():
+    a = 1
+    b = 2
+    c = 3
+
+
+foo()
+print('TEST SUCEEDED')

src/ptvsd/_vendored/pydevd/tests_python/test_debugger.py

@@ -1982,7 +1982,10 @@ def _get_generator_cases():
         return ('_debugger_case_generator_py2.py',)
     else:
         # On py3 we should check both versions.
-        return ('_debugger_case_generator_py2.py', '_debugger_case_generator_py3.py')
+        return (
+            '_debugger_case_generator_py2.py',
+            '_debugger_case_generator_py3.py',
+        )
 
 
 @pytest.mark.parametrize("filename", _get_generator_cases())
@@ -2127,7 +2130,7 @@ def test_multiprocessing(case_setup_multiprocessing):
         writer.write_run_thread(hit2.thread_id)
         writer.finished_ok = True
 
-    
+
 @pytest.mark.skipif(not IS_CPYTHON, reason='CPython only test.')
 def test_remote_debugger_basic(case_setup_remote):
     with case_setup_remote.test_file('_debugger_case_remote.py') as writer:
@@ -2516,6 +2519,53 @@ def test_top_level_exceptions_on_attach(case_setup_remote, check_scenario):
         writer.log.append('finished ok')
         writer.finished_ok = True
 
+
+@pytest.mark.parametrize('filename, break_at_lines', [
+    # Known limitation: when it's added to the first line of the module, the
+    # module becomes traced.
+    ('_debugger_case_tracing.py', {2: 'trace'}),
+
+    ('_debugger_case_tracing.py', {3: 'frame_eval'}),
+    ('_debugger_case_tracing.py', {4: 'frame_eval'}),
+    ('_debugger_case_tracing.py', {2: 'trace', 4: 'trace'}),
+
+    ('_debugger_case_tracing.py', {8: 'frame_eval'}),
+    ('_debugger_case_tracing.py', {9: 'frame_eval'}),
+    ('_debugger_case_tracing.py', {10: 'frame_eval'}),
+
+    # Note: second frame eval hit is actually a trace because after we
+    # hit the first frame eval we don't actually stop tracing a given
+    # frame (known limitation to be fixed in the future).
+    # -- needs a better test
+    ('_debugger_case_tracing.py', {8: 'frame_eval', 10: 'frame_eval'}),
+])
+def test_frame_eval_limitations(case_setup, filename, break_at_lines):
+    '''
+    Test with limitations to be addressed in the future.
+    '''
+    with case_setup.test_file(filename) as writer:
+        for break_at_line in break_at_lines:
+            writer.write_add_breakpoint(break_at_line)
+
+        writer.log.append('making initial run')
+        writer.write_make_initial_run()
+
+        for break_at_line, break_mode in break_at_lines.items():
+            writer.log.append('waiting for breakpoint hit')
+            hit = writer.wait_for_breakpoint_hit()
+            thread_id = hit.thread_id
+
+            if IS_PY36_OR_GREATER and TEST_CYTHON:
+                assert hit.suspend_type == break_mode
+            else:
+                # Before 3.6 frame eval is not available.
+                assert hit.suspend_type == 'trace'
+
+            writer.log.append('run thread')
+            writer.write_run_thread(thread_id)
+
+        writer.finished_ok = True
+
 # Jython needs some vars to be set locally.
 # set JAVA_HOME=c:\bin\jdk1.8.0_172
 # set PATH=%PATH%;C:\bin\jython2.7.0\bin