[Lldb-commits] [lldb] [LLDB] Add a way to avoid manual stepping over breakpoints when resuming (PR #165760)

[via lldb-commits]

================
@@ -558,41 +558,46 @@ 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;
+  // 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()) {
----------------
jimingham wrote:

This bit of code was a general "give threads that are going to run a chance to set up their pre-run state in case that effects the thread selection algorithm below.  It so happens that it was only used for inserting the "step over breakpoint" plan.  Your change makes it HAVE to only be about setting up breakpoints before run.  So it might make this structure clearer if you moved this code off to a separate routine that's explicitly about breakpoint stepping and called it here?

Other than that, the generic parts look fine to me, but I defer to Jason for gdb-remote protocol matters.

https://github.com/llvm/llvm-project/pull/165760