[Lldb-commits] [lldb] r303732 - RunThreadPlan: Fix halting logic in IgnoreBreakpoints = false

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Wed May 24 02:46:48 PDT 2017


Author: labath
Date: Wed May 24 04:46:48 2017
New Revision: 303732

URL: http://llvm.org/viewvc/llvm-project?rev=303732&view=rev
Log:
RunThreadPlan: Fix halting logic in IgnoreBreakpoints = false

Summary:
The function had logic to handle the case when the expression terminated
while we were trying to halt the process, but it failed to take into
account the possibility that the expression stopped because it hit a
breakpoint. This was caused by the fact that the handling of the stopped
events was duplicated for the "halting" and regular cases (the regular
case handled this situation correctly). I've tried to merge these two
cases into one to make sure they stay in sync.

I should call out that the two cases were checking whether the thread
plan has completed in slightly different ways. I am not sure what is the
difference between them, but I think the check should be the same in
both cases, whatever it is, so I just took the one from the regular
case, as that is probably more tested.

For the test, I modified TestUnwindExpression to run the expression with
a smaller timeout (this is how I found this bug originally). With a 1ms
one thread timeout, the test failed consistently without this patch.

Reviewers: jingham

Subscribers: lldb-commits

Differential Revision: https://reviews.llvm.org/D33283

Modified:
    lldb/trunk/packages/Python/lldbsuite/test/expression_command/unwind_expression/TestUnwindExpression.py
    lldb/trunk/source/Target/Process.cpp

Modified: lldb/trunk/packages/Python/lldbsuite/test/expression_command/unwind_expression/TestUnwindExpression.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/unwind_expression/TestUnwindExpression.py?rev=303732&r1=303731&r2=303732&view=diff
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/expression_command/unwind_expression/TestUnwindExpression.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/expression_command/unwind_expression/TestUnwindExpression.py Wed May 24 04:46:48 2017
@@ -57,19 +57,27 @@ class UnwindFromExpressionTest(TestBase)
         self.assertIsNotNone(
             thread, "Expected one thread to be stopped at the breakpoint")
 
-        #
-        # Use Python API to evaluate expressions while stopped in a stack frame.
-        #
-        main_frame = thread.GetFrameAtIndex(0)
-
         # Next set a breakpoint in this function, set up Expression options to stop on
         # breakpoint hits, and call the function.
         fun_bkpt = target.BreakpointCreateBySourceRegex(
             "// Stop inside the function here.", main_spec)
         self.assertTrue(fun_bkpt, VALID_BREAKPOINT)
+
+        # Run test with varying one thread timeouts to also test the halting
+        # logic in the IgnoreBreakpoints = False case
+        self.do_test(thread, fun_bkpt, 1000)
+        self.do_test(thread, fun_bkpt, 100000)
+
+    def do_test(self, thread, bkpt, timeout):
+        #
+        # Use Python API to evaluate expressions while stopped in a stack frame.
+        #
+        main_frame = thread.GetFrameAtIndex(0)
+
         options = lldb.SBExpressionOptions()
         options.SetIgnoreBreakpoints(False)
         options.SetUnwindOnError(False)
+        options.SetOneThreadTimeoutInMicroSeconds(timeout)
 
         val = main_frame.EvaluateExpression("a_function_to_call()", options)
 
@@ -82,7 +90,7 @@ class UnwindFromExpressionTest(TestBase)
             "And the reason was right.")
 
         thread = lldbutil.get_one_thread_stopped_at_breakpoint(
-            process, fun_bkpt)
+            self.process(), bkpt)
         self.assertTrue(
             thread.IsValid(),
             "We are indeed stopped at our breakpoint")

Modified: lldb/trunk/source/Target/Process.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Target/Process.cpp?rev=303732&r1=303731&r2=303732&view=diff
==============================================================================
--- lldb/trunk/source/Target/Process.cpp (original)
+++ lldb/trunk/source/Target/Process.cpp Wed May 24 04:46:48 2017
@@ -4823,6 +4823,47 @@ GetExpressionTimeout(const EvaluateExpre
     return *options.GetTimeout() - GetOneThreadExpressionTimeout(options);
 }
 
+static llvm::Optional<ExpressionResults>
+HandleStoppedEvent(Thread &thread, const ThreadPlanSP &thread_plan_sp,
+                   RestorePlanState &restorer, const EventSP &event_sp,
+                   EventSP &event_to_broadcast_sp,
+                   const EvaluateExpressionOptions &options, bool handle_interrupts) {
+  Log *log = GetLogIfAnyCategoriesSet(LIBLLDB_LOG_STEP | LIBLLDB_LOG_PROCESS);
+
+  ThreadPlanSP plan = thread.GetCompletedPlan();
+  if (plan == thread_plan_sp && plan->PlanSucceeded()) {
+    LLDB_LOG(log, "execution completed successfully");
+
+    // Restore the plan state so it will get reported as intended when we are
+    // done.
+    restorer.Clean();
+    return eExpressionCompleted;
+  }
+
+  StopInfoSP stop_info_sp = thread.GetStopInfo();
+  if (stop_info_sp && stop_info_sp->GetStopReason() == eStopReasonBreakpoint) {
+    LLDB_LOG(log, "stopped for breakpoint: {0}.", stop_info_sp->GetDescription());
+    if (!options.DoesIgnoreBreakpoints()) {
+      // Restore the plan state and then force Private to false.  We are going
+      // to stop because of this plan so we need it to become a public plan or
+      // it won't report correctly when we continue to its termination later on.
+      restorer.Clean();
+      thread_plan_sp->SetPrivate(false);
+      event_to_broadcast_sp = event_sp;
+    }
+    return eExpressionHitBreakpoint;
+  }
+
+  if (!handle_interrupts &&
+      Process::ProcessEventData::GetInterruptedFromEvent(event_sp.get()))
+    return llvm::None;
+
+  LLDB_LOG(log, "thread plan did not successfully complete");
+  if (!options.DoesUnwindOnError())
+    event_to_broadcast_sp = event_sp;
+  return eExpressionInterrupted;
+}
+
 ExpressionResults
 Process::RunThreadPlan(ExecutionContext &exe_ctx,
                        lldb::ThreadPlanSP &thread_plan_sp,
@@ -5228,65 +5269,22 @@ Process::RunThreadPlan(ExecutionContext
                               "but our thread (index-id=%u) has vanished.",
                               thread_idx_id);
                 return_value = eExpressionInterrupted;
-              } else {
+              } else if (Process::ProcessEventData::GetRestartedFromEvent(
+                             event_sp.get())) {
                 // If we were restarted, we just need to go back up to fetch
                 // another event.
-                if (Process::ProcessEventData::GetRestartedFromEvent(
-                        event_sp.get())) {
-                  if (log) {
-                    log->Printf("Process::RunThreadPlan(): Got a stop and "
-                                "restart, so we'll continue waiting.");
-                  }
-                  keep_going = true;
-                  do_resume = false;
-                  handle_running_event = true;
-                } else {
-                  ThreadPlanSP plan = thread->GetCompletedPlan();
-                  if (plan == thread_plan_sp && plan->PlanSucceeded()) {
-
-                    if (log)
-                      log->PutCString("Process::RunThreadPlan(): execution "
-                                      "completed successfully.");
-
-                    // Restore the plan state so it will get reported as
-                    // intended when we are done.
-                    thread_plan_restorer.Clean();
-
-                    return_value = eExpressionCompleted;
-                  } else {
-                    StopInfoSP stop_info_sp = thread_sp->GetStopInfo();
-                    // Something restarted the target, so just wait for it to
-                    // stop for real.
-                    if (stop_info_sp &&
-                        stop_info_sp->GetStopReason() == eStopReasonBreakpoint) {
-                      if (log)
-                        log->Printf("Process::RunThreadPlan() stopped for "
-                                    "breakpoint: %s.",
-                                    stop_info_sp->GetDescription());
-                      return_value = eExpressionHitBreakpoint;
-                      if (!options.DoesIgnoreBreakpoints()) {
-                        // Restore the plan state and then force Private to
-                        // false.  We are
-                        // going to stop because of this plan so we need it to
-                        // become a public
-                        // plan or it won't report correctly when we continue to
-                        // its termination
-                        // later on.
-                        thread_plan_restorer.Clean();
-                        if (thread_plan_sp)
-                          thread_plan_sp->SetPrivate(false);
-                        event_to_broadcast_sp = event_sp;
-                      }
-                    } else {
-                      if (log)
-                        log->PutCString("Process::RunThreadPlan(): thread plan "
-                                        "didn't successfully complete.");
-                      if (!options.DoesUnwindOnError())
-                        event_to_broadcast_sp = event_sp;
-                      return_value = eExpressionInterrupted;
-                    }
-                  }
+                if (log) {
+                  log->Printf("Process::RunThreadPlan(): Got a stop and "
+                              "restart, so we'll continue waiting.");
                 }
+                keep_going = true;
+                do_resume = false;
+                handle_running_event = true;
+              } else {
+                const bool handle_interrupts = true;
+                return_value = *HandleStoppedEvent(
+                    *thread, thread_plan_sp, thread_plan_restorer, event_sp,
+                    event_to_broadcast_sp, options, handle_interrupts);
               }
             } break;
 
@@ -5392,20 +5390,6 @@ Process::RunThreadPlan(ExecutionContext
               }
 
               if (stop_state == lldb::eStateStopped) {
-                // Between the time we initiated the Halt and the time we
-                // delivered it, the process could have
-                // already finished its job.  Check that here:
-
-                if (thread->IsThreadPlanDone(thread_plan_sp.get())) {
-                  if (log)
-                    log->PutCString("Process::RunThreadPlan(): Even though we "
-                                    "timed out, the call plan was done.  "
-                                    "Exiting wait loop.");
-                  return_value = eExpressionCompleted;
-                  back_to_top = false;
-                  break;
-                }
-
                 if (Process::ProcessEventData::GetRestartedFromEvent(
                         event_sp.get())) {
                   if (log)
@@ -5419,6 +5403,18 @@ Process::RunThreadPlan(ExecutionContext
                   continue;
                 }
 
+                // Between the time we initiated the Halt and the time we
+                // delivered it, the process could have
+                // already finished its job.  Check that here:
+                const bool handle_interrupts = false;
+                if (auto result = HandleStoppedEvent(
+                        *thread, thread_plan_sp, thread_plan_restorer, event_sp,
+                        event_to_broadcast_sp, options, handle_interrupts)) {
+                  return_value = *result;
+                  back_to_top = false;
+                  break;
+                }
+
                 if (!options.GetTryAllThreads()) {
                   if (log)
                     log->PutCString("Process::RunThreadPlan(): try_all_threads "




More information about the lldb-commits mailing list