[lldb][Process/FreeBSDKernelCore] Add command for refreshing thread list

[mchoo7]

Since /dev/mem is live memory, its thread list is updated while running LLDB. In the current model, users need to restart LLDB to get new thread list, and this is prone to error when writing to memory because LLDB's thread information and /dev/mem might be out of sync. The new command process plugin refresh-threads reconstructs thread list so users can synchronize thread list without restarting lldb.

Memory cache needs to be cleared prior to reconstruction otherwise lldb will read the same process information from cache memory. To invoke UpdateThreadList() from UpdateThreadListIfNeeded(), clear thread list as well before triggering an update.

This is enabled for all kvm invocation regardless of type of the target (crash dump or live kernel) because kvm hides the target type information. Elf-core based implementation in future will need to update thread list only at the first time as it is guaranteed that elf-core handles static files.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Minsoo Choo (mchoo7)

Changes

Since /dev/mem is live memory, its thread list is updated while running LLDB. In the current model, users need to restart LLDB to get new thread list, and this is prone to error when writing to memory because LLDB's thread information and /dev/mem might be out of sync. The new command process plugin refresh-threads reconstructs thread list so users can synchronize thread list without restarting lldb.

Memory cache needs to be cleared prior to reconstruction otherwise lldb will read the same process information from cache memory. To invoke UpdateThreadList() from UpdateThreadListIfNeeded(), clear thread list as well before triggering an update.

This is enabled for all kvm invocation regardless of type of the target (crash dump or live kernel) because kvm hides the target type information. Elf-core based implementation in future will need to update thread list only at the first time as it is guaranteed that elf-core handles static files.

---

Full diff: https://github.com/llvm/llvm-project/pull/188692.diff

2 Files Affected:

• (modified) lldb/source/Plugins/Process/FreeBSD-Kernel-Core/ProcessFreeBSDKernelCore.cpp (+65-1)
• (modified) lldb/source/Plugins/Process/FreeBSD-Kernel-Core/ProcessFreeBSDKernelCore.h (+6)

diff --git a/lldb/source/Plugins/Process/FreeBSD-Kernel-Core/ProcessFreeBSDKernelCore.cpp b/lldb/source/Plugins/Process/FreeBSD-Kernel-Core/ProcessFreeBSDKernelCore.cpp
index da8e22caaa80f..487a649957caa 100644
--- a/lldb/source/Plugins/Process/FreeBSD-Kernel-Core/ProcessFreeBSDKernelCore.cpp
+++ b/lldb/source/Plugins/Process/FreeBSD-Kernel-Core/ProcessFreeBSDKernelCore.cpp
@@ -9,6 +9,8 @@
 #include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
+#include "lldb/Interpreter/CommandObjectMultiword.h"
+#include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Symbol/Type.h"
 #include "lldb/Target/DynamicLoader.h"
 #include "lldb/Utility/LLDBLog.h"
@@ -60,6 +62,52 @@ static PluginProperties &GetGlobalPluginProperties() {
   return g_settings;
 }
 
+class CommandObjectProcessFreeBSDKernelCoreRefreshThreads
+    : public CommandObjectParsed {
+public:
+  CommandObjectProcessFreeBSDKernelCoreRefreshThreads(
+      CommandInterpreter &interpreter)
+      : CommandObjectParsed(
+            interpreter, "process plugin refresh-threads",
+            "Refresh the thread list from the FreeBSD kernel core dump. "
+            "This flushes the memory cache and re-reads the kernel's "
+            "allproc/zombie lists to rebuild the thread list from scratch.",
+            "process plugin refresh-threads") {}
+
+  ~CommandObjectProcessFreeBSDKernelCoreRefreshThreads() override = default;
+
+protected:
+  void DoExecute(Args &command, CommandReturnObject &result) override {
+    // TODO: Return early for elf-core based implementation.
+
+    auto *process = static_cast<ProcessFreeBSDKernelCore *>(
+        m_interpreter.GetExecutionContext().GetProcessPtr());
+    if (!process) {
+      result.AppendError("no process");
+      return;
+    }
+
+    // Clear the memory cache so DoUpdateThreadList() will re-read
+    // allproc, zombproc, and all thread/proc structures fresh from
+    // the core dump instead of getting stale cached values.
+    process->m_memory_cache.Clear();
+
+    // Explicitly clear the thread list after Flush() to guarantee that
+    // UpdateThreadListIfNeeded() sees size == 0 and enters the rebuild
+    // path regardless of stop-ID state.
+    process->GetThreadList().Clear();
+
+    // Rebuild the process thread list.
+    process->UpdateThreadListIfNeeded();
+
+    const uint32_t num_threads = process->GetThreadList().GetSize(false);
+    result.AppendMessageWithFormat(
+        "Thread list refreshed, %u thread%s found.\n", num_threads,
+        num_threads == 1 ? "" : "s");
+    result.SetStatus(eReturnStatusSuccessFinishResult);
+  }
+};
+
 ProcessFreeBSDKernelCore::ProcessFreeBSDKernelCore(lldb::TargetSP target_sp,
                                                    ListenerSP listener_sp,
                                                    kvm_t *kvm,
@@ -112,6 +160,22 @@ bool ProcessFreeBSDKernelCore::CanDebug(lldb::TargetSP target_sp,
   return true;
 }
 
+CommandObject *ProcessFreeBSDKernelCore::GetPluginCommandObject() {
+  if (!m_command_sp) {
+    CommandInterpreter &interp =
+        GetTarget().GetDebugger().GetCommandInterpreter();
+    m_command_sp = std::make_unique<CommandObjectMultiword>(
+        interp, "process plugin",
+        "Commands for the FreeBSD kernel process plug-in.",
+        "process plugin <subcommand> [<subcommand-options>]");
+    m_command_sp->LoadSubCommand(
+        "refresh-threads",
+        CommandObjectSP(
+            new CommandObjectProcessFreeBSDKernelCoreRefreshThreads(interp)));
+  }
+  return m_command_sp.get();
+}
+
 Status ProcessFreeBSDKernelCore::DoLoadCore() {
   // The core is already loaded by CreateInstance().
   SetKernelDisplacement();
@@ -157,7 +221,7 @@ size_t ProcessFreeBSDKernelCore::DoWriteMemory(lldb::addr_t addr,
 
 bool ProcessFreeBSDKernelCore::DoUpdateThreadList(ThreadList &old_thread_list,
                                                   ThreadList &new_thread_list) {
-  if (old_thread_list.GetSize(false) == 0) {
+  if (true || old_thread_list.GetSize(false) == 0) {
     // Make up the thread the first time this is called so we can set our one
     // and only core thread state up.
 
diff --git a/lldb/source/Plugins/Process/FreeBSD-Kernel-Core/ProcessFreeBSDKernelCore.h b/lldb/source/Plugins/Process/FreeBSD-Kernel-Core/ProcessFreeBSDKernelCore.h
index d82e55ea74bd9..ac30d9e194305 100644
--- a/lldb/source/Plugins/Process/FreeBSD-Kernel-Core/ProcessFreeBSDKernelCore.h
+++ b/lldb/source/Plugins/Process/FreeBSD-Kernel-Core/ProcessFreeBSDKernelCore.h
@@ -43,6 +43,8 @@ class ProcessFreeBSDKernelCore : public lldb_private::PostMortemProcess {
   bool CanDebug(lldb::TargetSP target_sp,
                 bool plugin_specified_by_name) override;
 
+  lldb_private::CommandObject *GetPluginCommandObject() override;
+
   lldb_private::Status DoLoadCore() override;
 
   lldb_private::DynamicLoader *GetDynamicLoader() override;
@@ -55,6 +57,8 @@ class ProcessFreeBSDKernelCore : public lldb_private::PostMortemProcess {
                        lldb_private::Status &error) override;
 
 protected:
+  friend class CommandObjectProcessFreeBSDKernelCoreRefreshThreads;
+
   bool DoUpdateThreadList(lldb_private::ThreadList &old_thread_list,
                           lldb_private::ThreadList &new_thread_list) override;
 
@@ -70,6 +74,8 @@ class ProcessFreeBSDKernelCore : public lldb_private::PostMortemProcess {
 
   const char *GetError();
 
+  std::unique_ptr<lldb_private::CommandObjectMultiword> m_command_sp;
+
   bool m_printed_unread_message = false;
 
   kvm_t *m_kvm;

[DavidSpickett]

Command looks fine, and it's specific to the plugin which was going to be my first complain if it wasn't. So that part is good.

I don't like this true ||, so let's get to the bottom of that. If you can find an alternative method to call on Process, which does work, that would be ok with me.

[mchoo7]

I wasn't aware of m_thread_list_real. Requesting review again.

[DavidSpickett]

Code looks good to me.

I would like to hear from a FreeBSD developer whether this is in fact something they would like to have. Just a guess will do, it will be easy to change or remove the command later if needed.

Edit: because I want to be reasonably sure we aren't solving a problem that no one actually has. I don't think we are, but I'm not a FreeBSD developer either.

[jrtc27]

How does KGDB handle this problem out of interest?

[emaste]

This seems sensible to me, but I am also curious about what kgdb does. @bsdjhb?

[DavidSpickett]

• This needs a release note.
• Did you find out what KGDB does (or does not)? Curious to know.

[mchoo7]

• This needs a release note.

Created at #191399

• Did you find out what KGDB does (or does not)? Curious to know.

From my observation on KGDB source code (addon to existing GDB code), there is no mechanism that automatically or manually refresh threads. This, however, it might exist in the original GDB source code.

I pinged @bsdjhb but unfortunately I cannot wait indefinitely.

[DavidSpickett]

unfortunately I cannot wait indefinitely.

Yeah that's fine I just wondered if the conversation had happened elsewhere already.