[Lldb-commits] [lldb] 7e2ef84 - [lldb] [test] Extend watchpoint test to wait for thread to start

[Michał Górny via lldb-commits]

Author: Michał Górny
Date: 2020-11-09T11:35:11+01:00
New Revision: 7e2ef84fe7232368f92ec0835c3eda869c85a445

URL: https://github.com/llvm/llvm-project/commit/7e2ef84fe7232368f92ec0835c3eda869c85a445
DIFF: https://github.com/llvm/llvm-project/commit/7e2ef84fe7232368f92ec0835c3eda869c85a445.diff

LOG: [lldb] [test] Extend watchpoint test to wait for thread to start

TestWatchpointMultipleThreads currently accounts for two scenarios:
setting the watchpoint before a new thread starts (presumably, verifying
that it will be propagated to the new thread) and setting it after
the thread starts (presumably, verifying that a new watchpoint is set
on all threads).  However, the latter test currently assumes that
the thread will be reported to the debugger before the breakpoint is
hit.  This is not the case on FreeBSD and NetBSD.

On NetBSD, new threads do not inherit debug registers from their parent
threads.  Instead, LLDB copies them manually after the new thread is
reported.  Since the thread is actually reported after the second
breakpoint location, both tests effectively check the same behavior
(i.e. watchpoint being set before the new thread is reported).

On FreeBSD, new threads inherit debug registers and we seem to hit
an interesting race condition.  While the thread is reported after
the breakpoint is hit, the kernel seems to construct it and copy
the debug register before that happens.  As a result, setting
the watchpoint at the second breakpoint location modifies the debug
registers of the first thread after they have been copied to the second
thread but before the debugger is aware of it.  Therefore,
the watchpoint is not propagated to the second thread and the test
fails.

Extend the test to cover all three possible scenarios: setting
watchpoint before the thread is lanched, after it is launched but before
it is guaranteed to have started and after it has actually started.  Add
a second barrier to account for the last case.  This should ensure that
the second assumption (i.e. that the watchpoint is set on all currently
known threads) is actually tested on FreeBSD and NetBSD.

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

Added: 
    

Modified: 
    lldb/test/API/commands/watchpoints/multiple_threads/TestWatchpointMultipleThreads.py
    lldb/test/API/commands/watchpoints/multiple_threads/main.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/test/API/commands/watchpoints/multiple_threads/TestWatchpointMultipleThreads.py b/lldb/test/API/commands/watchpoints/multiple_threads/TestWatchpointMultipleThreads.py
index d81c8cce09fa..aac83f9c3b75 100644
--- a/lldb/test/API/commands/watchpoints/multiple_threads/TestWatchpointMultipleThreads.py
+++ b/lldb/test/API/commands/watchpoints/multiple_threads/TestWatchpointMultipleThreads.py
@@ -23,8 +23,12 @@ def test_watchpoint_before_thread_start(self):
         self.do_watchpoint_test("Before running the thread")
 
     @expectedFailureAll(oslist=["freebsd"])
+    def test_watchpoint_after_thread_launch(self):
+        """Test that we can hit a watchpoint we set after launching another thread"""
+        self.do_watchpoint_test("After launching the thread")
+
     def test_watchpoint_after_thread_start(self):
-        """Test that we can hit a watchpoint we set after starting another thread"""
+        """Test that we can hit a watchpoint we set after another thread starts"""
         self.do_watchpoint_test("After running the thread")
 
     def do_watchpoint_test(self, line):

diff  --git a/lldb/test/API/commands/watchpoints/multiple_threads/main.cpp b/lldb/test/API/commands/watchpoints/multiple_threads/main.cpp
index 50f6058f50d2..ec41e8e8ebff 100644
--- a/lldb/test/API/commands/watchpoints/multiple_threads/main.cpp
+++ b/lldb/test/API/commands/watchpoints/multiple_threads/main.cpp
@@ -3,10 +3,11 @@
 #include <thread>
 
 volatile uint32_t g_val = 0;
-pseudo_barrier_t g_barrier;
+pseudo_barrier_t g_barrier, g_barrier2;
 
 void thread_func() {
   pseudo_barrier_wait(g_barrier);
+  pseudo_barrier_wait(g_barrier2);
   printf("%s starting...\n", __FUNCTION__);
   for (uint32_t i = 0; i < 10; ++i)
     g_val = i;
@@ -15,11 +16,15 @@ void thread_func() {
 int main(int argc, char const *argv[]) {
   printf("Before running the thread\n");
   pseudo_barrier_init(g_barrier, 2);
+  pseudo_barrier_init(g_barrier2, 2);
   std::thread thread(thread_func);
 
-  printf("After running the thread\n");
+  printf("After launching the thread\n");
   pseudo_barrier_wait(g_barrier);
 
+  printf("After running the thread\n");
+  pseudo_barrier_wait(g_barrier2);
+
   thread.join();
 
   return 0;