Allow scripted thread plans to modify the stop reason shown when the plan completes

[jingham]

Summary

We were just printing some fairly ugly boiler plate, for instance:

(lldb) thread step-scripted -C scripted_step.SimpleStep
Process 15467 stopped

• thread #1, queue = 'com.apple.main-thread', stop reason = Python thread plan implemented by class scripted_step.SimpleStep.

This change allows the scripted ThreadPlan to implement:

def stop_description(self, stream):
    stream.Print("Simple step completed")

and then you will see:

(lldb) thread step-scripted -C scripted_step.SimpleStep
Process 15226 stopped

• thread #1, queue = 'com.apple.main-thread', stop reason = Simple step completed

[mib]

This looks good to me, although I'm wondering whether instead of passing a string, we should pass a dictionary. This way the user could either fill the dictionary with a simple stop reason description or the MachException data What do you think Jim ?

lldb/bindings/python/python-wrapper.swig

390

If we passed a Status& instead of a bool& we would be able to get that error message in lldb.

lldb/examples/python/scripted_step.py

156

This is probably a typo

lldb/include/lldb/Interpreter/ScriptInterpreter.h

320

For Scripted Process, I pass a Status& argument which can hold an error message for better debugging. By doing so, you can also return Status.Success()

[jingham]

Address review comments.

[jingham]

lldb/bindings/python/python-wrapper.swig

390

GetDescription doesn't end up returning a Status, just a bool saying whether anything got added. So even if I added a Status here, there would be no place for it to go.

lldb/include/lldb/Interpreter/ScriptInterpreter.h

320

There isn't any place to return an error along the GetStopDescription path above us. That API just returns a bool saying whether you added information to the stream. So returning just a bool at this layer mirrors its use better.

[jingham]

In D149692#4313715, @mib wrote:

This looks good to me, although I'm wondering whether instead of passing a string, we should pass a dictionary. This way the user could either fill the dictionary with a simple stop reason description or the MachException data What do you think Jim ?

ThreadPlan stop descriptions are only for display purposes, we don't ever reason based on them. We do put data in StopInfo's but this isn't constructing a StopInfo, it's just adding the string to StopInfoPlanComplete. So I can't see what use would be made of the more complex return.

You are probably thinking of providing scripted StopInfo subclasses that do have structured data. That will be better implemented if we add:

SBStructuredData SBThread::GetStructuredStopInfo()

and then breakpoint stops, mach exception stops, etc. can be more reasonably presented, and ScriptedThreads will have a much easier time producing their StopInfo as well.

But even in that case, the providers of StopInfo's will also have to provide a StopDescription as a string, since lldb wouldn't know how to summarize the elements in the StopInfo dictionary.

So this bit of work is orthogonal to providing structured stop infos, IMO.

[mib]

Alright

[jingham]

Committed as: c2be702104284cb3facf31124494b9a400296179.

I forgot to put the differential revision cookie in the commit message...

[delcypher]

lldb/examples/python/scripted_step.py

130

@jingham Thanks for implementing this. Is there a reason why the interface needs to act on a stream object? Wouldn't this be simpler

def stop_description(self) -> str:
   return "Simple step completed"

lldb/bindings/python/python-wrapper.swig

else if (result.get() == Py_False)
     return false;
 
   // Somebody returned the wrong thing...
   got_error = true;
   printf("Wrong return value type for call to %s.\n", method_name);
   return false;
 }
 
+bool lldb_private::LLDBSWIGPythonCallThreadPlan(
+    void *implementor, const char *method_name, lldb_private::Stream *stream,
+    bool &got_error) {
+  got_error = false;
+
+  PyErr_Cleaner py_err_cleaner(false);
+  PythonObject self(PyRefType::Borrowed, static_cast<PyObject *>(implementor));
+  auto pfunc = self.ResolveName<PythonCallable>(method_name);
+
+  if (!pfunc.IsAllocated()) 
+    return false;
+
+  auto *sb_stream = new lldb::SBStream();
+  PythonObject sb_stream_arg =
+      ToSWIGWrapper(std::unique_ptr<lldb::SBStream>(sb_stream));
+
+  PythonObject result;
+  result = pfunc(sb_stream_arg);
+
+  if (PyErr_Occurred()) {
+    printf("Error occured for call to %s.\n",
+           method_name);
+    PyErr_Print();
+    got_error = true;
+    return false;
+  }
+  if (stream)
+    stream->PutCString(sb_stream->GetData());
+  return true;
+
+}
+
 PythonObject lldb_private::LLDBSwigPythonCreateScriptedBreakpointResolver(
     const char *python_class_name, const char *session_dictionary_name,
     const StructuredDataImpl &args_impl,
     const lldb::BreakpointSP &breakpoint_sp) {
 
   if (python_class_name == NULL || python_class_name[0] == '\0' ||
       !session_dictionary_name)
     return PythonObject();

lldb/examples/python/scripted_step.py

 #    happen here is that after continuing but before the finish is done, the program
 #    could hit another breakpoint and stop.  Then the user could use the step
 #    command repeatedly until they leave the frame of interest by stepping.
 #    In that case, the step plan is the one that will be responsible for stopping,
 #    and the finish plan won't be asked should_stop, it will just be asked if it
 #    is stale.  In this case, if the step_out plan that the FinishPrintAndContinue
 #    plan is driving is stale, so is ours, and it is time to do our printing.
 #
+# 4) If you implement the "stop_description(SBStream stream)" method in your
+#    python class, then that will show up as the "plan completed" reason when
+#    your thread plan is complete.
+#
 # Both examples show stepping through an address range for 20 bytes from the
 # current PC.  The first one does it by single stepping and checking a condition.
 # It doesn't, however handle the case where you step into another frame while
 # still in the current range in the starting frame.
 #
 # That is better handled in the second example by using the built-in StepOverRange
 # thread plan.
 #
def should_stop(self, event):
             self.thread_plan.SetPlanComplete(True)
             return True
         else:
             return False
 
     def should_step(self):
         return True
 
+    def stop_description(self, stream):
+        stream.Print("Simple step completed")
 
 class StepWithPlan:
 
     def __init__(self, thread_plan, dict):
         self.thread_plan = thread_plan
         self.start_address = thread_plan.GetThread().GetFrameAtIndex(0).GetPCAddress()
         self.step_thread_plan = thread_plan.QueueThreadPlanForStepOverRange(
             self.start_address, 20)
 
     def explains_stop(self, event):
         # Since all I'm doing is running a plan, I will only ever get askedthis
         # if myplan doesn't explain the stop, and in that caseI don'teither.
         return False
 
     def should_stop(self, event):
         if self.step_thread_plan.IsPlanComplete():
             self.thread_plan.SetPlanComplete(True)
             return True
         else:
             return False
 
     def should_step(self):
         return False
 
+    def stop_description(self, stream):
+        self.step_thread_plan.GetDescription(stream, lldb.eDescriptionLevelBrief)
+
 # Here's another example which does "step over" through the current function,
 # and when it stops at each line, it checks some condition (in this example the
 # value of a variable) and stops if that condition is true.
 
 
 class StepCheckingCondition:
 
     def __init__(self, thread_plan, dict):
def should_stop(self, event):
             return True
         else:
             self.queue_next_plan()
             return False
 
     def should_step(self):
         return True
 
+    def stop_description(self, stream):
+        stream.Print(f"Stepped until a == 20")
+
 # Here's an example that steps out of the current frame, gathers some information
 # and then continues.  The information in this case is rax.  Currently the thread
 # plans are not a safe place to call lldb command-line commands, so the information
 # is gathered through SB API calls.
 
 
 class FinishPrintAndContinue:
 
class FinishPrintAndContinue:
 
     def do_print(self):
         frame_0 = self.thread.frames[0]
         rax_value = frame_0.FindRegister("rax")
         if rax_value.GetError().Success():
             print("RAX on exit: ", rax_value.GetValue())
         else:
             print("Couldn't get rax value:", rax_value.GetError().GetCString())
+
+    def stop_description(self, stream):
+        self.step_out_thread_plan.GetDescription(stream, lldb.eDescriptionLevelBrief)

lldb/include/lldb/Interpreter/ScriptInterpreter.h

public:
 
   virtual lldb::StateType
   ScriptedThreadPlanGetRunState(StructuredData::ObjectSP implementor_sp,
                                 bool &script_error) {
     script_error = true;
     return lldb::eStateStepping;
   }
 
+  virtual bool
+  ScriptedThreadPlanGetStopDescription(StructuredData::ObjectSP implementor_sp,
+                                       lldb_private::Stream *stream,
+                                       bool &script_error) {
+    script_error = true;
+    return false;
+  }
+
   virtual StructuredData::GenericSP
   CreateScriptedBreakpointResolver(const char *class_name,
                                    const StructuredDataImpl &args_data,
                                    lldb::BreakpointSP &bkpt_sp) {
     return StructuredData::GenericSP();
   }
 
   virtual bool

lldb/include/lldb/Target/ThreadPlanPython.h

public:
 
   bool StopOthers() override { return m_stop_others; }
 
   void SetStopOthers(bool new_value) override { m_stop_others = new_value; }
 
   void DidPush() override;
 
   bool IsPlanStale() override;
   
+  bool DoWillResume(lldb::StateType resume_state, bool current_plan) override;
+
+
 protected:
   bool DoPlanExplainsStop(Event *event_ptr) override;
 
   lldb::StateType GetPlanRunState() override;
   
   ScriptInterpreter *GetScriptInterpreter();
 
 private:
   std::string m_class_name;
   StructuredDataImpl m_args_data;
   std::string m_error_str;
   StructuredData::ObjectSP m_implementation_sp;
+  StreamString m_stop_description; // Cache the stop description here
   bool m_did_push;
   bool m_stop_others;
 
   ThreadPlanPython(const ThreadPlanPython &) = delete;
   const ThreadPlanPython &operator=(const ThreadPlanPython &) = delete;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_TARGET_THREADPLANPYTHON_H

lldb/source/Plugins/ScriptInterpreter/Python/SWIGPythonBridge.h

 python::PythonObject LLDBSwigPythonCreateScriptedThreadPlan(
     const char *python_class_name, const char *session_dictionary_name,
     const StructuredDataImpl &args_data, std::string &error_string,
     const lldb::ThreadPlanSP &thread_plan_sp);
 
 bool LLDBSWIGPythonCallThreadPlan(void *implementor, const char *method_name,
                                   lldb_private::Event *event_sp,
                                   bool &got_error);
                                   
+bool LLDBSWIGPythonCallThreadPlan(void *implementor, 
+                                  const char *method_name,
+                                  lldb_private::Stream *stream,
+                                  bool &got_error);
+
 python::PythonObject LLDBSwigPythonCreateScriptedBreakpointResolver(
     const char *python_class_name, const char *session_dictionary_name,
     const StructuredDataImpl &args, const lldb::BreakpointSP &bkpt_sp);
 
 unsigned int
 LLDBSwigPythonCallBreakpointResolver(void *implementor, const char *method_name,
                                      lldb_private::SymbolContext *sym_ctx);
 

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp

bool ScriptInterpreterPythonImpl::ScriptedThreadPlanIsStale(
   bool is_stale = true;
   StructuredData::Generic *generic = nullptr;
   if (implementor_sp)
     generic = implementor_sp->GetAsGeneric();
   if (generic) {
     Locker py_lock(this,
                    Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
     is_stale = LLDBSWIGPythonCallThreadPlan(generic->GetValue(), "is_stale",
-                                            nullptr, script_error);
+                                            (Event *) nullptr, script_error);
     if (script_error)
       return true;
   }
   return is_stale;
 }
 
 lldb::StateType ScriptInterpreterPythonImpl::ScriptedThreadPlanGetRunState(
     StructuredData::ObjectSP implementor_sp, bool &script_error) {
   bool should_step = false;
   StructuredData::Generic *generic = nullptr;
   if (implementor_sp)
     generic = implementor_sp->GetAsGeneric();
   if (generic) {
     Locker py_lock(this,
                    Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
     should_step = LLDBSWIGPythonCallThreadPlan(
-        generic->GetValue(), "should_step", nullptr, script_error);
+        generic->GetValue(), "should_step", (Event *) nullptr, script_error);
     if (script_error)
       should_step = true;
   }
   if (should_step)
     return lldb::eStateStepping;
   return lldb::eStateRunning;
 }
 
+bool
+ScriptInterpreterPythonImpl::ScriptedThreadPlanGetStopDescription(
+    StructuredData::ObjectSP implementor_sp, lldb_private::Stream *stream, 
+    bool &script_error) {
+  StructuredData::Generic *generic = nullptr;
+  if (implementor_sp)
+    generic = implementor_sp->GetAsGeneric();
+  if (!generic) {
+    script_error = true;
+    return false;
+  }
+  Locker py_lock(this,
+                   Locker::AcquireLock | Locker::InitSession | Locker::NoSTDIN);
+  return LLDBSWIGPythonCallThreadPlan(generic->GetValue(), "stop_description", 
+      stream, script_error);
+}
+
+
 StructuredData::GenericSP
 ScriptInterpreterPythonImpl::CreateScriptedBreakpointResolver(
     const char *class_name, const StructuredDataImpl &args_data,
     lldb::BreakpointSP &bkpt_sp) {
 
   if (class_name == nullptr || class_name[0] == '\0')
     return StructuredData::GenericSP();
 

lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPythonImpl.h

public:
 
   bool ScriptedThreadPlanIsStale(StructuredData::ObjectSP implementor_sp,
                                  bool &script_error) override;
 
   lldb::StateType
   ScriptedThreadPlanGetRunState(StructuredData::ObjectSP implementor_sp,
                                 bool &script_error) override;
 
+  bool
+  ScriptedThreadPlanGetStopDescription(StructuredData::ObjectSP implementor_sp,
+                                lldb_private::Stream *s,
+                                bool &script_error) override;
+
   StructuredData::GenericSP
   CreateScriptedBreakpointResolver(const char *class_name,
                                    const StructuredDataImpl &args_data,
                                    lldb::BreakpointSP &bkpt_sp) override;
   bool ScriptedBreakpointResolverSearchCallback(
       StructuredData::GenericSP implementor_sp,
       SymbolContext *sym_ctx) override;
 

lldb/source/Target/ThreadPlanPython.cpp

bool ThreadPlanPython::MischiefManaged() {
   Log *log = GetLog(LLDBLog::Thread);
   LLDB_LOGF(log, "%s called on Python Thread Plan: %s )", LLVM_PRETTY_FUNCTION,
             m_class_name.c_str());
   bool mischief_managed = true;
   if (m_implementation_sp) {
     // I don't really need mischief_managed, since it's simpler to just call
     // SetPlanComplete in should_stop.
     mischief_managed = IsPlanComplete();
-    if (mischief_managed)
+    if (mischief_managed) {
+      // We need to cache the stop reason here we'll need it in GetDescription.
+      GetDescription(&m_stop_description, eDescriptionLevelBrief);
       m_implementation_sp.reset();
     }
+  }
   return mischief_managed;
 }
 
 lldb::StateType ThreadPlanPython::GetPlanRunState() {
   Log *log = GetLog(LLDBLog::Thread);
   LLDB_LOGF(log, "%s called on Python Thread Plan: %s )", LLVM_PRETTY_FUNCTION,
             m_class_name.c_str());
   lldb::StateType run_state = eStateRunning;
   if (m_implementation_sp) {
     ScriptInterpreter *script_interp = GetScriptInterpreter();
     if (script_interp) {
       bool script_error;
       run_state = script_interp->ScriptedThreadPlanGetRunState(
           m_implementation_sp, script_error);
     }
   }
   return run_state;
 }
 
-// The ones below are not currently exported to Python.
 void ThreadPlanPython::GetDescription(Stream *s, lldb::DescriptionLevel level) {
+  Log *log = GetLog(LLDBLog::Thread);
+  LLDB_LOGF(log, "%s called on Python Thread Plan: %s )", LLVM_PRETTY_FUNCTION,
+            m_class_name.c_str());
+  if (m_implementation_sp) {
+    ScriptInterpreter *script_interp = GetScriptInterpreter();
+    if (script_interp) {
+      bool script_error;
+      bool added_desc = script_interp->ScriptedThreadPlanGetStopDescription(
+          m_implementation_sp, s, script_error);
+      if (script_error || !added_desc)
         s->Printf("Python thread plan implemented by class %s.",
             m_class_name.c_str());
     }
+    return;
+  }
+  // It's an error not to have a description, so if we get here, we should
+  // add something.
+  if (m_stop_description.Empty())
+    s->Printf("Python thread plan implemented by class %s.",
+              m_class_name.c_str());
+  s->PutCString(m_stop_description.GetData());
+}
 
+// The ones below are not currently exported to Python.
 bool ThreadPlanPython::WillStop() {
   Log *log = GetLog(LLDBLog::Thread);
   LLDB_LOGF(log, "%s called on Python Thread Plan: %s )", LLVM_PRETTY_FUNCTION,
             m_class_name.c_str());
   return true;
 }
+
+bool ThreadPlanPython::DoWillResume(lldb::StateType resume_state, 
+                                  bool current_plan) {
+  m_stop_description.Clear();
+  return true;                                  
+}

lldb/test/API/functionalities/step_scripted/Steps.py

def should_stop(self, event):
 
         self.thread_plan.SetPlanComplete(True)
 
         return True
 
     def should_step(self):
         return False
 
+    def stop_description(self, stream):
+        if self.child_thread_plan.IsPlanComplete():
+            return self.child_thread_plan.GetDescription(stream)
+        return True
+
     def queue_child_thread_plan(self):
         return None
 
 class StepOut(StepWithChild):
     def __init__(self, thread_plan, dict):
         StepWithChild.__init__(self, thread_plan)
 
     def queue_child_thread_plan(self):
         return self.thread_plan.QueueThreadPlanForStepOut(0)
 
 class StepScripted(StepWithChild):
     def __init__(self, thread_plan, dict):
         StepWithChild.__init__(self, thread_plan)
 
     def queue_child_thread_plan(self):
         return self.thread_plan.QueueThreadPlanForStepScripted("Steps.StepOut")
 
 # This plan does a step-over until a variable changes value.
 class StepUntil(StepWithChild):
     def __init__(self, thread_plan, args_data, dict):
+        self.thread_plan = thread_plan
         self.frame = thread_plan.GetThread().frames[0]
         self.target = thread_plan.GetThread().GetProcess().GetTarget()
-        func_entry = args_data.GetValueForKey("variable_name")
+        var_entry = args_data.GetValueForKey("variable_name")
         
-        if not func_entry.IsValid():
+        if not var_entry.IsValid():
             print("Did not get a valid entry for variable_name")
-        func_name = func_entry.GetStringValue(100)
+        self.var_name = var_entry.GetStringValue(100)
 
-        self.value = self.frame.FindVariable(func_name)
+        self.value = self.frame.FindVariable(self.var_name)
         if self.value.GetError().Fail():
             print("Failed to get foo value: %s"%(self.value.GetError().GetCString()))
-        else:
-            print("'foo' value: %d"%(self.value.GetValueAsUnsigned()))
 
         StepWithChild.__init__(self, thread_plan)
 
 
     def queue_child_thread_plan(self):
         le = self.frame.GetLineEntry()
         start_addr = le.GetStartAddress() 
         start = start_addr.GetLoadAddress(self.target)
         end = le.GetEndAddress().GetLoadAddress(self.target)
         return self.thread_plan.QueueThreadPlanForStepOverRange(start_addr,
                                                                 end - start)
 
     def should_stop(self, event):
         if not self.child_thread_plan.IsPlanComplete():
             return False
 
         # If we've stepped out of this frame, stop.
         if not self.frame.IsValid():
+            self.thread_plan.SetPlanComplete(True)
             return True
 
         if not self.value.IsValid():
+            self.thread_plan.SetPlanComplete(True)
             return True
 
         if not self.value.GetValueDidChange():
             self.child_thread_plan = self.queue_child_thread_plan()
             return False
         else:
+            self.thread_plan.SetPlanComplete(True)
             return True
 
+    def stop_description(self, stream):
+        stream.Print(f"Stepped until {self.var_name} changed.")
+        
 # 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

lldb/test/API/functionalities/step_scripted/TestStepScripted.py

def step_out_with_scripted_plan(self, name):
         frame = thread.GetFrameAtIndex(0)
         self.assertEqual("foo", frame.GetFunctionName())
 
         err = thread.StepUsingScriptedThreadPlan(name)
         self.assertSuccess(err)
 
         frame = thread.GetFrameAtIndex(0)
         self.assertEqual("main", frame.GetFunctionName())
-
+        stop_desc = thread.GetStopDescription(1000)
+        self.assertIn("Stepping out from", stop_desc, "Got right description")
 
     def test_misspelled_plan_name(self):
         """Test that we get a useful error if we misspell the plan class name"""
         self.build()
         (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
                                                                             "Set a breakpoint here",
                                                                             self.main_source_file)
         stop_id = process.GetStopID()
def do_test_checking_variable(self, use_cli):
         self.assertState(process.GetState(), lldb.eStateStopped, "We are stopped")
 
         # We should still be in foo:
         self.assertEqual("foo", frame.GetFunctionName())
 
         # And foo should have changed:
         self.assertTrue(foo_val.GetValueDidChange(), "Foo changed")
 
+        # And we should have a reasonable stop description:
+        desc = thread.GetStopDescription(1000)
+        self.assertIn("Stepped until foo changed", desc, "Got right stop description")
+
     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)
+        if self.TraceOn():
             print(cmd)
         interp.HandleCommand(cmd, result)
         self.assertTrue(result.Succeeded(), "Step scripted failed: %s."%(result.GetError()))
+        if self.TraceOn():
             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)

lldb/unittests/ScriptInterpreter/Python/PythonTestSuite.cpp

 
 bool lldb_private::LLDBSWIGPythonCallThreadPlan(void *implementor,
                                                 const char *method_name,
                                                 Event *event_sp,
                                                 bool &got_error) {
   return false;
 }
 
+bool 
+lldb_private::LLDBSWIGPythonCallThreadPlan(void *implementor,
+                                                const char *method_name,
+                                                Stream *event_sp,
+                                                bool &got_error) {
+  return false;
+}
+
 python::PythonObject
 lldb_private::LLDBSwigPythonCreateScriptedBreakpointResolver(
     const char *python_class_name, const char *session_dictionary_name,
     const StructuredDataImpl &args, const lldb::BreakpointSP &bkpt_sp) {
   return python::PythonObject();
 }
 
 unsigned int lldb_private::LLDBSwigPythonCallBreakpointResolver(