labath updated this revision to Diff 100099.
labath added a comment.

Fixed version.

The original patch caused a regression in TestLoadUnload, which has only showed
up when running the remote test suite. The problem there was that we interrupted
the target just as it has hit the rendezvous breakpoint in the dlopen call. This
meant that the stop reason was set to "breakpoint" even though the event would
not have been broadcast if we had not stopped the process. I fix this by
checking StopInfo->ShouldNotify() before stopping.

I was hoping I would be able to create a more reliable test for this bug by
calling an expression which will hit a conditional breakpoint, whose condition
will evaluate to false. However, I have found that this does not work at all --
we always stop at the breakpoint, regardless of the expression. So, I add a
(disabled) test for that instead.


https://reviews.llvm.org/D33283

Files:
  
packages/Python/lldbsuite/test/expression_command/unwind_expression/TestUnwindExpression.py
  packages/Python/lldbsuite/test/expression_command/unwind_expression/main.cpp
  source/Target/Process.cpp

Index: source/Target/Process.cpp
===================================================================
--- source/Target/Process.cpp
+++ source/Target/Process.cpp
@@ -4823,6 +4823,48 @@
     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 &&
+      stop_info_sp->ShouldNotify(event_sp.get())) {
+    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 +5270,22 @@
                               "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 +5391,6 @@
               }
 
               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 +5404,18 @@
                   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 "
Index: packages/Python/lldbsuite/test/expression_command/unwind_expression/main.cpp
===================================================================
--- packages/Python/lldbsuite/test/expression_command/unwind_expression/main.cpp
+++ packages/Python/lldbsuite/test/expression_command/unwind_expression/main.cpp
@@ -7,8 +7,16 @@
     return static_value;
 }
 
+int second_function(int x){
+  for(int i=0; i<10; ++i) {
+    a_function_to_call();
+  }
+  return x;
+}
+
 int main (int argc, char const *argv[])
 {
     a_function_to_call();  // Set a breakpoint here to get started 
+    second_function(1);
     return 0;
 }
Index: packages/Python/lldbsuite/test/expression_command/unwind_expression/TestUnwindExpression.py
===================================================================
--- packages/Python/lldbsuite/test/expression_command/unwind_expression/TestUnwindExpression.py
+++ packages/Python/lldbsuite/test/expression_command/unwind_expression/TestUnwindExpression.py
@@ -18,26 +18,19 @@
 class UnwindFromExpressionTest(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
+    main_spec = lldb.SBFileSpec("main.cpp", False)
 
-    def setUp(self):
-        # Call super's setUp().
-        TestBase.setUp(self)
-
-    @add_test_categories(['pyapi'])
-    @expectedFailureAll(oslist=["windows"])
-    def test_unwind_expression(self):
-        """Test unwinding from an expression."""
+    def build_and_run_to_bkpt(self):
         self.build()
 
         exe = os.path.join(os.getcwd(), "a.out")
 
         target = self.dbg.CreateTarget(exe)
         self.assertTrue(target, VALID_TARGET)
 
         # Create the breakpoint.
-        main_spec = lldb.SBFileSpec("main.cpp", False)
         breakpoint = target.BreakpointCreateBySourceRegex(
-            "// Set a breakpoint here to get started", main_spec)
+            "// Set a breakpoint here to get started", self.main_spec)
         self.assertTrue(breakpoint, VALID_BREAKPOINT)
 
         # Launch the process, and do not stop at the entry point.
@@ -52,24 +45,60 @@
                       "instead the actual state is: '%s'" %
                       lldbutil.state_type_to_str(process.GetState()))
 
-        thread = lldbutil.get_one_thread_stopped_at_breakpoint(
+        self.thread = lldbutil.get_one_thread_stopped_at_breakpoint(
             process, breakpoint)
         self.assertIsNotNone(
-            thread, "Expected one thread to be stopped at the breakpoint")
+            self.thread, "Expected one thread to be stopped at the breakpoint")
+
+        # Next set a breakpoint in this function, set up Expression options to stop on
+        # breakpoint hits, and call the function.
+        self.fun_bkpt = self.target().BreakpointCreateBySourceRegex(
+            "// Stop inside the function here.", self.main_spec)
+        self.assertTrue(self.fun_bkpt, VALID_BREAKPOINT)
+
+
+    @no_debug_info_test
+    @expectedFailureAll()# TODO: Add bug # if Jim confirms this is a bug
+    def test_conditional_bktp(self):
+        """
+        Test conditional breakpoint handling in the IgnoreBreakpoints = False case
+        """
+        self.build_and_run_to_bkpt()
+
+        self.fun_bkpt.SetCondition("0") # Should not get hit
+        options = lldb.SBExpressionOptions()
+        options.SetIgnoreBreakpoints(False)
+        options.SetUnwindOnError(False)
+
+        main_frame = self.thread.GetFrameAtIndex(0)
+        val = main_frame.EvaluateExpression("second_function(47)", options)
+        self.assertTrue(
+            val.GetError().Success(),
+            "We did complete the execution.")
+        self.assertEquals(47, val.GetValueAsSigned())
 
+
+    @add_test_categories(['pyapi'])
+    @expectedFailureAll(oslist=["windows"])
+    def test_unwind_expression(self):
+        """Test unwinding from an expression."""
+        self.build_and_run_to_bkpt()
+
+        # Run test with varying one thread timeouts to also test the halting
+        # logic in the IgnoreBreakpoints = False case
+        self.do_unwind_test(self.thread, self.fun_bkpt, 1000)
+        self.do_unwind_test(self.thread, self.fun_bkpt, 100000)
+
+    def do_unwind_test(self, thread, bkpt, timeout):
         #
         # 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)
         options = lldb.SBExpressionOptions()
         options.SetIgnoreBreakpoints(False)
         options.SetUnwindOnError(False)
+        options.SetOneThreadTimeoutInMicroSeconds(timeout)
 
         val = main_frame.EvaluateExpression("a_function_to_call()", options)
 
@@ -82,7 +111,7 @@
             "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")
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to