[Lldb-commits] [PATCH] D70022: [lldb] [Process/NetBSD] Improve threading support

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added inline comments.


================
Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:241
   case TRAP_BRKPT:
-    for (const auto &thread : m_threads) {
-      static_cast<NativeThreadNetBSD &>(*thread).SetStoppedByBreakpoint();
-      FixupBreakpointPCAsNeeded(static_cast<NativeThreadNetBSD &>(*thread));
+    if (thread) {
+      thread->SetStoppedByBreakpoint();
----------------
This shall be always true unless there is a kernel issue.
But this check is fine, I would just add a comment.


================
Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:248
   case TRAP_TRACE:
-    for (const auto &thread : m_threads)
-      static_cast<NativeThreadNetBSD &>(*thread).SetStoppedByTrace();
+    if (thread)
+      thread->SetStoppedByTrace();
----------------
Same here. Isn't there a fixup for PC?

It's not needed in x86 and can be delayed into future.


================
Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:268
+    ptrace_state_t pst;
+    Status error = PtraceWrapper(PT_GET_PROCESS_STATE, GetID(), &pst,
+                                 sizeof(pst));
----------------
Maybe `GetID()` -> `pid`? Same later.


================
Comment at: lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:790
+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);
----------------
I would add an assert `thread_id > 0`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70022/new/

https://reviews.llvm.org/D70022