Skip to content

Conversation

@felipepiovezan
Copy link
Contributor

Currently, an LLDB target option controls whether plugins report all threads. However, it seems natural for this knowledge could come from the plugin itself. To support this, this commits adds a virtual method to the plugin base class, making the Python OS query the target option to preserve existing behavior.

Currently, an LLDB target option controls whether plugins report all
threads. However, it seems natural for this knowledge could come from
the plugin itself. To support this, this commits adds a virtual method
to the plugin base class, making the Python OS query the target option
to preserve existing behavior.
@llvmbot
Copy link
Member

llvmbot commented Jan 15, 2025

@llvm/pr-subscribers-lldb

Author: Felipe de Azevedo Piovezan (felipepiovezan)

Changes

Currently, an LLDB target option controls whether plugins report all threads. However, it seems natural for this knowledge could come from the plugin itself. To support this, this commits adds a virtual method to the plugin base class, making the Python OS query the target option to preserve existing behavior.


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

5 Files Affected:

  • (modified) lldb/include/lldb/Target/OperatingSystem.h (+2)
  • (modified) lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp (+4)
  • (modified) lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.h (+2)
  • (modified) lldb/source/Target/Process.cpp (+1-1)
  • (modified) lldb/source/Target/TargetProperties.td (+1-1)
diff --git a/lldb/include/lldb/Target/OperatingSystem.h b/lldb/include/lldb/Target/OperatingSystem.h
index ceeddceb0f2c12..128239569790fc 100644
--- a/lldb/include/lldb/Target/OperatingSystem.h
+++ b/lldb/include/lldb/Target/OperatingSystem.h
@@ -61,6 +61,8 @@ class OperatingSystem : public PluginInterface {
 
   virtual bool IsOperatingSystemPluginThread(const lldb::ThreadSP &thread_sp);
 
+  virtual bool DoesPluginReportAllThreads() = 0;
+
 protected:
   // Member variables.
   Process
diff --git a/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp b/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
index 3848a2b1deb97f..7230e590ba03b9 100644
--- a/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
+++ b/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
@@ -386,4 +386,8 @@ lldb::ThreadSP OperatingSystemPython::CreateThread(lldb::tid_t tid,
   return ThreadSP();
 }
 
+bool OperatingSystemPython::DoesPluginReportAllThreads() {
+  return m_process->GetOSPluginReportsAllThreads();
+}
+
 #endif // #if LLDB_ENABLE_PYTHON
diff --git a/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.h b/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.h
index 90973acde3ebfd..980a544241de4f 100644
--- a/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.h
+++ b/lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.h
@@ -60,6 +60,8 @@ class OperatingSystemPython : public lldb_private::OperatingSystem {
   // Method for lazy creation of threads on demand
   lldb::ThreadSP CreateThread(lldb::tid_t tid, lldb::addr_t context) override;
 
+  bool DoesPluginReportAllThreads() override;
+
 protected:
   bool IsValid() const {
     return m_script_object_sp && m_script_object_sp->IsValid();
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 68485a40a3fcce..c47e728fdf716c 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -1182,7 +1182,7 @@ void Process::UpdateThreadListIfNeeded() {
           // See if the OS plugin reports all threads.  If it does, then
           // it is safe to clear unseen thread's plans here.  Otherwise we
           // should preserve them in case they show up again:
-          clear_unused_threads = GetOSPluginReportsAllThreads();
+          clear_unused_threads = os->DoesPluginReportAllThreads();
 
           // Turn off dynamic types to ensure we don't run any expressions.
           // Objective-C can run an expression to determine if a SBValue is a
diff --git a/lldb/source/Target/TargetProperties.td b/lldb/source/Target/TargetProperties.td
index bb3b500d5fdfbd..38a345dfd88495 100644
--- a/lldb/source/Target/TargetProperties.td
+++ b/lldb/source/Target/TargetProperties.td
@@ -223,7 +223,7 @@ let Definition = "process_experimental" in {
   def OSPluginReportsAllThreads: Property<"os-plugin-reports-all-threads", "Boolean">,
     Global,
     DefaultTrue,
-    Desc<"Set to False if your OS Plugins doesn't report all threads on each stop.">;
+    Desc<"Set to False if your Python OS Plugin doesn't report all threads on each stop.">;
 }
 
 let Definition = "process" in {

print("Could not find g_value")

def does_plugin_report_all_threads(self):
return False
Copy link
Contributor Author

@felipepiovezan felipepiovezan Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW I have changed this to True and observed the test failing

Copy link
Collaborator

@jimingham jimingham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@felipepiovezan felipepiovezan merged commit cb82771 into llvm:main Jan 16, 2025
5 of 6 checks passed
@felipepiovezan felipepiovezan deleted the felipe/reports_all_threads_option branch January 16, 2025 23:05
felipepiovezan added a commit to felipepiovezan/llvm-project that referenced this pull request Jan 16, 2025
Currently, an LLDB target option controls whether plugins report all
threads. However, it seems natural for this knowledge could come from
the plugin itself. To support this, this commits adds a virtual method
to the plugin base class, making the Python OS query the target option
to preserve existing behavior.

(cherry picked from commit cb82771)
adrian-prantl added a commit to swiftlang/llvm-project that referenced this pull request Jan 17, 2025
…n-reports-all-threads

[cherry-pick][lldb] Add OS plugin property for reporting all threads (llvm#123145)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants