[Lldb-commits] [lldb] 5a4fe16 - [lldb/test] Remove sleeps from some lldb-server tests

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Wed Feb 9 02:05:15 PST 2022


Author: Pavel Labath
Date: 2022-02-09T11:05:02+01:00
New Revision: 5a4fe166d13bf89e6bc387bd96232e8c0f706d27

URL: https://github.com/llvm/llvm-project/commit/5a4fe166d13bf89e6bc387bd96232e8c0f706d27
DIFF: https://github.com/llvm/llvm-project/commit/5a4fe166d13bf89e6bc387bd96232e8c0f706d27.diff

LOG: [lldb/test] Remove sleeps from some lldb-server tests

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

Added: 
    

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
index 619d0f1c21729..cea963e456592 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
@@ -772,29 +772,20 @@ def parse_threadinfo_packets(self, context):
             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 @@ def parse_qSupported_response(self, context):
 
         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(
             [

diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py b/lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py
index e66424f962957..fcab1ea0e49a9 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py
@@ -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 @@ def gather_stop_reply_fields(self, post_startup_log_lines, thread_count,
 
         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 @@ def test_stop_reply_reports_multiple_threads(self):
         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 @@ def test_no_QListThreadsInStopReply_supplies_no_threads(self):
         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 @@ def test_stop_reply_reports_correct_threads(self):
         # 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 @@ def test_stop_reply_contains_thread_pcs(self):
         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"]

diff  --git a/lldb/test/API/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py b/lldb/test/API/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py
index c2b944475fadd..92e0596731da8 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemote_qThreadStopInfo.py
@@ -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

diff  --git a/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py b/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
index 18461568c02ce..4f19bac7441fa 100644
--- a/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
+++ b/lldb/test/API/tools/lldb-server/TestLldbGdbServer.py
@@ -355,18 +355,7 @@ def test_p_returns_correct_data_size_for_each_qRegisterInfo_launch(self):
             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 @@ def test_Hg_switches_to_3_threads_launch(self):
         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 @@ def Hc_then_Csignal_signals_correct_thread(self, segfault_signo):
         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 @@ def test_P_and_p_thread_suffix_work(self):
         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 @@ def test_P_and_p_thread_suffix_work(self):
         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

diff  --git a/lldb/test/API/tools/lldb-server/main.cpp b/lldb/test/API/tools/lldb-server/main.cpp
index f872eabcac98b..59e921f312f43 100644
--- a/lldb/test/API/tools/lldb-server/main.cpp
+++ b/lldb/test/API/tools/lldb-server/main.cpp
@@ -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]);


        


More information about the lldb-commits mailing list