Revert "[lldb-dap] Change the launch sequence (#138219)"

This reverts commit ba29e60.

As it broke tests on Windows on Arm: https://lab.llvm.org/buildbot/#/builders/141/builds/8500

********************
Unresolved Tests (2):
  lldb-api :: tools/lldb-dap/completions/TestDAP_completions.py
  lldb-api :: tools/lldb-dap/startDebugging/TestDAP_startDebugging.py
********************
Timed Out Tests (1):
  lldb-api :: tools/lldb-dap/send-event/TestDAP_sendEvent.py
********************
Failed Tests (6):
  lldb-api :: tools/lldb-dap/console/TestDAP_console.py
  lldb-api :: tools/lldb-dap/console/TestDAP_redirection_to_console.py
  lldb-api :: tools/lldb-dap/launch/TestDAP_launch.py
  lldb-api :: tools/lldb-dap/stackTrace/TestDAP_stackTrace.py
  lldb-api :: tools/lldb-dap/stackTraceDisassemblyDisplay/TestDAP_stackTraceDisassemblyDisplay.py
  lldb-api :: tools/lldb-dap/variables/children/TestDAP_variables_children.py

Author: DavidSpickett

lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py

@@ -132,6 +132,7 @@ def __init__(self, recv, send, init_commands, log_file=None):
         self.exit_status = None
         self.initialize_body = None
         self.thread_stop_reasons = {}
+        self.breakpoint_events = []
         self.progress_events = []
         self.reverse_requests = []
         self.module_events = []
@@ -243,6 +244,13 @@ def handle_recv_packet(self, packet):
                 self._process_stopped()
                 tid = body["threadId"]
                 self.thread_stop_reasons[tid] = body
+            elif event == "breakpoint":
+                # Breakpoint events come in when a breakpoint has locations
+                # added or removed. Keep track of them so we can look for them
+                # in tests.
+                self.breakpoint_events.append(packet)
+                # no need to add 'breakpoint' event packets to our packets list
+                return keepGoing
             elif event.startswith("progress"):
                 # Progress events come in as 'progressStart', 'progressUpdate',
                 # and 'progressEnd' events. Keep these around in case test
@@ -404,15 +412,6 @@ def wait_for_stopped(self, timeout=None):
             self.threads = []
         return stopped_events
 
-    def wait_for_breakpoint_events(self, timeout=None):
-        breakpoint_events = []
-        while True:
-            event = self.wait_for_event("breakpoint", timeout=timeout)
-            if not event:
-                break
-            breakpoint_events.append(event)
-        return breakpoint_events
-
     def wait_for_exited(self):
         event_dict = self.wait_for_event("exited")
         if event_dict is None:
@@ -592,7 +591,6 @@ def request_attach(
         attachCommands=None,
         terminateCommands=None,
         coreFile=None,
-        stopOnAttach=True,
         postRunCommands=None,
         sourceMap=None,
         gdbRemotePort=None,
@@ -622,8 +620,6 @@ def request_attach(
             args_dict["attachCommands"] = attachCommands
         if coreFile:
             args_dict["coreFile"] = coreFile
-        if stopOnAttach:
-            args_dict["stopOnEntry"] = stopOnAttach
         if postRunCommands:
             args_dict["postRunCommands"] = postRunCommands
         if sourceMap:
@@ -636,7 +632,7 @@ def request_attach(
         response = self.send_recv(command_dict)
 
         if response["success"]:
-            self.wait_for_event("process")
+            self.wait_for_events(["process", "initialized"])
         return response
 
     def request_breakpointLocations(
@@ -670,6 +666,10 @@ def request_configurationDone(self):
         response = self.send_recv(command_dict)
         if response:
             self.configuration_done_sent = True
+            # Client requests the baseline of currently existing threads after
+            # a successful launch or attach.
+            # Kick off the threads request that follows
+            self.request_threads()
         return response
 
     def _process_stopped(self):
@@ -887,7 +887,7 @@ def request_launch(
         response = self.send_recv(command_dict)
 
         if response["success"]:
-            self.wait_for_event("process")
+            self.wait_for_events(["process", "initialized"])
         return response
 
     def request_next(self, threadId, granularity="statement"):
@@ -1325,26 +1325,6 @@ def attach_options_specified(options):
 
 def run_vscode(dbg, args, options):
     dbg.request_initialize(options.sourceInitFile)
-
-    if options.sourceBreakpoints:
-        source_to_lines = {}
-        for file_line in options.sourceBreakpoints:
-            (path, line) = file_line.split(":")
-            if len(path) == 0 or len(line) == 0:
-                print('error: invalid source with line "%s"' % (file_line))
-
-            else:
-                if path in source_to_lines:
-                    source_to_lines[path].append(int(line))
-                else:
-                    source_to_lines[path] = [int(line)]
-        for source in source_to_lines:
-            dbg.request_setBreakpoints(source, source_to_lines[source])
-    if options.funcBreakpoints:
-        dbg.request_setFunctionBreakpoints(options.funcBreakpoints)
-
-    dbg.request_configurationDone()
-
     if attach_options_specified(options):
         response = dbg.request_attach(
             program=options.program,
@@ -1373,6 +1353,23 @@ def run_vscode(dbg, args, options):
         )
 
     if response["success"]:
+        if options.sourceBreakpoints:
+            source_to_lines = {}
+            for file_line in options.sourceBreakpoints:
+                (path, line) = file_line.split(":")
+                if len(path) == 0 or len(line) == 0:
+                    print('error: invalid source with line "%s"' % (file_line))
+
+                else:
+                    if path in source_to_lines:
+                        source_to_lines[path].append(int(line))
+                    else:
+                        source_to_lines[path] = [int(line)]
+            for source in source_to_lines:
+                dbg.request_setBreakpoints(source, source_to_lines[source])
+        if options.funcBreakpoints:
+            dbg.request_setFunctionBreakpoints(options.funcBreakpoints)
+        dbg.request_configurationDone()
         dbg.wait_for_stopped()
     else:
         if "message" in response:

lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py

@@ -340,7 +340,6 @@ def attach(
         exitCommands=None,
         attachCommands=None,
         coreFile=None,
-        stopOnAttach=True,
         disconnectAutomatically=True,
         terminateCommands=None,
         postRunCommands=None,
@@ -365,8 +364,6 @@ def cleanup():
         self.addTearDownHook(cleanup)
         # Initialize and launch the program
         self.dap_server.request_initialize(sourceInitFile)
-        self.dap_server.wait_for_event("initialized")
-        self.dap_server.request_configurationDone()
         response = self.dap_server.request_attach(
             program=program,
             pid=pid,
@@ -379,7 +376,6 @@ def cleanup():
             attachCommands=attachCommands,
             terminateCommands=terminateCommands,
             coreFile=coreFile,
-            stopOnAttach=stopOnAttach,
             postRunCommands=postRunCommands,
             sourceMap=sourceMap,
             gdbRemotePort=gdbRemotePort,
@@ -438,9 +434,6 @@ def cleanup():
 
         # Initialize and launch the program
         self.dap_server.request_initialize(sourceInitFile)
-        self.dap_server.wait_for_event("initialized")
-        self.dap_server.request_configurationDone()
-
         response = self.dap_server.request_launch(
             program,
             args=args,

lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py

@@ -27,8 +27,6 @@ def spawn_and_wait(program, delay):
 @skip
 class TestDAP_attach(lldbdap_testcase.DAPTestCaseBase):
     def set_and_hit_breakpoint(self, continueToExit=True):
-        self.dap_server.wait_for_stopped()
-
         source = "main.c"
         breakpoint1_line = line_number(source, "// breakpoint 1")
         lines = [breakpoint1_line]

lldb/test/API/tools/lldb-dap/attach/TestDAP_attachByPortNum.py

@@ -18,17 +18,17 @@
 import socket
 
 
+@skip
 class TestDAP_attachByPortNum(lldbdap_testcase.DAPTestCaseBase):
     default_timeout = 20
 
     def set_and_hit_breakpoint(self, continueToExit=True):
-        self.dap_server.wait_for_stopped()
-
         source = "main.c"
-        breakpoint1_line = line_number(source, "// breakpoint 1")
+        main_source_path = os.path.join(os.getcwd(), source)
+        breakpoint1_line = line_number(main_source_path, "// breakpoint 1")
         lines = [breakpoint1_line]
         # Set breakpoint in the thread function so we can step the threads
-        breakpoint_ids = self.set_source_breakpoints(source, lines)
+        breakpoint_ids = self.set_source_breakpoints(main_source_path, lines)
         self.assertEqual(
             len(breakpoint_ids), len(lines), "expect correct number of breakpoints"
         )

lldb/test/API/tools/lldb-dap/breakpoint-events/TestDAP_breakpointEvents.py

@@ -81,27 +81,52 @@ def test_breakpoint_events(self):
                 breakpoint["verified"], "expect foo breakpoint to not be verified"
             )
 
-        # Make sure we're stopped.
-        self.dap_server.wait_for_stopped()
+        # Get the stop at the entry point
+        self.continue_to_next_stop()
 
-        # Flush the breakpoint events.
-        self.dap_server.wait_for_breakpoint_events(timeout=5)
+        # We are now stopped at the entry point to the program. Shared
+        # libraries are not loaded yet (at least on macOS they aren't) and only
+        # the breakpoint in the main executable should be resolved.
+        self.assertEqual(len(self.dap_server.breakpoint_events), 1)
+        event = self.dap_server.breakpoint_events[0]
+        body = event["body"]
+        self.assertEqual(
+            body["reason"], "changed", "breakpoint event should say changed"
+        )
+        breakpoint = body["breakpoint"]
+        self.assertEqual(breakpoint["id"], main_bp_id)
+        self.assertTrue(breakpoint["verified"], "main breakpoint should be resolved")
+
+        # Clear the list of breakpoint events so we don't see this one again.
+        self.dap_server.breakpoint_events.clear()
 
         # Continue to the breakpoint
         self.continue_to_breakpoints(dap_breakpoint_ids)
 
-        verified_breakpoint_ids = []
-        unverified_breakpoint_ids = []
-        for breakpoint_event in self.dap_server.wait_for_breakpoint_events(timeout=5):
-            breakpoint = breakpoint_event["body"]["breakpoint"]
-            id = breakpoint["id"]
-            if breakpoint["verified"]:
-                verified_breakpoint_ids.append(id)
-            else:
-                unverified_breakpoint_ids.append(id)
-
-        self.assertIn(main_bp_id, unverified_breakpoint_ids)
-        self.assertIn(foo_bp_id, unverified_breakpoint_ids)
+        # When the process launches, we first expect to see both the main and
+        # foo breakpoint as unresolved.
+        for event in self.dap_server.breakpoint_events[:2]:
+            body = event["body"]
+            self.assertEqual(
+                body["reason"], "changed", "breakpoint event should say changed"
+            )
+            breakpoint = body["breakpoint"]
+            self.assertIn(str(breakpoint["id"]), dap_breakpoint_ids)
+            self.assertFalse(breakpoint["verified"], "breakpoint should be unresolved")
 
-        self.assertIn(main_bp_id, verified_breakpoint_ids)
-        self.assertIn(foo_bp_id, verified_breakpoint_ids)
+        # Then, once the dynamic loader has given us a load address, they
+        # should show up as resolved again.
+        for event in self.dap_server.breakpoint_events[3:]:
+            body = event["body"]
+            self.assertEqual(
+                body["reason"], "changed", "breakpoint event should say changed"
+            )
+            breakpoint = body["breakpoint"]
+            self.assertIn(str(breakpoint["id"]), dap_breakpoint_ids)
+            self.assertTrue(breakpoint["verified"], "breakpoint should be resolved")
+            self.assertNotIn(
+                "source",
+                breakpoint,
+                "breakpoint event should not return a source object",
+            )
+            self.assertIn("line", breakpoint, "breakpoint event should have line")

lldb/test/API/tools/lldb-dap/completions/TestDAP_completions.py

@@ -44,9 +44,9 @@ def verify_completions(self, actual_list, expected_list, not_expected_list=[]):
             self.assertNotIn(not_expected_item, actual_list)
 
 
-    def setup_debugee(self, stopOnEntry=False):
+    def setup_debugee(self):
         program = self.getBuildArtifact("a.out")
-        self.build_and_launch(program, stopOnEntry=stopOnEntry)
+        self.build_and_launch(program)
 
         source = "main.cpp"
         breakpoint1_line = line_number(source, "// breakpoint 1")
@@ -235,7 +235,7 @@ def test_auto_completions(self):
         """
         Tests completion requests in "repl-mode=auto"
         """
-        self.setup_debugee(stopOnEntry=True)
+        self.setup_debugee()
 
         res = self.dap_server.request_evaluate(
             "`lldb-dap repl-mode auto", context="repl"

lldb/test/API/tools/lldb-dap/console/TestDAP_console.py

@@ -167,7 +167,7 @@ def test_exit_status_message_ok(self):
 
     def test_diagnositcs(self):
         program = self.getBuildArtifact("a.out")
-        self.build_and_launch(program, stopOnEntry=True)
+        self.build_and_launch(program)
 
         core = self.getBuildArtifact("minidump.core")
         self.yaml2obj("minidump.yaml", core)

lldb/test/API/tools/lldb-dap/disconnect/TestDAP_disconnect.py

@@ -31,19 +31,15 @@ def test_launch(self):
         created.
         """
         program = self.getBuildArtifact("a.out")
-        self.build_and_launch(program, stopOnEntry=True, disconnectAutomatically=False)
+        self.build_and_launch(program, disconnectAutomatically=False)
 
         # We set a breakpoint right before the side effect file is created
         self.set_source_breakpoints(
             self.source, [line_number(self.source, "// breakpoint")]
         )
         self.continue_to_next_stop()
 
-        # verify we haven't produced the side effect file yet
-        self.assertFalse(os.path.exists(program + ".side_effect"))
-
         self.dap_server.request_disconnect()
-
         # verify we didn't produce the side effect file
         time.sleep(1)
         self.assertFalse(os.path.exists(program + ".side_effect"))

lldb/test/API/tools/lldb-dap/evaluate/TestDAP_evaluate.py

@@ -10,7 +10,6 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 
-
 # DAP tests are flakey, see https://github.com/llvm/llvm-project/issues/137660.
 @skip
 class TestDAP_evaluate(lldbdap_testcase.DAPTestCaseBase):
@@ -43,9 +42,7 @@ def run_test_evaluate_expressions(
         self.context = context
         program = self.getBuildArtifact("a.out")
         self.build_and_launch(
-            program,
-            enableAutoVariableSummaries=enableAutoVariableSummaries,
-            stopOnEntry=True,
+            program, enableAutoVariableSummaries=enableAutoVariableSummaries
         )
         source = "main.cpp"
         self.set_source_breakpoints(

lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py

@@ -88,8 +88,8 @@ def test_stopOnEntry(self):
         """
         program = self.getBuildArtifact("a.out")
         self.build_and_launch(program, stopOnEntry=True)
-
-        stopped_events = self.dap_server.wait_for_stopped()
+        self.set_function_breakpoints(["main"])
+        stopped_events = self.continue_to_next_stop()
         for stopped_event in stopped_events:
             if "body" in stopped_event:
                 body = stopped_event["body"]