[lldb/test] Remove sleeps from some lldb-server tests
authorPavel Labath <pavel@labath.sk>
Mon, 7 Feb 2022 19:04:42 +0000 (20:04 +0100)
committerPavel Labath <pavel@labath.sk>
Wed, 9 Feb 2022 10:05:02 +0000 (11:05 +0100)
Instead of using sleeps, have the inferior notify us (via a trap opcode) that
the requested number of threads have been created.

This allows us to get rid of some fairly dodgy test utility code --
wait_for_thread_count seemed like it was waiting for the threads to
appear, but it never actually let the inferior run, so it only succeeded
if the threads were already started when the function was called. Since
the function was called after a fairly small delay (1s, usually), this
is probably the reason why the tests were failing on some bots.

Differential Revision: https://reviews.llvm.org/D119167

lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py
lldb/test/API/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py
lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
lldb/test/API/tools/lldb-server/main.cpp

index 619d0f1..cea963e 100644 (file)
@@ -772,29 +772,20 @@ class GdbRemoteTestCaseBase(Base):
             thread_ids.extend(new_thread_infos)
         return thread_ids
 
-    def wait_for_thread_count(self, thread_count):
-        start_time = time.time()
-        timeout_time = start_time + self.DEFAULT_TIMEOUT
-
-        actual_thread_count = 0
-        while actual_thread_count < thread_count:
-            self.reset_test_sequence()
-            self.add_threadinfo_collection_packets()
-
-            context = self.expect_gdbremote_sequence()
-            self.assertIsNotNone(context)
-
-            threads = self.parse_threadinfo_packets(context)
-            self.assertIsNotNone(threads)
-
-            actual_thread_count = len(threads)
-
-            if time.time() > timeout_time:
-                raise Exception(
-                    'timed out after {} seconds while waiting for threads: waiting for at least {} threads, found {}'.format(
-                        self.DEFAULT_TIMEOUT, thread_count, actual_thread_count))
+    def launch_with_threads(self, thread_count):
+        procs = self.prep_debug_monitor_and_inferior(
+                inferior_args=["thread:new"]*(thread_count-1) + ["trap"])
 
-        return threads
+        self.test_sequence.add_log_lines([
+                "read packet: $c#00",
+                {"direction": "send",
+                    "regex": r"^\$T([0-9a-fA-F]{2})([^#]*)#..$",
+                    "capture": {1: "stop_signo", 2: "stop_reply_kv"}}], True)
+        self.add_threadinfo_collection_packets()
+        context = self.expect_gdbremote_sequence()
+        threads = self.parse_threadinfo_packets(context)
+        self.assertGreaterEqual(len(threads), thread_count)
+        return context, threads
 
     def add_set_breakpoint_packets(
             self,
@@ -896,28 +887,6 @@ class GdbRemoteTestCaseBase(Base):
 
         return supported_dict
 
-    def run_process_then_stop(self, run_seconds=1):
-        # Tell the stub to continue.
-        self.test_sequence.add_log_lines(
-            ["read packet: $vCont;c#a8"],
-            True)
-        context = self.expect_gdbremote_sequence()
-
-        # Wait for run_seconds.
-        time.sleep(run_seconds)
-
-        # Send an interrupt, capture a T response.
-        self.reset_test_sequence()
-        self.test_sequence.add_log_lines(
-            ["read packet: {}".format(chr(3)),
-             {"direction": "send", "regex": r"^\$T([0-9a-fA-F]+)([^#]+)#[0-9a-fA-F]{2}$", "capture": {1: "stop_result"}}],
-            True)
-        context = self.expect_gdbremote_sequence()
-        self.assertIsNotNone(context)
-        self.assertIsNotNone(context.get("stop_result"))
-
-        return context
-
     def continue_process_and_wait_for_stop(self):
         self.test_sequence.add_log_lines(
             [
index e66424f..fcab1ea 100644 (file)
@@ -16,71 +16,21 @@ class TestGdbRemoteThreadsInStopReply(
         "send packet: $OK#00",
     ]
 
-    def gather_stop_reply_fields(self, post_startup_log_lines, thread_count,
-            field_names):
-        # Set up the inferior args.
-        inferior_args = []
-        for i in range(thread_count - 1):
-            inferior_args.append("thread:new")
-        inferior_args.append("sleep:10")
-        procs = self.prep_debug_monitor_and_inferior(
-            inferior_args=inferior_args)
+    def gather_stop_reply_fields(self, thread_count, field_names):
+        context, threads = self.launch_with_threads(thread_count)
+        key_vals_text = context.get("stop_reply_kv")
+        self.assertIsNotNone(key_vals_text)
 
+        self.reset_test_sequence()
         self.add_register_info_collection_packets()
         self.add_process_info_collection_packets()
 
-        # Assumes test_sequence has anything added needed to setup the initial state.
-        # (Like optionally enabling QThreadsInStopReply.)
-        if post_startup_log_lines:
-            self.test_sequence.add_log_lines(post_startup_log_lines, True)
-        self.test_sequence.add_log_lines([
-            "read packet: $c#63"
-        ], True)
         context = self.expect_gdbremote_sequence()
         self.assertIsNotNone(context)
         hw_info = self.parse_hw_info(context)
 
-        # Give threads time to start up, then break.
-        time.sleep(self.DEFAULT_SLEEP)
-        self.reset_test_sequence()
-        self.test_sequence.add_log_lines(
-            [
-                "read packet: {}".format(
-                    chr(3)),
-                {
-                    "direction": "send",
-                    "regex": r"^\$T([0-9a-fA-F]+)([^#]+)#[0-9a-fA-F]{2}$",
-                    "capture": {
-                        1: "stop_result",
-                        2: "key_vals_text"}},
-            ],
-            True)
-        context = self.expect_gdbremote_sequence()
-        self.assertIsNotNone(context)
-
-        # Wait until all threads have started.
-        threads = self.wait_for_thread_count(thread_count)
-        self.assertIsNotNone(threads)
-        self.assertEqual(len(threads), thread_count)
-
-        # Run, then stop the process, grab the stop reply content.
-        self.reset_test_sequence()
-        self.test_sequence.add_log_lines(["read packet: $c#63",
-                                          "read packet: {}".format(chr(3)),
-                                          {"direction": "send",
-                                           "regex": r"^\$T([0-9a-fA-F]+)([^#]+)#[0-9a-fA-F]{2}$",
-                                           "capture": {1: "stop_result",
-                                                       2: "key_vals_text"}},
-                                          ],
-                                         True)
-        context = self.expect_gdbremote_sequence()
-        self.assertIsNotNone(context)
-
         # Parse the stop reply contents.
-        key_vals_text = context.get("key_vals_text")
-        self.assertIsNotNone(key_vals_text)
         kv_dict = self.parse_key_val_dict(key_vals_text)
-        self.assertIsNotNone(kv_dict)
 
         result = dict();
         result["pc_register"] = hw_info["pc_register"]
@@ -90,19 +40,18 @@ class TestGdbRemoteThreadsInStopReply(
 
         return result
 
-    def gather_stop_reply_threads(self, post_startup_log_lines, thread_count):
+    def gather_stop_reply_threads(self, thread_count):
         # Pull out threads from stop response.
         stop_reply_threads_text = self.gather_stop_reply_fields(
-                post_startup_log_lines, thread_count, ["threads"])["threads"]
+                thread_count, ["threads"])["threads"]
         if stop_reply_threads_text:
             return [int(thread_id, 16)
                     for thread_id in stop_reply_threads_text.split(",")]
         else:
             return []
 
-    def gather_stop_reply_pcs(self, post_startup_log_lines, thread_count):
-        results = self.gather_stop_reply_fields( post_startup_log_lines,
-                thread_count, ["threads", "thread-pcs"])
+    def gather_stop_reply_pcs(self, thread_count):
+        results = self.gather_stop_reply_fields(thread_count, ["threads", "thread-pcs"])
         if not results:
             return []
 
@@ -187,8 +136,9 @@ class TestGdbRemoteThreadsInStopReply(
         self.set_inferior_startup_launch()
         # Gather threads from stop notification when QThreadsInStopReply is
         # enabled.
-        stop_reply_threads = self.gather_stop_reply_threads(
-            self.ENABLE_THREADS_IN_STOP_REPLY_ENTRIES, 5)
+        self.test_sequence.add_log_lines(
+            self.ENABLE_THREADS_IN_STOP_REPLY_ENTRIES, True)
+        stop_reply_threads = self.gather_stop_reply_threads(5)
         self.assertEqual(len(stop_reply_threads), 5)
 
     @expectedFailureAll(oslist=["windows"])
@@ -198,7 +148,7 @@ class TestGdbRemoteThreadsInStopReply(
         self.set_inferior_startup_launch()
         # Gather threads from stop notification when QThreadsInStopReply is not
         # enabled.
-        stop_reply_threads = self.gather_stop_reply_threads(None, 5)
+        stop_reply_threads = self.gather_stop_reply_threads(5)
         self.assertEqual(len(stop_reply_threads), 0)
 
     @expectedFailureAll(oslist=["windows"])
@@ -209,9 +159,9 @@ class TestGdbRemoteThreadsInStopReply(
         # Gather threads from stop notification when QThreadsInStopReply is
         # enabled.
         thread_count = 5
-        stop_reply_threads = self.gather_stop_reply_threads(
-            self.ENABLE_THREADS_IN_STOP_REPLY_ENTRIES, thread_count)
-        self.assertEqual(len(stop_reply_threads), thread_count)
+        self.test_sequence.add_log_lines(
+            self.ENABLE_THREADS_IN_STOP_REPLY_ENTRIES, True)
+        stop_reply_threads = self.gather_stop_reply_threads(thread_count)
 
         # Gather threads from q{f,s}ThreadInfo.
         self.reset_test_sequence()
@@ -234,8 +184,9 @@ class TestGdbRemoteThreadsInStopReply(
         self.build()
         self.set_inferior_startup_launch()
         thread_count = 5
-        results = self.gather_stop_reply_pcs(
-                self.ENABLE_THREADS_IN_STOP_REPLY_ENTRIES, thread_count)
+        self.test_sequence.add_log_lines(
+            self.ENABLE_THREADS_IN_STOP_REPLY_ENTRIES, True)
+        results = self.gather_stop_reply_pcs(thread_count)
         stop_reply_pcs = results["thread_pcs"]
         pc_register = results["pc_register"]
         little_endian = results["little_endian"]
index c2b9444..92e0596 100644 (file)
@@ -10,43 +10,7 @@ class TestGdbRemote_qThreadStopInfo(gdbremote_testcase.GdbRemoteTestCaseBase):
     THREAD_COUNT = 5
 
     def gather_stop_replies_via_qThreadStopInfo(self, thread_count):
-        # Set up the inferior args.
-        inferior_args = []
-        for i in range(thread_count - 1):
-            inferior_args.append("thread:new")
-        inferior_args.append("sleep:10")
-        procs = self.prep_debug_monitor_and_inferior(
-            inferior_args=inferior_args)
-
-        # Assumes test_sequence has anything added needed to setup the initial state.
-        # (Like optionally enabling QThreadsInStopReply.)
-        self.test_sequence.add_log_lines([
-            "read packet: $c#63"
-        ], True)
-        context = self.expect_gdbremote_sequence()
-        self.assertIsNotNone(context)
-
-        # Give threads time to start up, then break.
-        time.sleep(self.DEFAULT_SLEEP)
-        self.reset_test_sequence()
-        self.test_sequence.add_log_lines(
-            [
-                "read packet: {}".format(
-                    chr(3)),
-                {
-                    "direction": "send",
-                    "regex": r"^\$T([0-9a-fA-F]+)([^#]+)#[0-9a-fA-F]{2}$",
-                    "capture": {
-                        1: "stop_result",
-                        2: "key_vals_text"}},
-            ],
-            True)
-        context = self.expect_gdbremote_sequence()
-        self.assertIsNotNone(context)
-
-        # Wait until all threads have started.
-        threads = self.wait_for_thread_count(thread_count)
-        self.assertIsNotNone(threads)
+        context, threads = self.launch_with_threads(thread_count)
 
         # On Windows, there could be more threads spawned. For example, DebugBreakProcess will
         # create a new thread from the debugged process to handle an exception event. So here we
index 1846156..4f19bac 100644 (file)
@@ -355,18 +355,7 @@ class LldbGdbServerTestCase(gdbremote_testcase.GdbRemoteTestCaseBase, DwarfOpcod
             reg_index += 1
 
     def Hg_switches_to_3_threads(self, pass_pid=False):
-        # Startup the inferior with three threads (main + 2 new ones).
-        procs = self.prep_debug_monitor_and_inferior(
-            inferior_args=["thread:new", "thread:new"])
-
-        # Let the inferior process have a few moments to start up the thread
-        # when launched.  (The launch scenario has no time to run, so threads
-        # won't be there yet.)
-        self.run_process_then_stop(run_seconds=1)
-
-        # Wait at most x seconds for 3 threads to be present.
-        threads = self.wait_for_thread_count(3)
-        self.assertEqual(len(threads), 3)
+        _, threads = self.launch_with_threads(3)
 
         pid_str = ""
         if pass_pid:
@@ -397,30 +386,8 @@ class LldbGdbServerTestCase(gdbremote_testcase.GdbRemoteTestCaseBase, DwarfOpcod
         self.set_inferior_startup_launch()
         self.Hg_switches_to_3_threads()
 
-    @expectedFailureAll(oslist=["windows"]) # expecting one more thread
-    @skipIf(compiler="clang", compiler_version=['<', '11.0'])
-    def test_Hg_switches_to_3_threads_attach(self):
-        self.build()
-        self.set_inferior_startup_attach()
-        self.Hg_switches_to_3_threads()
-
-    @expectedFailureAll(oslist=["windows"]) # expect 4 threads
-    @add_test_categories(["llgs"])
-    @skipIf(compiler="clang", compiler_version=['<', '11.0'])
-    def test_Hg_switches_to_3_threads_attach_pass_correct_pid(self):
-        self.build()
-        self.set_inferior_startup_attach()
-        self.Hg_switches_to_3_threads(pass_pid=True)
-
     def Hg_fails_on_pid(self, pass_pid):
-        # Start the inferior.
-        procs = self.prep_debug_monitor_and_inferior(
-            inferior_args=["thread:new"])
-
-        self.run_process_then_stop(run_seconds=1)
-
-        threads = self.wait_for_thread_count(2)
-        self.assertEqual(len(threads), 2)
+        _, threads = self.launch_with_threads(2)
 
         if pass_pid == -1:
             pid_str = "p-1."
@@ -479,13 +446,6 @@ class LldbGdbServerTestCase(gdbremote_testcase.GdbRemoteTestCaseBase, DwarfOpcod
         self.test_sequence.add_log_lines(["read packet: $c#63"], True)
         context = self.expect_gdbremote_sequence()
 
-        # Let the inferior process have a few moments to start up the thread when launched.
-        # context = self.run_process_then_stop(run_seconds=1)
-
-        # Wait at most x seconds for all threads to be present.
-        # threads = self.wait_for_thread_count(NUM_THREADS)
-        # self.assertEquals(len(threads), NUM_THREADS)
-
         signaled_tids = {}
         print_thread_ids = {}
 
@@ -1155,8 +1115,9 @@ class LldbGdbServerTestCase(gdbremote_testcase.GdbRemoteTestCaseBase, DwarfOpcod
         self.set_inferior_startup_launch()
 
         # Startup the inferior with three threads.
-        procs = self.prep_debug_monitor_and_inferior(
-            inferior_args=["thread:new", "thread:new"])
+        _, threads = self.launch_with_threads(3)
+
+        self.reset_test_sequence()
         self.add_thread_suffix_request_packets()
         self.add_register_info_collection_packets()
         self.add_process_info_collection_packets()
@@ -1178,15 +1139,6 @@ class LldbGdbServerTestCase(gdbremote_testcase.GdbRemoteTestCaseBase, DwarfOpcod
         reg_byte_size = int(reg_infos[reg_index]["bitsize"]) // 8
         self.assertTrue(reg_byte_size > 0)
 
-        # Run the process a bit so threads can start up, and collect register
-        # info.
-        context = self.run_process_then_stop(run_seconds=1)
-        self.assertIsNotNone(context)
-
-        # Wait for 3 threads to be present.
-        threads = self.wait_for_thread_count(3)
-        self.assertEqual(len(threads), 3)
-
         expected_reg_values = []
         register_increment = 1
         next_value = None
index f872eab..59e921f 100644 (file)
@@ -137,6 +137,22 @@ static void swap_chars() {
 #endif
 }
 
+static void trap() {
+#if defined(__x86_64__) || defined(__i386__)
+  asm volatile("int3");
+#elif defined(__aarch64__)
+  asm volatile("brk #0xf000");
+#elif defined(__arm__)
+  asm volatile("udf #254");
+#elif defined(__powerpc__)
+  asm volatile("trap");
+#elif __has_builtin(__builtin_debugtrap())
+  __builtin_debugtrap();
+#else
+#warning Don't know how to generate a trap. Some tests may fail.
+#endif
+}
+
 static void hello() {
   std::lock_guard<std::mutex> lock(g_print_mutex);
   printf("hello, world\n");
@@ -330,6 +346,8 @@ int main(int argc, char **argv) {
       // Print the value of specified envvar to stdout.
       const char *value = getenv(arg.c_str());
       printf("%s\n", value ? value : "__unset__");
+    } else if (consume_front(arg, "trap")) {
+      trap();
     } else {
       // Treat the argument as text for stdout.
       printf("%s\n", argv[i]);