[lldb] [Process/FreeBSDRemote] Explicitly copy dbregs to new threads

Explicitly copy dbregs to new threads to ensure that watchpoints
are propagated properly.  Fixes the test failure due to apparent kernel
race between reporting a new thread and resuming main thread execution
that makes implicit inheritance of dbregs unreliable.  By copying them
explicitly, we ensure that the new thread correctly respects watchpoints
that were set after the thread was created but before it was reported.

The code is copied from the NetBSD plugin and modernized to use
llvm::Error.

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

Author: mgorny

lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp

@@ -199,7 +199,25 @@ void NativeProcessFreeBSD::MonitorSIGTRAP(lldb::pid_t pid) {
   if (info.pl_flags & (PL_FLAG_BORN | PL_FLAG_EXITED)) {
     if (info.pl_flags & PL_FLAG_BORN) {
       LLDB_LOG(log, "monitoring new thread, tid = {0}", info.pl_lwpid);
-      AddThread(info.pl_lwpid);
+      NativeThreadFreeBSD &t = AddThread(info.pl_lwpid);
+
+      // Technically, the FreeBSD kernel copies the debug registers to new
+      // threads.  However, there is a non-negligible delay between acquiring
+      // the DR values and reporting the new thread during which the user may
+      // establish a new watchpoint.  In order to ensure that watchpoints
+      // established during this period are propagated to new threads,
+      // explicitly copy the DR value at the time the new thread is reported.
+      //
+      // See also: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=250954
+
+      llvm::Error error = t.CopyWatchpointsFrom(
+          static_cast<NativeThreadFreeBSD &>(*GetCurrentThread()));
+      if (error) {
+        LLDB_LOG(log, "failed to copy watchpoints to new thread {0}: {1}",
+                 info.pl_lwpid, llvm::toString(std::move(error)));
+        SetState(StateType::eStateInvalid);
+        return;
+      }
     } else /*if (info.pl_flags & PL_FLAG_EXITED)*/ {
       LLDB_LOG(log, "thread exited, tid = {0}", info.pl_lwpid);
       RemoveThread(info.pl_lwpid);

lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.h

@@ -29,6 +29,8 @@ class NativeRegisterContextFreeBSD
   static NativeRegisterContextFreeBSD *
   CreateHostNativeRegisterContextFreeBSD(const ArchSpec &target_arch,
                                          NativeThreadProtocol &native_thread);
+  virtual llvm::Error
+  CopyHardwareWatchpointsFrom(NativeRegisterContextFreeBSD &source) = 0;
 
 protected:
   virtual NativeProcessFreeBSD &GetProcess();

lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp

@@ -1169,4 +1169,19 @@ int NativeRegisterContextFreeBSD_x86_64::GetDR(int num) const {
   }
 }
 
+llvm::Error NativeRegisterContextFreeBSD_x86_64::CopyHardwareWatchpointsFrom(
+    NativeRegisterContextFreeBSD &source) {
+  auto &r_source = static_cast<NativeRegisterContextFreeBSD_x86_64 &>(source);
+  Status res = r_source.ReadRegisterSet(DBRegSet);
+  if (!res.Fail()) {
+    // copy dbregs only if any watchpoints were set
+    if ((r_source.m_dbr.dr[7] & 0xFF) == 0)
+      return llvm::Error::success();
+
+    m_dbr = r_source.m_dbr;
+    res = WriteRegisterSet(DBRegSet);
+  }
+  return res.ToError();
+}
+
 #endif // defined(__x86_64__)

lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h

@@ -50,6 +50,9 @@ class NativeRegisterContextFreeBSD_x86_64
 
   Status WriteAllRegisterValues(const lldb::DataBufferSP &data_sp) override;
 
+  llvm::Error
+  CopyHardwareWatchpointsFrom(NativeRegisterContextFreeBSD &source) override;
+
 private:
   // Private member types.
   enum { GPRegSet, FPRegSet, XSaveRegSet, DBRegSet };

lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp

@@ -165,14 +165,14 @@ std::string NativeThreadFreeBSD::GetName() {
     }
     if (error != 0) {
       len = 0;
-      LLDB_LOG(log, "tid = {0} in state {1} failed to get thread name: {2}", GetID(),
-               m_state, strerror(errno));
+      LLDB_LOG(log, "tid = {0} in state {1} failed to get thread name: {2}",
+               GetID(), m_state, strerror(errno));
     }
     kp.resize(len / sizeof(struct kinfo_proc));
     break;
   }
 
-  for (auto& procinfo : kp) {
+  for (auto &procinfo : kp) {
     if (procinfo.ki_tid == static_cast<lwpid_t>(GetID()))
       return procinfo.ki_tdname;
   }
@@ -273,3 +273,14 @@ Status NativeThreadFreeBSD::RemoveHardwareBreakpoint(lldb::addr_t addr) {
 
   return Status("Clearing hardware breakpoint failed.");
 }
+
+llvm::Error
+NativeThreadFreeBSD::CopyWatchpointsFrom(NativeThreadFreeBSD &source) {
+  llvm::Error s = GetRegisterContext().CopyHardwareWatchpointsFrom(
+      source.GetRegisterContext());
+  if (!s) {
+    m_watchpoint_index_map = source.m_watchpoint_index_map;
+    m_hw_break_index_map = source.m_hw_break_index_map;
+  }
+  return s;
+}

lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h

@@ -64,7 +64,7 @@ class NativeThreadFreeBSD : public NativeThreadProtocol {
   void SetRunning();
   void SetStepping();
 
-  Status CopyWatchpointsFrom(NativeThreadFreeBSD &source);
+  llvm::Error CopyWatchpointsFrom(NativeThreadFreeBSD &source);
 
   // Member Variables
   lldb::StateType m_state;

lldb/test/API/commands/watchpoints/multiple_threads/TestWatchpointMultipleThreads.py

@@ -22,7 +22,6 @@ 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=["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")