Speed up TestWatchpointMultipleThreads

[labath]

Summary

The inferior was sleeping before doing any interesting work. I remove that
to make the test faster.

While looking at the purpose of the test (to check that watchpoints are
propagated to all existing threads - r140757) I noticed that the test has
diverged from the original intention and now it creates the threads *after* the
watchpoint is set (this probably happened during the std::thread refactor), so
I reorganize the test to be closer to the original intention.

The watchpoint propagation functionality is not really debug info depenent, so
I also stop replication of this test. This brings the test's time from ~108s
down to 4s.

[jingham]

Getting watchpoints to propagate to threads that are created after the watchpoint has been set is one of the trickier parts of watchpoint propagation. On macOS, (where we don't get new thread creation notification) we rely on the kernel to propagate the control register settings to the thread contexts of newly created threads - a feature nobody but the debugger uses so we need to make sure it doesn't regress. On systems that don't have kernel cooperation you have to make sure that you catch new thread events and by hand set the control registers, again something we need to make sure we keep doing. If there's no other test that this works, then we should keep testing that in this test. It is the most fragile part of the interaction between watchpoints and threads.

OTOH making this a debug info independent test seems correct.

[labath]

That's a good point. I think that at least TestConcurrentEvents do it that way, but they are testing a different thing. I think it makes sense to test both things here actually. I'll create one test which sets the watchpoint before thread creation and one after.

BTW, do you think there's any value in having three threads running around here? I think the test would be more obvious (and deterministic) if we had just one thread tripping the watchpoint.

[jingham]

Thanks!

While it seems possible somebody could write code that only sets the control registers on one thread and then gets tired and goes out for a beer, that seems like a really unlikely error. I don't think it's worth complicating the test to catch that possibility.

[labath]

This makes the inferior more simple and deterministic by using a single thread.
Now we have one test which sets the watchpoint before the thread is created and
one that sets it after. I've also simplified the test a bit with new lldbutil
goodies.

[xiaobai]

Thanks for taking the time to do this!

Can you please double check some of the formatting in the python code? It looks like it's not indented properly.

[labath]

Replace all tabs in the python file and clang-format the cpp file.

[jingham]

Great, thanks!

[clayborg]

very nice!

lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multiple_threads/TestWatchpointMultipleThreads.py

 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
 
 class WatchpointForMultipleThreadsTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
+    NO_DEBUG_INFO_TESTCASE = True
+    main_spec = lldb.SBFileSpec("main.cpp", False)
 
     @expectedFailureAll(
         oslist=["windows"],
         bugnumber="llvm.org/pr24446: WINDOWS XFAIL TRIAGE - Watchpoints not supported on Windows")
-    def test_watchpoint_multiple_threads(self):
-        """Test that lldb watchpoint works for multiple threads."""
-        self.build()
-        self.setTearDownCleanup()
-        self.hello_multiple_threads()
+    def test_watchpoint_before_thread_start(self):
+        """Test that we can hit a watchpoint we set before starting another thread"""
+        self.do_watchpoint_test("Before running the thread")
 
     @expectedFailureAll(
         oslist=["windows"],
         bugnumber="llvm.org/pr24446: WINDOWS XFAIL TRIAGE - Watchpoints not supported on Windows")
-    def test_watchpoint_multiple_threads_wp_set_and_then_delete(self):
-        """Test that lldb watchpoint works for multiple threads, and after the watchpoint is deleted, the watchpoint event should no longer fires."""
-        self.build()
-        self.setTearDownCleanup()
-        self.hello_multiple_threads_wp_set_and_then_delete()
+    def test_watchpoint_after_thread_start(self):
+        """Test that we can hit a watchpoint we set after starting another thread"""
+        self.do_watchpoint_test("After running the thread")
 
-    def setUp(self):
-        # Call super's setUp().
-        TestBase.setUp(self)
-        # Our simple source filename.
-        self.source = 'main.cpp'
-        # Find the line number to break inside main().
-        self.first_stop = line_number(
-            self.source, '// Set break point at this line')
-
-    def hello_multiple_threads(self):
-        """Test that lldb watchpoint works for multiple threads."""
-        self.runCmd("file " + self.getBuildArtifact("a.out"),
-                    CURRENT_EXECUTABLE_SET)
-
-        # Add a breakpoint to set a watchpoint when stopped on the breakpoint.
-        lldbutil.run_break_set_by_file_and_line(
-            self, None, self.first_stop, num_expected_locations=1)
-
-        # Run the program.
-        self.runCmd("run", RUN_SUCCEEDED)
-
-        # We should be stopped again due to the breakpoint.
-        # The stop reason of the thread should be breakpoint.
-        self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
-                    substrs=['stopped',
-                             'stop reason = breakpoint'])
+    def do_watchpoint_test(self, line):
+        self.build()
+        lldbutil.run_to_source_breakpoint(self, line, self.main_spec)
 
         # Now let's set a write-type watchpoint for variable 'g_val'.
         self.expect(
             "watchpoint set variable -w write g_val",
             WATCHPOINT_CREATED,
             substrs=[
                 'Watchpoint created',
                 'size = 4',
                 'type = w'])
 
         # Use the '-v' option to do verbose listing of the watchpoint.
         # The hit count should be 0 initially.
         self.expect("watchpoint list -v",
                     substrs=['hit_count = 0'])
 
-        while True:
-            self.runCmd("process continue")
 
-            self.runCmd("thread list")
-            if "stop reason = watchpoint" in self.res.GetOutput():
-                # Good, we verified that the watchpoint works!
-                self.runCmd("thread backtrace all")
-                break
-            else:
-                self.fail("The stop reason should be either break or watchpoint")
 
         # Use the '-v' option to do verbose listing of the watchpoint.
         # The hit count should now be 1.
         self.expect("watchpoint list -v",
                     substrs=['hit_count = 1'])
 
-    def hello_multiple_threads_wp_set_and_then_delete(self):
+    @expectedFailureAll(
+        oslist=["windows"],
+        bugnumber="llvm.org/pr24446: WINDOWS XFAIL TRIAGE - Watchpoints not supported on Windows")
+    def test_watchpoint_multiple_threads_wp_set_and_then_delete(self):
         """Test that lldb watchpoint works for multiple threads, and after the watchpoint is deleted, the watchpoint event should no longer fires."""
-        self.runCmd("file " + self.getBuildArtifact("a.out"),
-                    CURRENT_EXECUTABLE_SET)
+        self.build()
+        self.setTearDownCleanup()
 
-        # Add a breakpoint to set a watchpoint when stopped on the breakpoint.
-        lldbutil.run_break_set_by_file_and_line(
-            self, None, self.first_stop, num_expected_locations=1)
-
-        # Run the program.
-        self.runCmd("run", RUN_SUCCEEDED)
-
-        # We should be stopped again due to the breakpoint.
-        # The stop reason of the thread should be breakpoint.
-        self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
-                    substrs=['stopped',
-                             'stop reason = breakpoint'])
+        lldbutil.run_to_source_breakpoint(self, "After running the thread", self.main_spec)
 
         # Now let's set a write-type watchpoint for variable 'g_val'.
         self.expect(
             "watchpoint set variable -w write g_val",
             WATCHPOINT_CREATED,
             substrs=[
                 'Watchpoint created',
                 'size = 4',

lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/multiple_threads/main.cpp

 //===-- main.cpp ------------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
-#include <chrono>
+#include "pseudo_barrier.h"
 #include <cstdio>
-#include <mutex>
-#include <random>
 #include <thread>
 
-std::default_random_engine g_random_engine{std::random_device{}()};
-std::uniform_int_distribution<> g_distribution{0, 3000000};
+volatile uint32_t g_val = 0;
+pseudo_barrier_t g_barrier;
 
-uint32_t g_val = 0;
-
-
-uint32_t
-access_pool (bool flag = false)
-{
-    static std::mutex g_access_mutex;
-    g_access_mutex.lock();
-
-    uint32_t old_val = g_val;
-    if (flag)
-    {
-        printf("changing g_val to %d...\n", old_val + 1);
-        g_val = old_val + 1;
-    }
-
-    g_access_mutex.unlock();
-    return g_val;
-}
-
-void
-thread_func (uint32_t thread_index)
-{
-    printf ("%s (thread index = %u) starting...\n", __FUNCTION__, thread_index);
-
-    uint32_t count = 0;
-    uint32_t val;
-    while (count++ < 15)
-    {
-        // random micro second sleep from zero to 3 seconds
-        int usec = g_distribution(g_random_engine);
-        printf ("%s (thread = %u) doing a usleep (%d)...\n", __FUNCTION__, thread_index, usec);
-        std::this_thread::sleep_for(std::chrono::microseconds{usec});
-
-        if (count < 7)
-            val = access_pool ();
-        else
-            val = access_pool (true);
-
-        printf ("%s (thread = %u) after usleep access_pool returns %d (count=%d)...\n", __FUNCTION__, thread_index, val, count);
-    }
-    printf ("%s (thread index = %u) exiting...\n", __FUNCTION__, thread_index);
+void thread_func() {
+  pseudo_barrier_wait(g_barrier);
+  printf("%s starting...\n", __FUNCTION__);
+  for (uint32_t i = 0; i < 10; ++i)
+    g_val = i;
 }
 
+int main(int argc, char const *argv[]) {
+  printf("Before running the thread\n");
+  pseudo_barrier_init(g_barrier, 2);
+  std::thread thread(thread_func);
 
-int main (int argc, char const *argv[])
-{
-    std::thread threads[3];
-
-    printf ("Before turning all three threads loose...\n"); // Set break point at this line,
-                                                            // in order to set our watchpoint.
-    // Create 3 threads
-    for (auto &thread : threads)
-        thread = std::thread{thread_func, std::distance(threads, &thread)};
+  printf("After running the thread\n");
+  pseudo_barrier_wait(g_barrier);
 
-    // Join all of our threads
-    for (auto &thread : threads)
-        thread.join();
 
-    return 0;
 }