jingham created this revision.
jingham added a reviewer: friss.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
jingham requested review of this revision.
Herald added a subscriber: JDevlieghere.

There are some platforms (the xnu kernel being one) where trying to run just 
one thread can cause problems, and you need to always run all threads whenever 
you do more than a single step-i.

This patch adds a target.process.run-all-threads setting which overrides the 
default "run-mode" value for all the stepping commands.

I also needed to come up with a way to test this.  The easiest way to do that 
was to use a scripted thread plan so that I could check the incoming value.  
But to do that I needed to plumb the stop others through the scripted-step & 
the ThreadPlanPython, which this patch also does.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85265

Files:
  lldb/bindings/interface/SBThreadPlan.i
  lldb/include/lldb/API/SBThreadPlan.h
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/ThreadPlanPython.h
  lldb/source/API/SBThreadPlan.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/TargetProperties.td
  lldb/source/Target/Thread.cpp
  lldb/source/Target/ThreadPlanPython.cpp
  lldb/test/API/functionalities/step_scripted/Steps.py
  lldb/test/API/functionalities/step_scripted/TestStepScripted.py

Index: lldb/test/API/functionalities/step_scripted/TestStepScripted.py
===================================================================
--- lldb/test/API/functionalities/step_scripted/TestStepScripted.py
+++ lldb/test/API/functionalities/step_scripted/TestStepScripted.py
@@ -1,7 +1,7 @@
 """
 Tests stepping with scripted thread plans.
 """
-
+import threading
 import lldb
 import lldbsuite.test.lldbutil as lldbutil
 from lldbsuite.test.decorators import *
@@ -111,3 +111,58 @@
 
         # And foo should have changed:
         self.assertTrue(foo_val.GetValueDidChange(), "Foo changed")
+
+    def test_stop_others_from_command(self):
+        """Test that the stop-others flag is set correctly by the command line.
+           Also test that the run-all-threads property overrides this."""
+        self.do_test_stop_others()
+
+    def run_step(self, stop_others_value, run_mode, token):
+        import Steps
+        interp = self.dbg.GetCommandInterpreter()
+        result = lldb.SBCommandReturnObject()
+
+        cmd = "thread step-scripted -C Steps.StepReportsStopOthers -k token -v %s"%(token)
+        if run_mode != None:
+            cmd = cmd + " --run-mode %s"%(run_mode)
+        print(cmd)
+        interp.HandleCommand(cmd, result)
+        self.assertTrue(result.Succeeded(), "Step scripted failed: %s."%(result.GetError()))
+        print(Steps.StepReportsStopOthers.stop_mode_dict)
+        value = Steps.StepReportsStopOthers.stop_mode_dict[token]
+        self.assertEqual(value, stop_others_value, "Stop others has the correct value.")
+        
+    def do_test_stop_others(self):
+        self.build()
+        (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
+                                                                            "Set a breakpoint here",
+                                                                            self.main_source_file)
+        # First run with stop others false and see that we got that.
+        thread_id = ""
+        if sys.version_info.major == 2:
+            thread_id = str(threading._get_ident())
+        else:
+            thread_id = str(threading.get_ident())
+
+        # all-threads should set stop others to False.
+        self.run_step(False, "all-threads", thread_id)
+
+        # this-thread should set stop others to True
+        self.run_step(True, "this-thread", thread_id)
+
+        # The default value should be stop others:
+        self.run_step(True, None, thread_id)
+
+        # The target.process.run-all-threads should override this:
+        interp = self.dbg.GetCommandInterpreter()
+        result = lldb.SBCommandReturnObject()
+        
+        interp.HandleCommand("settings set target.process.run-all-threads true", result)
+        self.assertTrue(result.Succeeded, "setting run-all-threads works.")
+
+        self.run_step(False, None, thread_id)
+
+        
+        
+
+        
Index: lldb/test/API/functionalities/step_scripted/Steps.py
===================================================================
--- lldb/test/API/functionalities/step_scripted/Steps.py
+++ lldb/test/API/functionalities/step_scripted/Steps.py
@@ -75,9 +75,29 @@
         if not self.value.IsValid():
             return True
 
-        print("Got next value: %d"%(self.value.GetValueAsUnsigned()))
         if not self.value.GetValueDidChange():
             self.child_thread_plan = self.queue_child_thread_plan()
             return False
         else:
             return True
+
+# This plan does nothing, but sets stop_mode to the
+# value of GetStopOthers for this plan.
+class StepReportsStopOthers():
+    stop_mode_dict = {}
+    
+    def __init__(self, thread_plan, args_data, dict):
+        self.thread_plan = thread_plan
+        self.key = args_data.GetValueForKey("token").GetStringValue(1000)
+        
+    def should_stop(self, event):
+        self.thread_plan.SetPlanComplete(True)
+        print("Called in should_stop")
+        StepReportsStopOthers.stop_mode_dict[self.key] = self.thread_plan.GetStopOthers()
+        return True
+
+    def should_step(self):
+        return True
+
+    def explains_stop(self, event):
+        return True
Index: lldb/source/Target/ThreadPlanPython.cpp
===================================================================
--- lldb/source/Target/ThreadPlanPython.cpp
+++ lldb/source/Target/ThreadPlanPython.cpp
@@ -29,7 +29,8 @@
                                    StructuredDataImpl *args_data)
     : ThreadPlan(ThreadPlan::eKindPython, "Python based Thread Plan", thread,
                  eVoteNoOpinion, eVoteNoOpinion),
-      m_class_name(class_name), m_args_data(args_data), m_did_push(false) {
+      m_class_name(class_name), m_args_data(args_data), m_did_push(false),
+      m_stop_others(false) {
   SetIsMasterPlan(true);
   SetOkayToDiscard(true);
   SetPrivate(false);
@@ -162,13 +163,6 @@
 }
 
 // The ones below are not currently exported to Python.
-
-bool ThreadPlanPython::StopOthers() {
-  // For now Python plans run all threads, but we should add some controls for
-  // this.
-  return false;
-}
-
 void ThreadPlanPython::GetDescription(Stream *s, lldb::DescriptionLevel level) {
   s->Printf("Python thread plan implemented by class %s.",
             m_class_name.c_str());
Index: lldb/source/Target/Thread.cpp
===================================================================
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -1380,7 +1380,7 @@
 
   ThreadPlanSP thread_plan_sp(new ThreadPlanPython(*this, class_name, 
                                                    extra_args_impl));
-
+  thread_plan_sp->SetStopOthers(stop_other_threads);                                                 
   status = QueueThreadPlan(thread_plan_sp, abort_other_plans);
   return thread_plan_sp;
 }
Index: lldb/source/Target/TargetProperties.td
===================================================================
--- lldb/source/Target/TargetProperties.td
+++ lldb/source/Target/TargetProperties.td
@@ -214,6 +214,9 @@
   def UtilityExpressionTimeout: Property<"utility-expression-timeout", "UInt64">,
     DefaultUnsignedValue<15>,
     Desc<"The time in seconds to wait for LLDB-internal utility expressions.">;
+  def SteppingRunsAllThreads: Property<"run-all-threads", "Boolean">,
+    DefaultFalse,
+    Desc<"If true, stepping operations will run all threads.  This is equivalent to setting the run-mode option to 'all-threads'.">;
 }
 
 let Definition = "platform" in {
Index: lldb/source/Target/Process.cpp
===================================================================
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -278,6 +278,12 @@
   return std::chrono::seconds(value);
 }
 
+bool ProcessProperties::GetSteppingRunsAllThreads() const {
+  const uint32_t idx = ePropertySteppingRunsAllThreads;
+  return m_collection_sp->GetPropertyAtIndexAsBoolean(
+      nullptr, idx, g_process_properties[idx].default_uint_value != 0);
+}
+
 bool ProcessProperties::GetOSPluginReportsAllThreads() const {
   const bool fail_value = true;
   const Property *exp_property =
Index: lldb/source/Commands/CommandObjectThread.cpp
===================================================================
--- lldb/source/Commands/CommandObjectThread.cpp
+++ lldb/source/Commands/CommandObjectThread.cpp
@@ -482,9 +482,20 @@
     // Check if we are in Non-Stop mode
     TargetSP target_sp =
         execution_context ? execution_context->GetTargetSP() : TargetSP();
-    if (target_sp && target_sp->GetNonStopModeEnabled())
+    if (target_sp && target_sp->GetNonStopModeEnabled()) {
+      // NonStopMode runs all threads down in the ProcessPlugin layer, but
+      // at this level we need to pretend we are actually only running this
+      // thread.  So functionally it does the same thing as 
+      // GetSteppingRunsAllThreads.  So it overrides the runs all threads
+      // setting.
       m_run_mode = eOnlyThisThread;
-
+    } else {      
+      ProcessSP process_sp = 
+          execution_context ? execution_context->GetProcessSP() : ProcessSP();
+      if (process_sp && process_sp->GetSteppingRunsAllThreads())
+        m_run_mode = eAllThreads;
+    }
+    
     m_avoid_regexp.clear();
     m_step_in_target.clear();
     m_step_count = 1;
@@ -612,8 +623,7 @@
     if (m_options.m_run_mode == eAllThreads)
       bool_stop_other_threads = false;
     else if (m_options.m_run_mode == eOnlyDuringStepping)
-      bool_stop_other_threads =
-          (m_step_type != eStepTypeOut && m_step_type != eStepTypeScripted);
+      bool_stop_other_threads = (m_step_type != eStepTypeOut);
     else
       bool_stop_other_threads = true;
 
Index: lldb/source/API/SBThreadPlan.cpp
===================================================================
--- lldb/source/API/SBThreadPlan.cpp
+++ lldb/source/API/SBThreadPlan.cpp
@@ -196,6 +196,23 @@
   return false;
 }
 
+bool SBThreadPlan::GetStopOthers() {
+  LLDB_RECORD_METHOD_NO_ARGS(bool, SBThreadPlan, GetStopOthers);
+
+  ThreadPlanSP thread_plan_sp(GetSP());
+  if (thread_plan_sp)
+    return thread_plan_sp->StopOthers();
+  return false;
+}  
+
+void SBThreadPlan::SetStopOthers(bool stop_others) {
+  LLDB_RECORD_METHOD(void, SBThreadPlan, SetStopOthers, (bool), stop_others);
+
+  ThreadPlanSP thread_plan_sp(GetSP());
+  if (thread_plan_sp)
+    thread_plan_sp->SetStopOthers(stop_others);
+}
+
 // This section allows an SBThreadPlan to push another of the common types of
 // plans...
 //
@@ -463,6 +480,8 @@
   LLDB_REGISTER_METHOD(bool, SBThreadPlan, IsPlanComplete, ());
   LLDB_REGISTER_METHOD(bool, SBThreadPlan, IsPlanStale, ());
   LLDB_REGISTER_METHOD(bool, SBThreadPlan, IsValid, ());
+  LLDB_REGISTER_METHOD(void, SBThreadPlan, SetStopOthers, (bool));
+  LLDB_REGISTER_METHOD(bool, SBThreadPlan, GetStopOthers, ());
   LLDB_REGISTER_METHOD(lldb::SBThreadPlan, SBThreadPlan,
                        QueueThreadPlanForStepOverRange,
                        (lldb::SBAddress &, lldb::addr_t));
Index: lldb/include/lldb/Target/ThreadPlanPython.h
===================================================================
--- lldb/include/lldb/Target/ThreadPlanPython.h
+++ lldb/include/lldb/Target/ThreadPlanPython.h
@@ -45,7 +45,13 @@
 
   bool WillStop() override;
 
-  bool StopOthers() override;
+  bool StopOthers() override {
+    return m_stop_others;
+  }
+  
+  void SetStopOthers(bool new_value) {
+    m_stop_others = new_value;
+  }
 
   void DidPush() override;
 
@@ -67,6 +73,7 @@
   std::string m_error_str;
   StructuredData::ObjectSP m_implementation_sp;
   bool m_did_push;
+  bool m_stop_others;
 
   ThreadPlanPython(const ThreadPlanPython &) = delete;
   const ThreadPlanPython &operator=(const ThreadPlanPython &) = delete;
Index: lldb/include/lldb/Target/Process.h
===================================================================
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -91,6 +91,7 @@
   std::chrono::seconds GetUtilityExpressionTimeout() const;
   bool GetOSPluginReportsAllThreads() const;
   void SetOSPluginReportsAllThreads(bool does_report);
+  bool GetSteppingRunsAllThreads() const;
 
 protected:
   Process *m_process; // Can be nullptr for global ProcessProperties
Index: lldb/include/lldb/API/SBThreadPlan.h
===================================================================
--- lldb/include/lldb/API/SBThreadPlan.h
+++ lldb/include/lldb/API/SBThreadPlan.h
@@ -76,6 +76,10 @@
   bool IsPlanStale();
 
   bool IsValid();
+  
+  bool GetStopOthers();
+  
+  void SetStopOthers(bool stop_others);
 
   // This section allows an SBThreadPlan to push another of the common types of
   // plans...
Index: lldb/bindings/interface/SBThreadPlan.i
===================================================================
--- lldb/bindings/interface/SBThreadPlan.i
+++ lldb/bindings/interface/SBThreadPlan.i
@@ -92,6 +92,14 @@
     bool
     IsPlanStale();
 
+    %feature("docstring", "Return whether this plan will ask to stop other threads when it runs.") GetStopOthers;
+    bool
+    GetStopOthers();
+  
+    %feature("docstring", "Set whether this plan will ask to stop other threads when it runs.")	GetStopOthers;
+    void
+    SetStopOthers(bool stop_others);
+
     SBThreadPlan
     QueueThreadPlanForStepOverRange (SBAddress &start_address,
                                      lldb::addr_t range_size);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to