Improve SBThread's stepping API using SBError parameter.

[apolyakov]

Summary

The new methods will allow to get error messages from stepping API.

[apolyakov]

Also, I suggest to make similar things in other SBThread methods like StepOut etc.

[aprantl]

This looks reasonable to me, but I'd like to also hear from Greg and Jim since SBAPI changes are kind of final.

include/lldb/API/SBThread.h

96 ↗

(On Diff #150650)

Where is the SBAPI documentation that is displayed on http://lldb.llvm.org ? Apparently no in this file, but it must exist *somewhere*, can you update it too, please?

[clayborg]

This is fine. Make sure all other steps have error overloads.

[clayborg]

It is unfortunate that we can't overload with the return value and return an SBError...

[apolyakov]

include/lldb/API/SBThread.h

96 ↗

(On Diff #150650)

Due to http://lldb.llvm.org/: This documentation is generated directly from the source code with doxygen.. So I think that we don't need to change anything except source files.

[aprantl]

Hmm.. it looks like most C++ API calls don't have any documentation.

http://lldb.llvm.org/cpp_reference/html/classlldb_1_1SBThread.html#a42755a170e127881a5dd65162217f68b

It does look like the python API has more documentation.. where does that come from?
http://lldb.llvm.org/python_reference/index.html
http://lldb.llvm.org/python_reference/lldb.SBThread-class.html#StepInto

[apolyakov]

I found it out. Info about each method is located in header file. For example, you may look at include/lldb/API/SBThread.h and find text from docs.

[labath]

In D47991#1128433, @aprantl wrote:

Hmm.. it looks like most C++ API calls don't have any documentation.

http://lldb.llvm.org/cpp_reference/html/classlldb_1_1SBThread.html#a42755a170e127881a5dd65162217f68b

It does look like the python API has more documentation.. where does that come from?

scripts/interface/SBThread.i

[aprantl]

It would be good to add documentation for the new API call there, then.

[apolyakov]

In D47991#1128477, @labath wrote:

In D47991#1128433, @aprantl wrote:

Hmm.. it looks like most C++ API calls don't have any documentation.

http://lldb.llvm.org/cpp_reference/html/classlldb_1_1SBThread.html#a42755a170e127881a5dd65162217f68b

It does look like the python API has more documentation.. where does that come from?

scripts/interface/SBThread.i

Do you know what is the difference between autodoc and docstring there?

[aprantl]

No, but I found this: http://www.swig.org/Doc1.3/Python.html#Python_nn67

[jingham]

Sorry for coming in late (WWDC...) It does seem asymmetric to only have one stepping API take an error. If one is going to they all should.

You need to add the new API to the SBThread.i file as well or it won't get exported to Python.

Beyond that, the SBError parameter is the last parameter in pretty much all the other SB API's. We do inputs first and outputs second as a general rule. So if you are going to add one here it would be better to make it the last parameters.

BTW, the web docs are auto-generated but not live. I don't remember when somebody last refreshed them (and actually I didn't follow in detail how this was done.) But we should kick that as an additional step.

[lemo]

include/lldb/API/SBThread.h

96 ↗

(On Diff #150650)

While not perfectly consistent across SB API, most methods having an out SBError parameter place it last.

96 ↗

(On Diff #150650)

I believe you need to update SBThread.i as well. It is required for Python binding through SWIG and it also defines the Python API documentation.

[apolyakov]

Documented changes and overloaded all stepping functions.

[aprantl]

Have you looked into making the error the first vs the last argument? If the majority of all SBAPI calls put the error last, we should do this here, too.

[apolyakov]

In D47991#1128831, @aprantl wrote:

Have you looked into making the error the first vs the last argument? If the majority of all SBAPI calls put the error last, we should do this here, too.

As you could see, SBError is on first place only in one function - StepOver. When I replaced it to last place, I got an error:

/home/alexander/workspace/gsoc/llvm/tools/lldb/scripts/./interface/SBThread.i:218: Error: Non-optional argument 'error' follows an optional argument.

so I returned it back.

[apolyakov]

By the way, we can do something like:

void StepOver() {
  StepOver(lldb:eOnlyDuringStepping);
}

void StepOver(lldb::RunMode stop_other_threads) {
  SBError error;
  StepOver(stop_other_threads, error);
}

and so on.

[aprantl]

Ah I see. That's because the last argument is a C++ default argument. It looks like the convention in this file is that the error argument should be the last non-defaulted argument.

[lemo]

Ah I see. That's because the last argument is a C++ default argument. It
looks like the convention in this file is that the error argument should be
the last non-defaulted argument.

I think it should be:

void StepOver(lldb::RunMode stop_other_threads, lldb::SBError &error); //
no default argument!

Ie. if you want the overload with error you need to pass RunMode
explicitly. Keeping the overload set manageable is a good practice in
general, not just because of SWIG (default arguments + overloaded arguments
can easily get out of hand)

[apolyakov]

Some readability improvements(more comments). I think that we should keep StepOver signature as

void StepOver(SBError &error, lldb::RunMode stop_other_threads);

since it seems that in SBThread methods SBError paramater is the last non-optional one, as @aprantl said.

[aprantl]

This comment has been deleted.

scripts/interface/SBThread.i

257 ↗

(On Diff #150913)

a -> an

source/API/SBThread.cpp

703 ↗

(On Diff #150913)

Please always prefer early exits (https://www.llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code)

if (!exe_ctx.HasThreadScope())
  return error.SetErrorString("SBThread object is invalid");

752 ↗

(On Diff #150913)

same here

1148 ↗

(On Diff #150913)

and here.

[apolyakov]

Added early exits.

[apolyakov]

source/API/SBThread.cpp

1136 ↗

(On Diff #150983)

@aprantl do you think that we should use early exit here? There is printing to log at the end of the function, so in case of early exit, we must duplicate log->Printf(). Is it worth it?

[jingham]

I agree with Leonard, for the StepOver overload that returns errors, just make the mode parameter required. That will reduce confusion.

[aprantl]

I agree with Leonard, for the StepOver overload that returns errors, just make the mode parameter required. That will reduce confusion.

Works for me, too.

source/API/SBThread.cpp

1136 ↗

(On Diff #150983)

Duplicating the log message is probably not worth it.

1141 ↗

(On Diff #150983)

It looks like the error string should be set here as well?

1146 ↗

(On Diff #150983)

I think clang-format would do this as:

} else
   error.SetErrorString("this SBThread object is invalid");

1171 ↗

(On Diff #150983)

same here.

1179 ↗

(On Diff #150983)

Looking at the implementation of SBThread::Resume and SBThread::Suspend, the only difference seems to be the API called and the log message. If there is a third command with a similar implementation, it might be a fun exercise to factor that out into a helper function that takes std::function<> and a log string argument...

[apolyakov]

Added StepOver overload without optional parameters.

[apolyakov]

source/API/SBThread.cpp

816 ↗

(On Diff #150996)

I found this place and added return.

[apolyakov]

Minor update: moved error.SetErrorString to the top of if statement to increase readability.

[clayborg]

Looks good. Just a question about including the commented out default arguments

source/API/SBThread.cpp

636 ↗

(On Diff #151000)

might avoid showing the default argument in case it changes and we don't do this anywhere else that I know of.

678 ↗

(On Diff #151000)

Ditto

684 ↗

(On Diff #151000)

Ditto

691 ↗

(On Diff #151000)

Ditto

[apolyakov]

In D47991#1131043, @clayborg wrote:

Looks good. Just a question about including the commented out default arguments

Don't you think it increases readability of this code?

[clayborg]

In D47991#1131181, @apolyakov wrote:

In D47991#1131043, @clayborg wrote:

Looks good. Just a question about including the commented out default arguments

Don't you think it increases readability of this code?

It does if it stays in sync. This isn't done anywhere else and I don't believe it is in the LLVM coding conventions. If it is, please correct me. I didn't mark this as Request Changes as I wanted to gauge what others think. I am happy to go either way, but if we do go this way, it will have impact on all patches going forward so I wanted to point it out for discussion.

[apolyakov]

I removed comments about optional arguments since I didn't find any info about it in LLVM coding style.

[apolyakov]

Just reminder that it still needs a review. Thanks in advance.

[aprantl]

This patch is adding new overloads to SBAPI calls that don't return an SBError, such as:

// Old:
void StepOutOfFrame(SBFrame &frame);
// New:
void StepOutOfFrame(SBFrame &frame, SBError &error);

I wonder if it would be easier to use and more consistent with the rest of the API if we instead added an overload that returns an SBError, like this:

// New:
SBError StepOutOfFrameWithError(SBFrame &frame);
// Alternative names that are just as ugly.
SBError StepOutOfFrameE(SBFrame &frame);
SBError StepOutOfFrame2(SBFrame &frame);

@clayborg, @jingham: What do you think?

[jingham]

The SB API's tend to take SBError as an in/out parameter for the cases where the function in question naturally returns some other value, and return an SBError when there's no other logical return value.

So in these cases it would be more in line with the general practice to return an SBError. OTOH, I really don't like the practice of adding "Ex" or other similar function name decorations to get around the fact that C++ doesn't overload on return values. IMO they make the API harder to remember. And especially since we really should use the error version, so you will end up with the primary function having this weird tag as part of the name...

In balance I think adding it as an argument is better, but this is one of those cases where I would leave it to the taste of the implementor...

Jim

[apolyakov]

I think we just can make old versions of API methods returning SBError in all cases. This way we'll deal with SBError and won't break down API calls.
For example:

// old
void StepOver() {
  ...
}

// new
SBError StepOver() {
  ...
  return sb_error;
}

[aprantl]

Okay.. then let's go with this version.

[jingham]

Won't this break client code that was calling StepOver? We are pretty serious about maintaining binary compatibility with the SB API's.

Jim

[aprantl]

In D47991#1138029, @jingham wrote:

Won't this break client code that was calling StepOver? We are pretty serious about maintaining binary compatibility with the SB API's.

Yeah, we can't replace existing function calls: The C++ name mangling could change, and the calling convention could change, too, depending on how big SBError is.

[apolyakov]

If some client uses StepOver like sbThread.StepOver(), then making StepOver return SBError won't break anything since in this case returned SBError value will just be ignored. Am I missing something?

[jingham]

The client won't be expecting a struct return, and will have generated code to take a void return. If SBError happens to be returned in registers, nothing bad will happen, but if it's returned on the stack, that would corrupt the caller's stack. Relying on the particular kind of struct return to keep out of trouble seems like a bad practice to me.

Jim

[apolyakov]

In D47991#1138071, @jingham wrote:

The client won't be expecting a struct return, and will have generated code to take a void return. If SBError happens to be returned in registers, nothing bad will happen, but if it's returned on the stack, that would corrupt the caller's stack. Relying on the particular kind of struct return to keep out of trouble seems like a bad practice to me.

Jim

Ok, then I suggest not to increase amount of methods in API doing the same stuff and keep SBError as in/out parameter.

[jingham]

That seems fine to me.

[clayborg]

In D47991#1137834, @aprantl wrote:

This patch is adding new overloads to SBAPI calls that don't return an SBError, such as:

// Old:
void StepOutOfFrame(SBFrame &frame);
// New:
void StepOutOfFrame(SBFrame &frame, SBError &error);

I wonder if it would be easier to use and more consistent with the rest of the API if we instead added an overload that returns an SBError, like this:

// New:
SBError StepOutOfFrameWithError(SBFrame &frame);
// Alternative names that are just as ugly.
SBError StepOutOfFrameE(SBFrame &frame);
SBError StepOutOfFrame2(SBFrame &frame);

@clayborg, @jingham: What do you think?

that is the right way to do it, but we can't overload on return type only. We will need the old version of the code to be in the API for compatibility. Overloading by return type will result is two symbols with the same name. If we change the API over to return an SBError, we will crash programs that linked against the old API as it now becomes returns something, possibly a struct return type, so it will crash older programs.

[aprantl]

In D47991#1139249, @clayborg wrote:

that is the right way to do it, but we can't overload on return type only. We will need the old version of the code to be in the API for compatibility. Overloading by return type will result is two symbols with the same name. If we change the API over to return an SBError, we will crash programs that linked against the old API as it now becomes returns something, possibly a struct return type, so it will crash older programs.

I get that. My question was whether it is better to add a new overload that adds an SBError& argument versus adding a new overload that returns and SBError *and* has a different name. In any case, Jim pointed out that we are using both variants in other places, so it doesn't really seem to matter.

lldb/trunk/include/lldb/API/SBThread.h

public:
   const char *GetQueueName() const;
 
   lldb::queue_id_t GetQueueID() const;
 
   bool GetInfoItemByPathAsString(const char *path, SBStream &strm);
 
   void StepOver(lldb::RunMode stop_other_threads = lldb::eOnlyDuringStepping);
 
+  void StepOver(lldb::RunMode stop_other_threads, SBError &error);
+
   void StepInto(lldb::RunMode stop_other_threads = lldb::eOnlyDuringStepping);
 
   void StepInto(const char *target_name,
                 lldb::RunMode stop_other_threads = lldb::eOnlyDuringStepping);
 
   void StepInto(const char *target_name, uint32_t end_line, SBError &error,
                 lldb::RunMode stop_other_threads = lldb::eOnlyDuringStepping);
 
   void StepOut();
 
-  void StepOutOfFrame(lldb::SBFrame &frame);
+  void StepOut(SBError &error);
+
+  void StepOutOfFrame(SBFrame &frame);
+
+  void StepOutOfFrame(SBFrame &frame, SBError &error);
 
   void StepInstruction(bool step_over);
 
+  void StepInstruction(bool step_over, SBError &error);
+
   SBError StepOverUntil(lldb::SBFrame &frame, lldb::SBFileSpec &file_spec,
                         uint32_t line);
 
   SBError StepUsingScriptedThreadPlan(const char *script_class_name);
 
   SBError StepUsingScriptedThreadPlan(const char *script_class_name,
                                       bool resume_immediately);
 
   SBError JumpToLine(lldb::SBFileSpec &file_spec, uint32_t line);
 
   void RunToAddress(lldb::addr_t addr);
 
+  void RunToAddress(lldb::addr_t addr, SBError &error);
+
   SBError ReturnFromFrame(SBFrame &frame, SBValue &return_value);
 
   SBError UnwindInnermostExpression();
 
   //--------------------------------------------------------------------------
   /// LLDB currently supports process centric debugging which means when any
   /// thread in a process stops, all other threads are stopped. The Suspend()
   /// call here tells our process to suspend a thread and not let it run when
public:
   /// when the process is continued.
   ///
   /// Suspend() and Resume() functions are not currently reference counted, if
   /// anyone has the need for them to be reference counted, please let us
   /// know.
   //--------------------------------------------------------------------------
   bool Suspend();
 
+  bool Suspend(SBError &error);
+
   bool Resume();
 
+  bool Resume(SBError &error);
+
   bool IsSuspended();
 
   bool IsStopped();
 
   uint32_t GetNumFrames();
 
   lldb::SBFrame GetFrameAtIndex(uint32_t idx);
 

lldb/trunk/scripts/interface/SBThread.i

~SBThread();
     ") GetQueue;
 
     lldb::SBQueue
     GetQueue () const;
 
     void
     StepOver (lldb::RunMode stop_other_threads = lldb::eOnlyDuringStepping);
 
+    %feature("autodoc",
+    "Do a source level single step over in the currently selected thread.") StepOver;
+    void
+    StepOver (lldb::RunMode stop_other_threads, SBError &error);
+
     void
     StepInto (lldb::RunMode stop_other_threads = lldb::eOnlyDuringStepping);
 
     void
     StepInto (const char *target_name, lldb::RunMode stop_other_threads = lldb::eOnlyDuringStepping);
 
     %feature("autodoc", "
-    Step  the current thread from the current source line to the line given by end_line, stopping if
+    Step the current thread from the current source line to the line given by end_line, stopping if
     the thread steps into the function given by target_name.  If target_name is None, then stepping will stop
     in any of the places we would normally stop.
     ") StepInto;
     void
     StepInto (const char *target_name,
               uint32_t end_line,
               SBError &error,
               lldb::RunMode stop_other_threads = lldb::eOnlyDuringStepping);
 
     void
     StepOut ();
 
+    %feature("autodoc",
+    "Step out of the currently selected thread.") StepOut;
+    void
+    StepOut (SBError &error);
+
     void
-    StepOutOfFrame (lldb::SBFrame &frame);
+    StepOutOfFrame (SBFrame &frame);
+
+    %feature("autodoc",
+    "Step out of the specified frame.") StepOutOfFrame;
+    void
+    StepOutOfFrame (SBFrame &frame, SBError &error);
 
     void
     StepInstruction(bool step_over);
 
+    %feature("autodoc",
+    "Do an instruction level single step in the currently selected thread.
+    ") StepInstruction;
+    void
+    StepInstruction(bool step_over, SBError &error);
+
     SBError
     StepOverUntil (lldb::SBFrame &frame,
                    lldb::SBFileSpec &file_spec,
                    uint32_t line);
 
     SBError
     StepUsingScriptedThreadPlan (const char *script_class_name);
 
     SBError
     StepUsingScriptedThreadPlan (const char *script_class_name, bool resume_immediately);
 
     SBError
     JumpToLine (lldb::SBFileSpec &file_spec, uint32_t line);
 
     void
     RunToAddress (lldb::addr_t addr);
 
+    void
+    RunToAddress (lldb::addr_t addr, SBError &error);
+
     %feature("autodoc", "
     Force a return from the frame passed in (and any frames younger than it)
     without executing any more code in those frames.  If return_value contains
     a valid SBValue, that will be set as the return value from frame.  Note, at
     present only scalar return values are supported.
     ") ReturnFromFrame;
     
     SBError
~SBThread();
     ///
     /// Suspend() and Resume() functions are not currently reference counted, if
     /// anyone has the need for them to be reference counted, please let us
     /// know.
     //--------------------------------------------------------------------------
     ") Suspend;
     bool
     Suspend();
-    
     bool
+    Suspend(SBError &error);
+    
+    bool
     Resume ();
-    
     bool
+    Resume (SBError &error);
+    
+    bool
     IsSuspended();
 
     bool
     IsStopped();
 
     uint32_t
     GetNumFrames ();
 

lldb/trunk/source/API/SBThread.cpp

if (process->GetTarget().GetDebugger().GetAsyncExecution())
     sb_error.ref() = process->Resume();
   else
     sb_error.ref() = process->ResumeSynchronous(NULL);
 
   return sb_error;
 }
 
 void SBThread::StepOver(lldb::RunMode stop_other_threads) {
+  SBError error; // Ignored
+  StepOver(stop_other_threads, error);
+}
+
+void SBThread::StepOver(lldb::RunMode stop_other_threads, SBError &error) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
 
   std::unique_lock<std::recursive_mutex> lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
 
   if (log)
     log->Printf("SBThread(%p)::StepOver (stop_other_threads='%s')",
                 static_cast<void *>(exe_ctx.GetThreadPtr()),
                 Thread::RunModeAsCString(stop_other_threads));
 
-  if (exe_ctx.HasThreadScope()) {
-    Thread *thread = exe_ctx.GetThreadPtr();
-    bool abort_other_plans = false;
-    StackFrameSP frame_sp(thread->GetStackFrameAtIndex(0));
+  if (!exe_ctx.HasThreadScope()) {
+    error.SetErrorString("this SBThread object is invalid");
+    return;
+  }
+
 
-    ThreadPlanSP new_plan_sp;
-    if (frame_sp) {
-      if (frame_sp->HasDebugInformation()) {
-        const LazyBool avoid_no_debug = eLazyBoolCalculate;
-        SymbolContext sc(frame_sp->GetSymbolContext(eSymbolContextEverything));
-        new_plan_sp = thread->QueueThreadPlanForStepOverRange(
-            abort_other_plans, sc.line_entry, sc, stop_other_threads,
-            avoid_no_debug);
-      } else {
-        new_plan_sp = thread->QueueThreadPlanForStepSingleInstruction(
-            true, abort_other_plans, stop_other_threads);
-      }
-    }
-
-    // This returns an error, we should use it!
-    ResumeNewPlan(exe_ctx, new_plan_sp.get());
-  }
+  error = ResumeNewPlan(exe_ctx, new_plan_sp.get());
 }
 
 void SBThread::StepInto(lldb::RunMode stop_other_threads) {
   StepInto(NULL, stop_other_threads);
 }
 
 void SBThread::StepInto(const char *target_name,
                         lldb::RunMode stop_other_threads) {
-  SBError error;
+  SBError error; // Ignored
   StepInto(target_name, LLDB_INVALID_LINE_NUMBER, error, stop_other_threads);
 }
 
 void SBThread::StepInto(const char *target_name, uint32_t end_line,
                         SBError &error, lldb::RunMode stop_other_threads) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
 
   std::unique_lock<std::recursive_mutex> lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
 
   if (log)
     log->Printf(
         "SBThread(%p)::StepInto (target_name='%s', stop_other_threads='%s')",
         static_cast<void *>(exe_ctx.GetThreadPtr()),
         target_name ? target_name : "<NULL>",
         Thread::RunModeAsCString(stop_other_threads));
 
-  if (exe_ctx.HasThreadScope()) {
-    bool abort_other_plans = false;
+  if (!exe_ctx.HasThreadScope()) {
+    error.SetErrorString("this SBThread object is invalid");
+    return;
+  }
+
 
-    Thread *thread = exe_ctx.GetThreadPtr();
-    StackFrameSP frame_sp(thread->GetStackFrameAtIndex(0));
-    ThreadPlanSP new_plan_sp;
 
-    if (frame_sp && frame_sp->HasDebugInformation()) {
-      SymbolContext sc(frame_sp->GetSymbolContext(eSymbolContextEverything));
-      AddressRange range;
-      if (end_line == LLDB_INVALID_LINE_NUMBER)
-        range = sc.line_entry.range;
-      else {
-        if (!sc.GetAddressRangeFromHereToEndLine(end_line, range, error.ref()))
-          return;
-      }
 
-      const LazyBool step_out_avoids_code_without_debug_info =
-          eLazyBoolCalculate;
-      const LazyBool step_in_avoids_code_without_debug_info =
-          eLazyBoolCalculate;
-      new_plan_sp = thread->QueueThreadPlanForStepInRange(
-          abort_other_plans, range, sc, target_name, stop_other_threads,
-          step_in_avoids_code_without_debug_info,
-          step_out_avoids_code_without_debug_info);
-    } else {
-      new_plan_sp = thread->QueueThreadPlanForStepSingleInstruction(
-          false, abort_other_plans, stop_other_threads);
-    }
-
-    error = ResumeNewPlan(exe_ctx, new_plan_sp.get());
-  }
-}
 
 void SBThread::StepOut() {
+  SBError error; // Ignored
+  StepOut(error);
+}
+
+void SBThread::StepOut(SBError &error) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
 
   std::unique_lock<std::recursive_mutex> lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
 
   if (log)
     log->Printf("SBThread(%p)::StepOut ()",
                 static_cast<void *>(exe_ctx.GetThreadPtr()));
 
-  if (exe_ctx.HasThreadScope()) {
-    bool abort_other_plans = false;
-    bool stop_other_threads = false;
+  if (!exe_ctx.HasThreadScope()) {
+    error.SetErrorString("this SBThread object is invalid");
+    return;
+  }
+
 
-    Thread *thread = exe_ctx.GetThreadPtr();
 
-    const LazyBool avoid_no_debug = eLazyBoolCalculate;
-    ThreadPlanSP new_plan_sp(thread->QueueThreadPlanForStepOut(
-        abort_other_plans, NULL, false, stop_other_threads, eVoteYes,
-        eVoteNoOpinion, 0, avoid_no_debug));
 
-    // This returns an error, we should use it!
-    ResumeNewPlan(exe_ctx, new_plan_sp.get());
-  }
+  error = ResumeNewPlan(exe_ctx, new_plan_sp.get());
+
+void SBThread::StepOutOfFrame(SBFrame &sb_frame) {
+  SBError error; // Ignored
+  StepOutOfFrame(sb_frame, error);
 }
 
-void SBThread::StepOutOfFrame(lldb::SBFrame &sb_frame) {
+void SBThread::StepOutOfFrame(SBFrame &sb_frame, SBError &error) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
 
   std::unique_lock<std::recursive_mutex> lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
 
   if (!sb_frame.IsValid()) {
     if (log)
       log->Printf(
           "SBThread(%p)::StepOutOfFrame passed an invalid frame, returning.",
           static_cast<void *>(exe_ctx.GetThreadPtr()));
+    error.SetErrorString("passed invalid SBFrame object");
     return;
   }
 
   StackFrameSP frame_sp(sb_frame.GetFrameSP());
   if (log) {
     SBStream frame_desc_strm;
     sb_frame.GetDescription(frame_desc_strm);
     log->Printf("SBThread(%p)::StepOutOfFrame (frame = SBFrame(%p): %s)",
                 static_cast<void *>(exe_ctx.GetThreadPtr()),
                 static_cast<void *>(frame_sp.get()), frame_desc_strm.GetData());
   }
 
-  if (exe_ctx.HasThreadScope()) {
-    bool abort_other_plans = false;
-    bool stop_other_threads = false;
-    Thread *thread = exe_ctx.GetThreadPtr();
-    if (sb_frame.GetThread().GetThreadID() != thread->GetID()) {
-      log->Printf("SBThread(%p)::StepOutOfFrame passed a frame from another "
-                  "thread (0x%" PRIx64 " vrs. 0x%" PRIx64 ", returning.",
-                  static_cast<void *>(exe_ctx.GetThreadPtr()),
-                  sb_frame.GetThread().GetThreadID(), thread->GetID());
-    }
+  if (!exe_ctx.HasThreadScope()) {
+    error.SetErrorString("this SBThread object is invalid");
+    return;
+  }
+
+    error.SetErrorString("passed a frame from another thread");
+    return;
 
-    ThreadPlanSP new_plan_sp(thread->QueueThreadPlanForStepOut(
-        abort_other_plans, NULL, false, stop_other_threads, eVoteYes,
-        eVoteNoOpinion, frame_sp->GetFrameIndex()));
 
-    // This returns an error, we should use it!
-    ResumeNewPlan(exe_ctx, new_plan_sp.get());
-  }
+  error = ResumeNewPlan(exe_ctx, new_plan_sp.get());
 }
 
 void SBThread::StepInstruction(bool step_over) {
+  SBError error; // Ignored
+  StepInstruction(step_over, error);
+}
+
+void SBThread::StepInstruction(bool step_over, SBError &error) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
 
   std::unique_lock<std::recursive_mutex> lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
 
   if (log)
     log->Printf("SBThread(%p)::StepInstruction (step_over=%i)",
                 static_cast<void *>(exe_ctx.GetThreadPtr()), step_over);
 
-  if (exe_ctx.HasThreadScope()) {
-    Thread *thread = exe_ctx.GetThreadPtr();
-    ThreadPlanSP new_plan_sp(
-        thread->QueueThreadPlanForStepSingleInstruction(step_over, true, true));
+  if (!exe_ctx.HasThreadScope()) {
+    error.SetErrorString("this SBThread object is invalid");
+    return;
+  }
+
 
-    // This returns an error, we should use it!
-    ResumeNewPlan(exe_ctx, new_plan_sp.get());
-  }
+  error = ResumeNewPlan(exe_ctx, new_plan_sp.get());
 }
 
 void SBThread::RunToAddress(lldb::addr_t addr) {
+  SBError error; // Ignored
+  RunToAddress(addr, error);
+}
+
+void SBThread::RunToAddress(lldb::addr_t addr, SBError &error) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
 
   std::unique_lock<std::recursive_mutex> lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
 
   if (log)
     log->Printf("SBThread(%p)::RunToAddress (addr=0x%" PRIx64 ")",
                 static_cast<void *>(exe_ctx.GetThreadPtr()), addr);
 
-  if (exe_ctx.HasThreadScope()) {
-    bool abort_other_plans = false;
-    bool stop_other_threads = true;
+  if (!exe_ctx.HasThreadScope()) {
+    error.SetErrorString("this SBThread object is invalid");
+    return;
+  }
+
 
-    Address target_addr(addr);
 
-    Thread *thread = exe_ctx.GetThreadPtr();
 
-    ThreadPlanSP new_plan_sp(thread->QueueThreadPlanForRunToAddress(
-        abort_other_plans, target_addr, stop_other_threads));
 
-    // This returns an error, we should use it!
-    ResumeNewPlan(exe_ctx, new_plan_sp.get());
-  }
+  error = ResumeNewPlan(exe_ctx, new_plan_sp.get());
 }
 
 SBError SBThread::StepOverUntil(lldb::SBFrame &sb_frame,
                                 lldb::SBFileSpec &sb_file_spec, uint32_t line) {
   SBError sb_error;
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
   char path[PATH_MAX];
 
if (exe_ctx.HasThreadScope()) {
     if (sb_error.Success())
       thread->SetSelectedFrameByIndex(0, false);
   }
 
   return sb_error;
 }
 
 bool SBThread::Suspend() {
+  SBError error; // Ignored
+  return Suspend(error);
+}
+
+bool SBThread::Suspend(SBError &error) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
   std::unique_lock<std::recursive_mutex> lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
 
   bool result = false;
   if (exe_ctx.HasThreadScope()) {
     Process::StopLocker stop_locker;
     if (stop_locker.TryLock(&exe_ctx.GetProcessPtr()->GetRunLock())) {
       exe_ctx.GetThreadPtr()->SetResumeState(eStateSuspended);
       result = true;
     } else {
+      error.SetErrorString("process is running");
       if (log)
         log->Printf("SBThread(%p)::Suspend() => error: process is running",
                     static_cast<void *>(exe_ctx.GetThreadPtr()));
     }
-  }
+  } else
+    error.SetErrorString("this SBThread object is invalid");
   if (log)
     log->Printf("SBThread(%p)::Suspend() => %i",
                 static_cast<void *>(exe_ctx.GetThreadPtr()), result);
   return result;
 }
 
 bool SBThread::Resume() {
+  SBError error; // Ignored
+  return Resume(error);
+}
+
+bool SBThread::Resume(SBError &error) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
   std::unique_lock<std::recursive_mutex> lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
 
   bool result = false;
   if (exe_ctx.HasThreadScope()) {
     Process::StopLocker stop_locker;
     if (stop_locker.TryLock(&exe_ctx.GetProcessPtr()->GetRunLock())) {
       const bool override_suspend = true;
       exe_ctx.GetThreadPtr()->SetResumeState(eStateRunning, override_suspend);
       result = true;
     } else {
+      error.SetErrorString("process is running");
       if (log)
         log->Printf("SBThread(%p)::Resume() => error: process is running",
                     static_cast<void *>(exe_ctx.GetThreadPtr()));
     }
-  }
+  } else
+    error.SetErrorString("this SBThread object is invalid");
   if (log)
     log->Printf("SBThread(%p)::Resume() => %i",
                 static_cast<void *>(exe_ctx.GetThreadPtr()), result);
   return result;
 }
 
 bool SBThread::IsSuspended() {
   std::unique_lock<std::recursive_mutex> lock;