[LLDB] Add a way to avoid manual stepping over breakpoints when resuming

Currently, the LLDB client will try to step over breakpoints as part of
the resume operation. However, some servers don't need that
functionality because of performance reasons (e.g. there are too many
threads to step over). This has been mentioned in https://discourse.llvm.org/t/lldb-resume-without-single-stepping-over-breakpoints/88538

This patch adds an lldb-server extension that disables the default
behavior of LLDB. I ended up using the extension list instead of
qHostInfo because qHostInfo is currently building a static list, and
this feature is more of a feature of the server than of the platform or
host.

Author: walter-erquinigo

lldb/include/lldb/Host/common/NativeProcessProtocol.h

@@ -264,8 +264,9 @@ class NativeProcessProtocol {
     memory_tagging = (1u << 6),
     savecore = (1u << 7),
     siginfo_read = (1u << 8),
+    resume_without_disabling_breakpoints = (1u << 9),
 
-    LLVM_MARK_AS_BITMASK_ENUM(siginfo_read)
+    LLVM_MARK_AS_BITMASK_ENUM(resume_without_disabling_breakpoints)
   };
 
   class Manager {

lldb/include/lldb/Target/Process.h

@@ -3076,6 +3076,8 @@ void PruneThreadPlans();
   ///     false otherwise.
   virtual bool SupportsMemoryTagging() { return false; }
 
+  virtual bool SupportsResumeWithoutDisablingBreakpoints() { return false; }
+
   /// Does the final operation to read memory tags. E.g. sending a GDB packet.
   /// It assumes that ReadMemoryTags has checked that memory tagging is enabled
   /// and has expanded the memory range as needed.

lldb/include/lldb/Target/ThreadList.h

@@ -126,6 +126,17 @@ class ThreadList : public ThreadCollection {
   ///
   bool WillResume(lldb::RunDirection &direction);
 
+  /// Set up a step-over breakpoint plan before resuming if needed..
+  ///
+  /// \param[in] thread_to_run
+  ///     An optional thread that will have the run priority in the resume.
+  ///
+  /// \param[in] direction
+  ///     The direction to run in.
+  void
+  SetUpStepOverBreakpointBeforeResumeIfNeeded(lldb::ThreadSP thread_to_run,
+                                              lldb::RunDirection &direction);
+
   void DidResume();
 
   void DidStop();

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

@@ -373,6 +373,7 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
   m_supports_reverse_continue = eLazyBoolNo;
   m_supports_reverse_step = eLazyBoolNo;
   m_supports_multi_mem_read = eLazyBoolNo;
+  m_supports_resume_without_disabling_breakpoints = eLazyBoolNo;
 
   m_max_packet_size = UINT64_MAX; // It's supposed to always be there, but if
                                   // not, we assume no limit
@@ -434,6 +435,8 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
         m_supports_reverse_step = eLazyBoolYes;
       else if (x == "MultiMemRead+")
         m_supports_multi_mem_read = eLazyBoolYes;
+      else if (x == "resume-without-disabling-breakpoints+")
+        m_supports_resume_without_disabling_breakpoints = eLazyBoolYes;
       // Look for a list of compressions in the features list e.g.
       // qXfer:features:read+;PacketSize=20000;qEcho+;SupportedCompressions=zlib-
       // deflate,lzma
@@ -686,6 +689,13 @@ bool GDBRemoteCommunicationClient::GetMemoryTaggingSupported() {
   return m_supports_memory_tagging == eLazyBoolYes;
 }
 
+bool GDBRemoteCommunicationClient::
+    GetResumeWithoutDisablingBreakpointsSupported() {
+  if (m_supports_resume_without_disabling_breakpoints == eLazyBoolCalculate)
+    GetRemoteQSupported();
+  return m_supports_resume_without_disabling_breakpoints == eLazyBoolYes;
+}
+
 DataBufferSP GDBRemoteCommunicationClient::ReadMemoryTags(lldb::addr_t addr,
                                                           size_t len,
                                                           int32_t type) {

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h

@@ -445,6 +445,8 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase {
 
   bool GetMemoryTaggingSupported();
 
+  bool GetResumeWithoutDisablingBreakpointsSupported();
+
   bool UsesNativeSignals();
 
   lldb::DataBufferSP ReadMemoryTags(lldb::addr_t addr, size_t len,
@@ -577,6 +579,7 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase {
   LazyBool m_supports_reverse_continue = eLazyBoolCalculate;
   LazyBool m_supports_reverse_step = eLazyBoolCalculate;
   LazyBool m_supports_multi_mem_read = eLazyBoolCalculate;
+  LazyBool m_supports_resume_without_disabling_breakpoints = eLazyBoolCalculate;
 
   bool m_supports_qProcessInfoPID : 1, m_supports_qfProcessInfo : 1,
       m_supports_qUserName : 1, m_supports_qGroupName : 1,

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

@@ -4326,6 +4326,8 @@ std::vector<std::string> GDBRemoteCommunicationServerLLGS::HandleFeatures(
     ret.push_back("memory-tagging+");
   if (bool(plugin_features & Extension::savecore))
     ret.push_back("qSaveCore+");
+  if (bool(plugin_features & Extension::resume_without_disabling_breakpoints))
+    ret.push_back("resume-without-disabling-breakpoints+");
 
   // check for client features
   m_extensions_supported = {};

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

@@ -2869,6 +2869,10 @@ bool ProcessGDBRemote::SupportsMemoryTagging() {
   return m_gdb_comm.GetMemoryTaggingSupported();
 }
 
+bool ProcessGDBRemote::SupportsResumeWithoutDisablingBreakpoints() {
+  return m_gdb_comm.GetResumeWithoutDisablingBreakpointsSupported();
+}
+
 llvm::Expected<std::vector<uint8_t>>
 ProcessGDBRemote::DoReadMemoryTags(lldb::addr_t addr, size_t len,
                                    int32_t type) {

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

@@ -266,6 +266,10 @@ class ProcessGDBRemote : public Process,
 
   bool SupportsMemoryTagging() override;
 
+  /// \returns true if the process doesn't need the client to disable
+  /// breakpoints before issuing a resume operation.
+  bool SupportsResumeWithoutDisablingBreakpoints() override;
+
   /// Broadcaster event bits definitions.
   enum {
     eBroadcastBitAsyncContinue = (1 << 0),

lldb/source/Target/ThreadList.cpp

@@ -494,6 +494,49 @@ void ThreadList::DiscardThreadPlans() {
     (*pos)->DiscardThreadPlans(true);
 }
 
+void ThreadList::SetUpStepOverBreakpointBeforeResumeIfNeeded(
+    ThreadSP thread_to_run, RunDirection &direction) {
+  // If the process doesn't need the client to disable breakpoints before
+  // issuing a resume operation, we can skip the step-over breakpoint plan
+  // setup.
+  if (!m_process.SupportsResumeWithoutDisablingBreakpoints())
+    return;
+
+  // Give all the threads that are likely to run a chance to set up their
+  // step over breakpoint behavior.
+  if (thread_to_run != nullptr) {
+    if (thread_to_run->SetupToStepOverBreakpointIfNeeded(direction)) {
+      // We only need to step over breakpoints when running forward, and the
+      // step-over-breakpoint plan itself wants to run forward, so this
+      // keeps our desired direction.
+      assert(thread_to_run->GetCurrentPlan()->GetDirection() == direction);
+    }
+  } else {
+    collection::iterator end = m_threads.end();
+    for (auto pos = m_threads.begin(); pos != end; ++pos) {
+      ThreadSP thread_sp(*pos);
+      if (thread_sp->GetResumeState() != eStateSuspended) {
+        if (thread_sp->IsOperatingSystemPluginThread() &&
+            !thread_sp->GetBackingThread())
+          continue;
+        if (thread_sp->SetupToStepOverBreakpointIfNeeded(direction)) {
+          // We only need to step over breakpoints when running forward, and the
+          // step-over-breakpoint plan itself wants to run forward, so this
+          // keeps our desired direction.
+          assert(thread_sp->GetCurrentPlan()->GetDirection() == direction);
+          // You can't say "stop others" and also want yourself to be suspended.
+          assert(thread_sp->GetCurrentPlan()->RunState() != eStateSuspended);
+          thread_to_run = thread_sp;
+          if (thread_sp->ShouldRunBeforePublicStop()) {
+            // This takes precedence, so if we find one of these, service it:
+            break;
+          }
+        }
+      }
+    }
+  }
+}
+
 bool ThreadList::WillResume(RunDirection &direction) {
   // Run through the threads and perform their momentary actions. But we only
   // do this for threads that are running, user suspended threads stay where
@@ -558,46 +601,7 @@ bool ThreadList::WillResume(RunDirection &direction) {
     direction = m_process.GetBaseDirection();
   }
 
-  // Give all the threads that are likely to run a last chance to set up their
-  // state before we negotiate who is actually going to get a chance to run...
-  // Don't set to resume suspended threads, and if any thread wanted to stop
-  // others, only call setup on the threads that request StopOthers...
-  if (thread_to_run != nullptr) {
-    // See if any thread wants to run stopping others.  If it does, then we
-    // won't setup the other threads for resume, since they aren't going to get
-    // a chance to run.  This is necessary because the SetupForResume might add
-    // "StopOthers" plans which would then get to be part of the who-gets-to-run
-    // negotiation, but they're coming in after the fact, and the threads that
-    // are already set up should take priority.
-    if (thread_to_run->SetupToStepOverBreakpointIfNeeded(direction)) {
-      // We only need to step over breakpoints when running forward, and the
-      // step-over-breakpoint plan itself wants to run forward, so this
-      // keeps our desired direction.
-      assert(thread_to_run->GetCurrentPlan()->GetDirection() == direction);
-    }
-  } else {
-    for (pos = m_threads.begin(); pos != end; ++pos) {
-      ThreadSP thread_sp(*pos);
-      if (thread_sp->GetResumeState() != eStateSuspended) {
-        if (thread_sp->IsOperatingSystemPluginThread() &&
-            !thread_sp->GetBackingThread())
-          continue;
-        if (thread_sp->SetupToStepOverBreakpointIfNeeded(direction)) {
-          // We only need to step over breakpoints when running forward, and the
-          // step-over-breakpoint plan itself wants to run forward, so this
-          // keeps our desired direction.
-          assert(thread_sp->GetCurrentPlan()->GetDirection() == direction);
-          // You can't say "stop others" and also want yourself to be suspended.
-          assert(thread_sp->GetCurrentPlan()->RunState() != eStateSuspended);
-          thread_to_run = thread_sp;
-          if (thread_sp->ShouldRunBeforePublicStop()) {
-            // This takes precedence, so if we find one of these, service it:
-            break;
-          }
-        }
-      }
-    }
-  }
+  SetUpStepOverBreakpointBeforeResumeIfNeeded(thread_to_run, direction);
 
   if (thread_to_run != nullptr) {
     Log *log = GetLog(LLDBLog::Step);