[lldb] [Process/NetBSD] Copy watchpoints to newly-created threads

[mgorny]

Summary

NetBSD ptrace interface does not populate watchpoints to newly-created
threads. Solve this via copying the watchpoints from the current thread
when new thread is reported via TRAP_LWP.

Add a test that verifies that when the user does not have permissions
to set watchpoints on NetBSD, the 'watchpoint set' errors out gracefully
and thread monitoring does not crash on being unable to copy watchpoints
to new threads.

[krytarowski]

How does it deal with security.models.extensions.user_set_dbregs? If there is a handled error than it's fine.

[mgorny]

In D70023#1739356, @krytarowski wrote:

How does it deal with security.models.extensions.user_set_dbregs? If there is a handled error than it's fine.

I need to test it. Might turn out it's so broken it's better to address it globally in a separate patch.

[mgorny]

Included fixed tests + new test from D70050.

[mgorny]

lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp

280

Note to self: need to add error handling here.

lldb/test/Shell/Watchpoint/netbsd-nouserdbregs.test

6

Note to self: try %clang_host as suggested in D70050.

[mgorny]

Made dbregs copied only if any watchpoints are enabled, and added error handling.

[labath]

lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h

74–76

Are you sure this is the proper formatting here? It seems fairly odd..

lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp

291–292

I'd probably make a covariant override of GetRegisterContext which returns a NativeRegisterContextNetBSD&.

lldb/test/Shell/Watchpoint/netbsd-nouserdbregs.test

6

So, did you try it?

[mgorny]

Switched to %clang_host.

Fun fact: things work better if you remove the file before 'readding' it via patch (i.e. ending up appending new contents to old).

[mgorny]

Switched to RegisterContextNetBSD & return type as suggested, and fixed formatting. Had to do some include shifting too.

[mgorny]

lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp

291–292

Thanks. I didn't know this was actually legal in C++.

lldb/test/Shell/Watchpoint/netbsd-nouserdbregs.test

6

Sorry, I've forgotten about it. Done now. I presume I don't need native in REQUIRES now.

Will address remaining comments soonish.

[labath]

lldb/test/Shell/Watchpoint/netbsd-nouserdbregs.test

6

system-netbsd should be enough, but we never really bothered to add "native" even before we had %clang_host. I'm pretty sure a _lot_ of things would blow up if you configured your build so that the default clang target is not the host system.

lldb/packages/Python/lldbsuite/test/commands/watchpoints/multiple_threads/TestWatchpointMultipleThreads.py

 
 
 class WatchpointForMultipleThreadsTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
     NO_DEBUG_INFO_TESTCASE = True
     main_spec = lldb.SBFileSpec("main.cpp", False)
 
-    @expectedFailureNetBSD
     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")
 
-    @expectedFailureNetBSD
     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 do_watchpoint_test(self, line):
         self.build()
         lldbutil.run_to_source_breakpoint(self, line, self.main_spec)
 

lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentBreakpointsDelayedBreakpointOneWatchpoint.py

 class ConcurrentBreakpointsDelayedBreakpointOneWatchpoint(
         ConcurrentEventsBase):
 
     mydir = ConcurrentEventsBase.compute_mydir(__file__)
 
     @skipIfFreeBSD  # timing out on buildbot
     # Atomic sequences are not supported yet for MIPS in LLDB.
     @skipIf(triple='^mips')
-    @expectedFailureNetBSD
     @add_test_categories(["watchpoint"])
     def test(self):
         """Test a breakpoint, a delayed breakpoint, and one watchpoint thread. """
         self.build(dictionary=self.getBuildFlags())
         self.do_thread_actions(num_breakpoint_threads=1,
                                num_delay_breakpoint_threads=1,
                                num_watchpoint_threads=1)

lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentDelaySignalWatch.py

 @skipIfWindows
 class ConcurrentDelaySignalWatch(ConcurrentEventsBase):
 
     mydir = ConcurrentEventsBase.compute_mydir(__file__)
 
     @skipIfFreeBSD  # timing out on buildbot
     # Atomic sequences are not supported yet for MIPS in LLDB.
     @skipIf(triple='^mips')
-    @expectedFailureNetBSD
     @add_test_categories(["watchpoint"])
     def test(self):
         """Test a watchpoint and a (1 second delay) signal in multiple threads."""
         self.build(dictionary=self.getBuildFlags())
         self.do_thread_actions(
             num_delay_signal_threads=1,
             num_watchpoint_threads=1)

lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentDelayWatchBreak.py

 @skipIfWindows
 class ConcurrentDelayWatchBreak(ConcurrentEventsBase):
 
     mydir = ConcurrentEventsBase.compute_mydir(__file__)
 
     @skipIfFreeBSD  # timing out on buildbot
     # Atomic sequences are not supported yet for MIPS in LLDB.
     @skipIf(triple='^mips')
-    @expectedFailureNetBSD
     @add_test_categories(["watchpoint"])
     def test(self):
         """Test (1-second delay) watchpoint and a breakpoint in multiple threads."""
         self.build(dictionary=self.getBuildFlags())
         self.do_thread_actions(
             num_breakpoint_threads=1,
             num_delay_watchpoint_threads=1)

lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentSignalWatch.py

 @skipIfWindows
 class ConcurrentSignalWatch(ConcurrentEventsBase):
 
     mydir = ConcurrentEventsBase.compute_mydir(__file__)
 
     @skipIfFreeBSD  # timing out on buildbot
     # Atomic sequences are not supported yet for MIPS in LLDB.
     @skipIf(triple='^mips')
-    @expectedFailureNetBSD
     @add_test_categories(["watchpoint"])
     def test(self):
         """Test a watchpoint and a signal in multiple threads."""
         self.build(dictionary=self.getBuildFlags())
         self.do_thread_actions(num_signal_threads=1, num_watchpoint_threads=1)

lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentTwoBreakpointsOneWatchpoint.py

 @skipIfWindows
 class ConcurrentTwoBreakpointsOneWatchpoint(ConcurrentEventsBase):
 
     mydir = ConcurrentEventsBase.compute_mydir(__file__)
 
     @skipIfFreeBSD  # timing out on buildbot
     # Atomic sequences are not supported yet for MIPS in LLDB.
     @skipIf(triple='^mips')
-    @expectedFailureNetBSD
     @add_test_categories(["watchpoint"])
     def test(self):
         """Test two threads that trigger a breakpoint and one watchpoint thread. """
         self.build(dictionary=self.getBuildFlags())
         self.do_thread_actions(
             num_breakpoint_threads=2,
             num_watchpoint_threads=1)

lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentWatchBreak.py

 @skipIfWindows
 class ConcurrentWatchBreak(ConcurrentEventsBase):
 
     mydir = ConcurrentEventsBase.compute_mydir(__file__)
 
     @skipIfFreeBSD  # timing out on buildbot
     # Atomic sequences are not supported yet for MIPS in LLDB.
     @skipIf(triple='^mips')
-    @expectedFailureNetBSD
     @add_test_categories(["watchpoint"])
     def test(self):
         """Test watchpoint and a breakpoint in multiple threads."""
         self.build(dictionary=self.getBuildFlags())
         self.do_thread_actions(
             num_breakpoint_threads=1,
             num_watchpoint_threads=1)

lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentWatchBreakDelay.py

 @skipIfWindows
 class ConcurrentWatchBreakDelay(ConcurrentEventsBase):
 
     mydir = ConcurrentEventsBase.compute_mydir(__file__)
 
     @skipIfFreeBSD  # timing out on buildbot
     # Atomic sequences are not supported yet for MIPS in LLDB.
     @skipIf(triple='^mips')
-    @expectedFailureNetBSD
     @add_test_categories(["watchpoint"])
     def test(self):
         """Test watchpoint and a (1 second delay) breakpoint in multiple threads."""
         self.build(dictionary=self.getBuildFlags())
         self.do_thread_actions(
             num_delay_breakpoint_threads=1,
             num_watchpoint_threads=1)

lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentWatchpointDelayWatchpointOneBreakpoint.py

 @skipIfWindows
 class ConcurrentWatchpointDelayWatchpointOneBreakpoint(ConcurrentEventsBase):
 
     mydir = ConcurrentEventsBase.compute_mydir(__file__)
 
     @skipIfFreeBSD  # timing out on buildbot
     # Atomic sequences are not supported yet for MIPS in LLDB.
     @skipIf(triple='^mips')
-    @expectedFailureNetBSD
     @add_test_categories(["watchpoint"])
     def test(self):
         """Test two threads that trigger a watchpoint (one with a 1 second delay) and one breakpoint thread. """
         self.build(dictionary=self.getBuildFlags())
         self.do_thread_actions(num_watchpoint_threads=1,
                                num_delay_watchpoint_threads=1,
                                num_breakpoint_threads=1)

lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentWatchpointWithDelayWatchpointThreads.py

 @skipIfWindows
 class ConcurrentWatchpointWithDelayWatchpointThreads(ConcurrentEventsBase):
 
     mydir = ConcurrentEventsBase.compute_mydir(__file__)
 
     @skipIfFreeBSD  # timing out on buildbot
     # Atomic sequences are not supported yet for MIPS in LLDB.
     @skipIf(triple='^mips')
-    @expectedFailureNetBSD
     @add_test_categories(["watchpoint"])
     def test(self):
         """Test two threads that trigger a watchpoint where one thread has a 1 second delay. """
         self.build(dictionary=self.getBuildFlags())
         self.do_thread_actions(num_watchpoint_threads=1,
                                num_delay_watchpoint_threads=1)

lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp

case TRAP_LWP: {
     ptrace_state_t pst;
     Status error = PtraceWrapper(PT_GET_PROCESS_STATE, pid, &pst, sizeof(pst));
     if (error.Fail()) {
       SetState(StateType::eStateInvalid);
       return;
     }
 
     switch (pst.pe_report_event) {
-      case PTRACE_LWP_CREATE:
+      case PTRACE_LWP_CREATE: {
         LLDB_LOG(log,
                  "monitoring new thread, pid = {0}, LWP = {1}", pid,
                  pst.pe_lwp);
-        AddThread(pst.pe_lwp);
-        break;
+        NativeThreadNetBSD& t = AddThread(pst.pe_lwp);
+        error = t.CopyWatchpointsFrom(
+            static_cast<NativeThreadNetBSD &>(*GetCurrentThread()));
+        if (error.Fail()) {
+          LLDB_LOG(log,
+                   "failed to copy watchpoints to new thread {0}: {1}",
+                   pst.pe_lwp, error);
+          SetState(StateType::eStateInvalid);
+          return;
+        }
+      } break;
       case PTRACE_LWP_EXIT:
         LLDB_LOG(log,
                  "removing exited thread, pid = {0}, LWP = {1}", pid,
                  pst.pe_lwp);
         RemoveThread(pst.pe_lwp);
         break;
     }
 

lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.h

 //===-- NativeRegisterContextNetBSD.h ---------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef lldb_NativeRegisterContextNetBSD_h
 #define lldb_NativeRegisterContextNetBSD_h
 
 #include "lldb/Host/common/NativeThreadProtocol.h"
 
-#include "Plugins/Process/NetBSD/NativeProcessNetBSD.h"
 #include "Plugins/Process/Utility/NativeRegisterContextRegisterInfo.h"
 
 namespace lldb_private {
 namespace process_netbsd {
 
+class NativeProcessNetBSD;
+
 class NativeRegisterContextNetBSD : public NativeRegisterContextRegisterInfo {
 public:
   NativeRegisterContextNetBSD(NativeThreadProtocol &native_thread,
                               RegisterInfoInterface *reg_info_interface_p);
 
   // This function is implemented in the NativeRegisterContextNetBSD_*
   // subclasses to create a new instance of the host specific
   // NativeRegisterContextNetBSD. The implementations can't collide as only one
   // NativeRegisterContextNetBSD_* variant should be compiled into the final
   // executable.
   static NativeRegisterContextNetBSD *
   CreateHostNativeRegisterContextNetBSD(const ArchSpec &target_arch,
                                         NativeThreadProtocol &native_thread);
+  virtual Status
+  CopyHardwareWatchpointsFrom(NativeRegisterContextNetBSD &source) = 0;
 
 protected:
   Status DoRegisterSet(int req, void *buf);
   virtual NativeProcessNetBSD &GetProcess();
   virtual ::pid_t GetProcessPid();
 };
 
 } // namespace process_netbsd
 } // namespace lldb_private
 
 #endif // #ifndef lldb_NativeRegisterContextNetBSD_h

lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.cpp

 //===-- NativeRegisterContextNetBSD.cpp -------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "NativeRegisterContextNetBSD.h"
 
+#include "Plugins/Process/NetBSD/NativeProcessNetBSD.h"
+
 #include "lldb/Host/common/NativeProcessProtocol.h"
 
 using namespace lldb_private;
 using namespace lldb_private::process_netbsd;
 
 // clang-format off
 #include <sys/types.h>
 #include <sys/ptrace.h>

lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h

public:
 
   uint32_t SetHardwareWatchpoint(lldb::addr_t addr, size_t size,
                                  uint32_t watch_flags) override;
 
   lldb::addr_t GetWatchpointAddress(uint32_t wp_index) override;
 
   uint32_t NumSupportedHardwareWatchpoints() override;
 
+  Status
+  CopyHardwareWatchpointsFrom(NativeRegisterContextNetBSD &source) override;
+
 private:
   // Private member types.
   enum { GPRegSet, FPRegSet, XStateRegSet, DBRegSet };
 
   // Private member variables.
   struct reg m_gpr_x86_64;
   struct fpreg m_fpr_x86_64;
   struct dbreg m_dbr_x86_64;

lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp

NativeRegisterContextNetBSD_x86_64::GetWatchpointAddress(uint32_t wp_index) {
   return reg_value.GetAsUInt64();
 }
 
 uint32_t NativeRegisterContextNetBSD_x86_64::NumSupportedHardwareWatchpoints() {
   // Available debug address registers: dr0, dr1, dr2, dr3
   return 4;
 }
 
+Status NativeRegisterContextNetBSD_x86_64::CopyHardwareWatchpointsFrom(
+    NativeRegisterContextNetBSD &source) {
+  auto &r_source = static_cast<NativeRegisterContextNetBSD_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_x86_64.dr[7] & 0xFF) == 0)
+      return res;
+
+    m_dbr_x86_64 = r_source.m_dbr_x86_64;
+    res = WriteRegisterSet(DBRegSet);
+  }
+  return res;
+}
+
 #endif // defined(__x86_64__)

lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.h

 //===-- NativeThreadNetBSD.h ---------------------------------- -*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef liblldb_NativeThreadNetBSD_H_
 #define liblldb_NativeThreadNetBSD_H_
 
 #include "lldb/Host/common/NativeThreadProtocol.h"
 
+#include "Plugins/Process/NetBSD/NativeRegisterContextNetBSD.h"
+
 #include <csignal>
 #include <map>
 #include <string>
 
 namespace lldb_private {
 namespace process_netbsd {
 
 class NativeProcessNetBSD;
 
 class NativeThreadNetBSD : public NativeThreadProtocol {
   friend class NativeProcessNetBSD;
 
 public:
   NativeThreadNetBSD(NativeProcessNetBSD &process, lldb::tid_t tid);
 
   // NativeThreadProtocol Interface
   std::string GetName() override;
 
   lldb::StateType GetState() override;
 
   bool GetStopReason(ThreadStopInfo &stop_info,
                      std::string &description) override;
 
-  NativeRegisterContext& GetRegisterContext() override;
+  NativeRegisterContextNetBSD &GetRegisterContext() override;
 
   Status SetWatchpoint(lldb::addr_t addr, size_t size, uint32_t watch_flags,
                        bool hardware) override;
 
   Status RemoveWatchpoint(lldb::addr_t addr) override;
 
   Status SetHardwareBreakpoint(lldb::addr_t addr, size_t size) override;
 
private:
   void SetStoppedByTrace();
   void SetStoppedByExec();
   void SetStoppedByWatchpoint(uint32_t wp_index);
   void SetStoppedWithNoReason();
   void SetStopped();
   void SetRunning();
   void SetStepping();
 
+  Status CopyWatchpointsFrom(NativeThreadNetBSD& source);
+
   // Member Variables
   lldb::StateType m_state;
   ThreadStopInfo m_stop_info;
-  std::unique_ptr<NativeRegisterContext> m_reg_context_up;
+  std::unique_ptr<NativeRegisterContextNetBSD> m_reg_context_up;
   std::string m_stop_description;
   using WatchpointIndexMap = std::map<lldb::addr_t, uint32_t>;
   WatchpointIndexMap m_watchpoint_index_map;
   WatchpointIndexMap m_hw_break_index_map;
 };
 
 typedef std::shared_ptr<NativeThreadNetBSD> NativeThreadNetBSDSP;
 } // namespace process_netbsd
 } // namespace lldb_private
 
 #endif // #ifndef liblldb_NativeThreadNetBSD_H_

lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp

bool NativeThreadNetBSD::GetStopReason(ThreadStopInfo &stop_info,
   case eStateDetached:
     LLDB_LOG(log, "tid = {0} in state {1} cannot answer stop reason", GetID(),
              StateAsCString(m_state));
     return false;
   }
   llvm_unreachable("unhandled StateType!");
 }
 
-NativeRegisterContext& NativeThreadNetBSD::GetRegisterContext() {
+NativeRegisterContextNetBSD &NativeThreadNetBSD::GetRegisterContext() {
   assert(m_reg_context_up);
-return  *m_reg_context_up;
+  return *m_reg_context_up;
 }
 
 Status NativeThreadNetBSD::SetWatchpoint(lldb::addr_t addr, size_t size,
                                          uint32_t watch_flags, bool hardware) {
   if (!hardware)
     return Status("not implemented");
   if (m_state == eStateLaunching)
     return Status();
Status NativeThreadNetBSD::RemoveHardwareBreakpoint(lldb::addr_t addr) {
   uint32_t bp_index = bp->second;
   if (GetRegisterContext().ClearHardwareBreakpoint(bp_index)) {
     m_hw_break_index_map.erase(bp);
     return Status();
   }
 
   return Status("Clearing hardware breakpoint failed.");
 }
+
+Status NativeThreadNetBSD::CopyWatchpointsFrom(NativeThreadNetBSD &source) {
+  Status s = GetRegisterContext().CopyHardwareWatchpointsFrom(
+      source.GetRegisterContext());
+  if (!s.Fail()) {
+    m_watchpoint_index_map = source.m_watchpoint_index_map;
+    m_hw_break_index_map = source.m_hw_break_index_map;
+  }
+  return s;
+}

lldb/test/Shell/Watchpoint/Inputs/thread-dbreg.c

+#include <pthread.h>
+
+int g_watchme = 0;
+
+void *thread_func(void *arg) {
+  /* watchpoint trigger from subthread */
+  g_watchme = 2;
+  return 0;
+}
+
+int main() {
+  pthread_t thread;
+  if (pthread_create(&thread, 0, thread_func, 0))
+    return 1;
+
+  /* watchpoint trigger from main thread */
+  g_watchme = 1;
+
+  if (pthread_join(thread, 0))
+    return 2;
+
+  return 0;
+}

lldb/test/Shell/Watchpoint/netbsd-nouserdbregs.test

+# Check that 'watchpoint set' errors out gracefully when we can't set dbregs
+# and that new threads are monitored correctly even though we can't copy dbregs.
+
+# REQUIRES: system-netbsd && (target-x86 || target-x86_64) && !dbregs-set
+# RUN: %clang_host %p/Inputs/thread-dbreg.c -pthread -g -o %t.out
+# RUN: %lldb -b -o 'settings set interpreter.stop-command-source-on-error false' -s %s %t.out 2>&1 | FileCheck %s
+
+settings show interpreter.stop-command-source-on-error
+# CHECK: interpreter.stop-command-source-on-error (boolean) = false
+
+b main
+# CHECK: Breakpoint {{[0-9]+}}: where = {{.*}}`main
+b thread_func
+# CHECK: Breakpoint {{[0-9]+}}: where = {{.*}}`thread_func
+run
+# CHECK: stop reason = breakpoint
+watchpoint set variable g_watchme
+# CHECK: error: Watchpoint creation failed
+cont
+# CHECK: stop reason = breakpoint
+cont
+# CHECK: Process {{[0-9]+}} exited with status = 0

lldb/test/Shell/lit.cfg.py

 # -*- Python -*-
 
 import os
 import platform
 import re
 import shutil
 import site
+import subprocess
 import sys
 
 import lit.formats
 from lit.llvm import llvm_config
 from lit.llvm.subst import FindTool
 from lit.llvm.subst import ToolSubst
 from distutils.spawn import find_executable
 

 if not config.lldb_disable_python:
     config.available_features.add('python')
 
 if config.lldb_enable_lzma:
     config.available_features.add('lzma')
 
 if find_executable('xz') != None:
     config.available_features.add('xz')
+
+# NetBSD permits setting dbregs either if one is root
+# or if user_set_dbregs is enabled
+can_set_dbregs = True
+if platform.system() == 'NetBSD' and os.geteuid() != 0:
+    try:
+        output = subprocess.check_output(["/sbin/sysctl", "-n",
+          "security.models.extensions.user_set_dbregs"]).decode().strip()
+        if output != "1":
+            can_set_dbregs = False
+    except subprocess.CalledProcessError:
+        can_set_dbregs = False
+if can_set_dbregs:
+    config.available_features.add('dbregs-set')