labath created this revision.
labath added reviewers: mgorny, DavidSpickett.
labath requested review of this revision.
Herald added a project: LLDB.

Instead of using sleeps, have the inferior notify us (via a signal) 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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119167

Files:
  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: lldb/test/API/tools/lldb-server/main.cpp
===================================================================
--- lldb/test/API/tools/lldb-server/main.cpp
+++ lldb/test/API/tools/lldb-server/main.cpp
@@ -330,6 +330,8 @@
       // 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, "raise:SIGUSR1")) {
+      raise(SIGUSR1);
     } else {
       // Treat the argument as text for stdout.
       printf("%s\n", argv[i]);
Index: lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
===================================================================
--- lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -355,18 +355,7 @@
             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 @@
         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 @@
         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 @@
         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 @@
         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: lldb/test/API/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py
===================================================================
--- lldb/test/API/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py
@@ -10,43 +10,7 @@
     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: lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py
===================================================================
--- lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py
+++ lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py
@@ -16,69 +16,20 @@
         "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)
 
@@ -90,19 +41,18 @@
 
         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 +137,9 @@
         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 +149,7 @@
         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 +160,9 @@
         # 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 +185,9 @@
         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: lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
@@ -684,29 +684,20 @@
             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) + ["raise:SIGUSR1"])
 
-        return threads
+        self.test_sequence.add_log_lines([
+                "read packet: $c#00",
+                {"direction": "send",
+                    "regex": r"^\$T{0:02x}([^#]*)#..$".format(lldbutil.get_signal_number('SIGUSR1')),
+                    "capture": {1: "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,
@@ -808,28 +799,6 @@
 
         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(
             [
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to