[lldb] [Process/NetBSD] Enable reporting of new and exited threads [WIP]

[mgorny]

Summary

// NB:
Now, this patch is most likely correct. However, I've tested it only with the other patch applied, and many tests started hanging forever. If you have any comments, I'd be happy to process them but I won't commit it until all the issues have been resolved.

[labath]

Looks fine to me. Minor comments inline.

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

271

I don't think you need this. Given that you never reported a stop event, as far as the rest of the world is concerned the process is still "running" (i.e., this "stop to notice a new thread" is just an implementation detail).

737

It looks like the rest of the code assumes (as I think it should) that the thread list does not contain null pointers. See e.g. the loop on line 276.

[mgorny]

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

738

Note to self, this should be it =...

[mgorny]

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

737

For the record, I've copied it from Linux ;-P.

[jfb]

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

737

Are you serious? Because you absolutely cannot copy code from Linux into LLVM.

[mgorny]

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

737

I meant LLDB Linux Process plugin.

[jingham]

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

737

I believe Michal means he copied it from the Linux NativeProcess implementation in lldb to the NetBSD Process plugin in lldb.

[jfb]

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

737

OK, thank you for clarifying.

[mgorny]

@jfb, could you please remove your red flag now? ;-)

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

738

Nevermind myself. It breaks immediately, so it doesn't matter.

[krytarowski]

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

104

I would go for more generic PT_GET_EVENT_MASK and then |=. In theory we could add more bits and make them enabled by default.

[mgorny]

The problem is to complex too be implemented in smaller blocks. I'm going to close this in favor of a single unified patch in D64647.

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

 @skipIfWindows
 class ConcurrentBreakpointOneDelayBreakpointThreads(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
     def test(self):
         """Test threads that trigger a breakpoint where one thread has a 1 second delay. """
         self.build(dictionary=self.getBuildFlags())
         self.do_thread_actions(num_breakpoint_threads=1,
                                num_delay_breakpoint_threads=1)

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

 @skipIfWindows
 class ConcurrentCrashWithBreak(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
     def test(self):
         """ Test a thread that crashes while another thread hits a breakpoint."""
         self.build(dictionary=self.getBuildFlags())
         self.do_thread_actions(num_crash_threads=1, num_breakpoint_threads=1)

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

 @skipIfWindows
 class ConcurrentCrashWithSignal(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
     def test(self):
         """ Test a thread that crashes while another thread generates a signal."""
         self.build(dictionary=self.getBuildFlags())
         self.do_thread_actions(num_crash_threads=1, num_signal_threads=1)

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

 @skipIfWindows
 class ConcurrentDelayedCrashWithBreakpointSignal(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
     def test(self):
         """ Test a thread with a delayed crash while other threads generate a signal and hit a breakpoint. """
         self.build(dictionary=self.getBuildFlags())
         self.do_thread_actions(num_delay_crash_threads=1,
                                num_breakpoint_threads=1,
                                num_signal_threads=1)

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

 @skipIfWindows
 class ConcurrentTwoBreakpointThreads(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
     def test(self):
         """Test two threads that trigger a breakpoint. """
         self.build(dictionary=self.getBuildFlags())
         self.do_thread_actions(num_breakpoint_threads=2)

lldb/packages/Python/lldbsuite/test/functionalities/thread/exit_during_step/TestExitDuringStep.py

 
 
 class ExitDuringStepTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     @skipIfFreeBSD  # llvm.org/pr21411: test is hanging
     @skipIfWindows # This is flakey on Windows: llvm.org/pr38373
-    @expectedFailureNetBSD
     def test(self):
         """Test thread exit during step handling."""
         self.build(dictionary=self.getBuildFlags())
         self.exit_during_step_base(
             "thread step-inst -m all-threads",
             'stop reason = instruction step',
             True)
 

lldb/packages/Python/lldbsuite/test/functionalities/thread/step_out/TestThreadStepOut.py

class ThreadStepOutTestCase(TestBase):
     @expectedFailureAll(
         oslist=["linux"],
         bugnumber="llvm.org/pr23477 Test occasionally times out on the Linux build bot")
     @expectedFailureAll(
         oslist=["freebsd"],
         bugnumber="llvm.org/pr18066 inferior does not exit")
     @skipIfWindows # This test will hang on windows llvm.org/pr21753
     @expectedFailureAll(oslist=["windows"])
-    @expectedFailureNetBSD
     def test_step_single_thread(self):
         """Test thread step out on one thread via command interpreter. """
         self.build(dictionary=self.getBuildFlags())
         self.step_out_test(self.step_out_single_thread_with_cmd)
 
     # Test occasionally times out on the Linux build bot
     @skipIfLinux
     @expectedFailureAll(
         oslist=["linux"],
         bugnumber="llvm.org/pr23477 Test occasionally times out on the Linux build bot")
     @expectedFailureAll(
         oslist=["freebsd"],
         bugnumber="llvm.org/pr19347 2nd thread stops at breakpoint")
     @skipIfWindows # This test will hang on windows llvm.org/pr21753
     @expectedFailureAll(oslist=["windows"])
     @expectedFailureAll(oslist=["watchos"], archs=['armv7k'], bugnumber="rdar://problem/34674488") # stop reason is trace when it should be step-out
-    @expectedFailureNetBSD
     def test_step_all_threads(self):
         """Test thread step out on all threads via command interpreter. """
         self.build(dictionary=self.getBuildFlags())
         self.step_out_test(self.step_out_all_threads_with_cmd)
 
     # Test occasionally times out on the Linux build bot
     @skipIfLinux
     @expectedFailureAll(

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

private:
 
   // Private Instance Methods
   NativeProcessNetBSD(::pid_t pid, int terminal_fd, NativeDelegate &delegate,
                       const ArchSpec &arch, MainLoop &mainloop);
 
   bool HasThreadNoLock(lldb::tid_t thread_id);
 
   NativeThreadNetBSD &AddThread(lldb::tid_t thread_id);
+  void RemoveThread(lldb::tid_t thread_id);
 
   void MonitorCallback(lldb::pid_t pid, int signal);
   void MonitorExited(lldb::pid_t pid, WaitStatus status);
   void MonitorSIGSTOP(lldb::pid_t pid);
   void MonitorSIGTRAP(lldb::pid_t pid);
   void MonitorSignal(lldb::pid_t pid, int signal);
 
   Status PopulateMemoryRegionCache();

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

 //===-- NativeProcessNetBSD.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 "NativeProcessNetBSD.h"
 
 
-
 #include "Plugins/Process/POSIX/ProcessPOSIXLog.h"
 #include "lldb/Host/HostProcess.h"
 #include "lldb/Host/common/NativeRegisterContext.h"
 #include "lldb/Host/posix/ProcessLauncherPosixFork.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Utility/State.h"
 #include "llvm/Support/Errno.h"
 
NativeProcessNetBSD::Factory::Launch(ProcessLaunchInfo &launch_info,
   // Set the architecture to the exe architecture.
   LLDB_LOG(log, "pid = {0:x}, detected architecture {1}", pid,
            Info.GetArchitecture().GetArchitectureName());
 
   std::unique_ptr<NativeProcessNetBSD> process_up(new NativeProcessNetBSD(
       pid, launch_info.GetPTY().ReleaseMasterFileDescriptor(), native_delegate,
       Info.GetArchitecture(), mainloop));
 
+  // Enable event reporting
+  ptrace_event_t events;
+  // TODO: PTRACE_FORK | PTRACE_VFORK | PTRACE_POSIX_SPAWN?
+  events.pe_set_event = PTRACE_LWP_CREATE | PTRACE_LWP_EXIT;
+  status = PtraceWrapper(PT_SET_EVENT_MASK, pid, &events, sizeof(events));
+  if (status.Fail())
+    return status.ToError();
+
   status = process_up->ReinitializeThreads();
   if (status.Fail())
     return status.ToError();
 
   for (const auto &thread : process_up->m_threads)
     static_cast<NativeThreadNetBSD &>(*thread).SetStoppedBySignal(SIGSTOP);
   process_up->SetState(StateType::eStateStopped, false);
 
case TRAP_EXEC: {
 
     // Let our delegate know we have just exec'd.
     NotifyDidExec();
 
     for (const auto &thread : m_threads)
       static_cast<NativeThreadNetBSD &>(*thread).SetStoppedByExec();
     SetState(StateType::eStateStopped, true);
   } break;
+  case TRAP_LWP: {
+    ptrace_state_t pst;
+    Status error = PtraceWrapper(PT_GET_PROCESS_STATE, GetID(), &pst,
+                                 sizeof(pst));
+    if (error.Fail()) {
+      SetState(StateType::eStateInvalid);
+      return;
+    }
+
+    switch (pst.pe_report_event) {
+      case PTRACE_LWP_CREATE:
+        AddThread(pst.pe_lwp);
+        break;
+      case PTRACE_LWP_EXIT:
+        RemoveThread(pst.pe_lwp);
+        break;
+    }
+
+    error = PtraceWrapper(PT_CONTINUE, GetID(), reinterpret_cast<void*>(1), 0);
+    if (error.Fail()) {
+      SetState(StateType::eStateInvalid);
+      return;
+    }
+    SetState(StateType::eStateRunning, true);
+  } break;
   case TRAP_DBREG: {
     // Find the thread.
     NativeThreadNetBSD* thread = nullptr;
     for (const auto &t : m_threads) {
       if (t->GetID() == info.psi_lwpid) {
         thread = static_cast<NativeThreadNetBSD *>(t.get());
         break;
       }
NativeThreadNetBSD &NativeProcessNetBSD::AddThread(lldb::tid_t thread_id) {
   // If this is the first thread, save it as the current thread
   if (m_threads.empty())
     SetCurrentThreadID(thread_id);
 
   m_threads.push_back(llvm::make_unique<NativeThreadNetBSD>(*this, thread_id));
   return static_cast<NativeThreadNetBSD &>(*m_threads.back());
 }
 
+void NativeProcessNetBSD::RemoveThread(lldb::tid_t thread_id) {
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_THREAD));
+  LLDB_LOG(log, "pid {0} removing thread with tid {1}", GetID(), thread_id);
+
+  assert(HasThreadNoLock(thread_id) &&
+         "attempted to remove a thread that does not exist");
+
+  for (auto it = m_threads.begin(); it != m_threads.end(); ++it) {
+    if (*it && ((*it)->GetID() == thread_id)) {
+      m_threads.erase(it);
+      break;
+    }
+  }
+}
+
 Status NativeProcessNetBSD::Attach() {
   // Attach to the requested process.
   // An attach will cause the thread to stop with a SIGSTOP.
   Status status = PtraceWrapper(PT_ATTACH, m_pid);
   if (status.Fail())
     return status;
 
   int wstatus;