[Lldb-commits] [PATCH] D54221: Add setting to require hardware breakpoints.

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB346920: Add setting to require hardware breakpoints. 
(authored by JDevlieghere, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D54221?vs=174091=174136#toc

Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54221

Files:
  include/lldb/API/SBBreakpoint.h
  include/lldb/API/SBThreadPlan.h
  include/lldb/Breakpoint/Breakpoint.h
  include/lldb/Target/Target.h
  include/lldb/Target/Thread.h
  include/lldb/Target/ThreadPlan.h
  include/lldb/Target/ThreadPlanPython.h
  include/lldb/Target/ThreadPlanShouldStopHere.h
  include/lldb/Target/ThreadPlanStepInRange.h
  include/lldb/Target/ThreadPlanStepInstruction.h
  include/lldb/Target/ThreadPlanStepOut.h
  include/lldb/Target/ThreadPlanStepThrough.h
  include/lldb/Target/ThreadPlanStepUntil.h
  include/lldb/lldb-private-interfaces.h
  
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/Makefile
  
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py
  
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/main.c
  
packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
  scripts/interface/SBBreakpoint.i
  source/API/SBBreakpoint.cpp
  source/API/SBThread.cpp
  source/API/SBThreadPlan.cpp
  source/Breakpoint/Breakpoint.cpp
  source/Commands/CommandObjectThread.cpp
  
source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
  source/Target/Process.cpp
  source/Target/StopInfo.cpp
  source/Target/Target.cpp
  source/Target/Thread.cpp
  source/Target/ThreadPlan.cpp
  source/Target/ThreadPlanCallOnFunctionExit.cpp
  source/Target/ThreadPlanPython.cpp
  source/Target/ThreadPlanRunToAddress.cpp
  source/Target/ThreadPlanShouldStopHere.cpp
  source/Target/ThreadPlanStepInRange.cpp
  source/Target/ThreadPlanStepInstruction.cpp
  source/Target/ThreadPlanStepOut.cpp
  source/Target/ThreadPlanStepOverRange.cpp
  source/Target/ThreadPlanStepRange.cpp
  source/Target/ThreadPlanStepThrough.cpp
  source/Target/ThreadPlanStepUntil.cpp

Index: include/lldb/lldb-private-interfaces.h
===
--- include/lldb/lldb-private-interfaces.h
+++ include/lldb/lldb-private-interfaces.h
@@ -84,10 +84,10 @@
OptionValue *option_value);
 typedef bool (*ThreadPlanShouldStopHereCallback)(
 ThreadPlan *current_plan, Flags , lldb::FrameComparison operation,
-void *baton);
+Status , void *baton);
 typedef lldb::ThreadPlanSP (*ThreadPlanStepFromHereCallback)(
 ThreadPlan *current_plan, Flags , lldb::FrameComparison operation,
-void *baton);
+Status , void *baton);
 typedef UnwindAssembly *(*UnwindAssemblyCreateInstance)(const ArchSpec );
 typedef lldb::MemoryHistorySP (*MemoryHistoryCreateInstance)(
 const lldb::ProcessSP _sp);
Index: include/lldb/Target/ThreadPlanStepOut.h
===
--- include/lldb/Target/ThreadPlanStepOut.h
+++ include/lldb/Target/ThreadPlanStepOut.h
@@ -77,7 +77,7 @@
   friend lldb::ThreadPlanSP Thread::QueueThreadPlanForStepOut(
   bool abort_other_plans, SymbolContext *addr_context, bool first_insn,
   bool stop_others, Vote stop_vote, Vote run_vote, uint32_t frame_idx,
-  LazyBool step_out_avoids_code_without_debug_info);
+  Status , LazyBool step_out_avoids_code_without_debug_info);
 
   void SetupAvoidNoDebug(LazyBool step_out_avoids_code_without_debug_info);
   // Need an appropriate marker for the current stack so we can tell step out
Index: include/lldb/Target/ThreadPlanShouldStopHere.h
===
--- include/lldb/Target/ThreadPlanShouldStopHere.h
+++ include/lldb/Target/ThreadPlanShouldStopHere.h
@@ -97,10 +97,12 @@
 
   void ClearShouldStopHereCallbacks() { m_callbacks.Clear(); }
 
-  bool InvokeShouldStopHereCallback(lldb::FrameComparison operation);
+  bool InvokeShouldStopHereCallback(lldb::FrameComparison operation,
+Status );
 
   lldb::ThreadPlanSP
-  CheckShouldStopHereAndQueueStepOut(lldb::FrameComparison operation);
+  CheckShouldStopHereAndQueueStepOut(lldb::FrameComparison operation,
+ Status );
 
   lldb_private::Flags () { return m_flags; }
 
@@ -110,14 +112,16 @@
   static bool DefaultShouldStopHereCallback(ThreadPlan *current_plan,
 Flags ,
 lldb::FrameComparison operation,
-void *baton);
+Status , void *baton);
 
   static lldb::ThreadPlanSP
   DefaultStepFromHereCallback(ThreadPlan *current_plan, Flags ,
-  lldb::FrameComparison 

[Lldb-commits] [PATCH] D54221: Add setting to require hardware breakpoints.

[Jim Ingham via Phabricator via lldb-commits]

jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.

Looks good to me.


https://reviews.llvm.org/D54221



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54221: Add setting to require hardware breakpoints.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 174091.
JDevlieghere added a comment.

Feedback Jim: move argument to the end of the list.


https://reviews.llvm.org/D54221

Files:
  include/lldb/API/SBBreakpoint.h
  include/lldb/API/SBThreadPlan.h
  include/lldb/Breakpoint/Breakpoint.h
  include/lldb/Target/Target.h
  include/lldb/Target/Thread.h
  include/lldb/Target/ThreadPlan.h
  include/lldb/Target/ThreadPlanPython.h
  include/lldb/Target/ThreadPlanShouldStopHere.h
  include/lldb/Target/ThreadPlanStepInRange.h
  include/lldb/Target/ThreadPlanStepInstruction.h
  include/lldb/Target/ThreadPlanStepOut.h
  include/lldb/Target/ThreadPlanStepThrough.h
  include/lldb/Target/ThreadPlanStepUntil.h
  include/lldb/lldb-private-interfaces.h
  
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/Makefile
  
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py
  
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/main.c
  
packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
  scripts/interface/SBBreakpoint.i
  source/API/SBBreakpoint.cpp
  source/API/SBThread.cpp
  source/API/SBThreadPlan.cpp
  source/Breakpoint/Breakpoint.cpp
  source/Commands/CommandObjectThread.cpp
  
source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
  source/Target/Process.cpp
  source/Target/StopInfo.cpp
  source/Target/Target.cpp
  source/Target/Thread.cpp
  source/Target/ThreadPlan.cpp
  source/Target/ThreadPlanCallOnFunctionExit.cpp
  source/Target/ThreadPlanPython.cpp
  source/Target/ThreadPlanRunToAddress.cpp
  source/Target/ThreadPlanShouldStopHere.cpp
  source/Target/ThreadPlanStepInRange.cpp
  source/Target/ThreadPlanStepInstruction.cpp
  source/Target/ThreadPlanStepOut.cpp
  source/Target/ThreadPlanStepOverRange.cpp
  source/Target/ThreadPlanStepRange.cpp
  source/Target/ThreadPlanStepThrough.cpp
  source/Target/ThreadPlanStepUntil.cpp

Index: source/Target/ThreadPlanStepUntil.cpp
===
--- source/Target/ThreadPlanStepUntil.cpp
+++ source/Target/ThreadPlanStepUntil.cpp
@@ -53,7 +53,10 @@
   m_return_addr = return_frame_sp->GetStackID().GetPC();
   Breakpoint *return_bp =
   target_sp->CreateBreakpoint(m_return_addr, true, false).get();
+
   if (return_bp != nullptr) {
+if (return_bp->IsHardware() && !return_bp->HasResolvedLocations())
+  m_could_not_resolve_hw_bp = true;
 return_bp->SetThreadID(thread_id);
 m_return_bp_id = return_bp->GetID();
 return_bp->SetBreakpointKind("until-return-backstop");
@@ -93,6 +96,7 @@
 }
   }
   m_until_points.clear();
+  m_could_not_resolve_hw_bp = false;
 }
 
 void ThreadPlanStepUntil::GetDescription(Stream *s,
@@ -123,9 +127,16 @@
 }
 
 bool ThreadPlanStepUntil::ValidatePlan(Stream *error) {
-  if (m_return_bp_id == LLDB_INVALID_BREAK_ID)
+  if (m_could_not_resolve_hw_bp) {
+if (error)
+  error->PutCString(
+  "Could not create hardware breakpoint for thread plan.");
+return false;
+  } else if (m_return_bp_id == LLDB_INVALID_BREAK_ID) {
+if (error)
+  error->PutCString("Could not create return breakpoint.");
 return false;
-  else {
+  } else {
 until_collection::iterator pos, end = m_until_points.end();
 for (pos = m_until_points.begin(); pos != end; pos++) {
   if (!LLDB_BREAK_ID_IS_VALID((*pos).second))
Index: source/Target/ThreadPlanStepThrough.cpp
===
--- source/Target/ThreadPlanStepThrough.cpp
+++ source/Target/ThreadPlanStepThrough.cpp
@@ -58,7 +58,10 @@
   ->GetTarget()
   .CreateBreakpoint(m_backstop_addr, true, false)
   .get();
+
   if (return_bp != nullptr) {
+if (return_bp->IsHardware() && !return_bp->HasResolvedLocations())
+  m_could_not_resolve_hw_bp = true;
 return_bp->SetThreadID(m_thread.GetID());
 m_backstop_bkpt_id = return_bp->GetID();
 return_bp->SetBreakpointKind("step-through-backstop");
@@ -135,7 +138,26 @@
 }
 
 bool ThreadPlanStepThrough::ValidatePlan(Stream *error) {
-  return m_sub_plan_sp.get() != nullptr;
+  if (m_could_not_resolve_hw_bp) {
+if (error)
+  error->PutCString(
+  "Could not create hardware breakpoint for thread plan.");
+return false;
+  }
+
+  if (m_backstop_bkpt_id == LLDB_INVALID_BREAK_ID) {
+if (error)
+  error->PutCString("Could not create backstop breakpoint.");
+return false;
+  }
+
+  if (!m_sub_plan_sp.get()) {
+if (error)
+  error->PutCString("Does not have a subplan.");
+return false;
+  }
+
+  return true;
 }
 
 bool ThreadPlanStepThrough::DoPlanExplainsStop(Event *event_ptr) {
@@ -211,6 +233,7 @@
   if (m_backstop_bkpt_id != LLDB_INVALID_BREAK_ID) {
 

[Lldb-commits] [PATCH] D54221: Add setting to require hardware breakpoints.

[Jim Ingham via Phabricator via lldb-commits]

jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.

The lldb API's parameters are ordered input first than output.  Pretty much all 
the API's that take a Status as a parameter take it as the last parameter.  So 
it looks weird to have the Status  first in the QueueThreadPlan... API's. 
 This pattern gets annoying when you have default parameters, so it's okay to 
put the out parameters before the default parameters (though default parameters 
are also not so great, so removing them is also okay...)

Other than that this looked fine.


https://reviews.llvm.org/D54221



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54221: Add setting to require hardware breakpoints.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In https://reviews.llvm.org/D54221#1290638, @JDevlieghere wrote:

> In https://reviews.llvm.org/D54221#1290572, @labath wrote:
>
> > I recall something about linux on arm having a magic unmodifiable (even by 
> > ptrace) page of memory, so this could be useful there too. However, it's 
> > not clear to me how a user is going to figure out that he needs to enable 
> > this setting. Would it make sense to automatically try setting a hardware 
> > breakpoint if software breakpoint fails?
>
>
> My main concern would be that hardware breakpoints are a limited resource and 
> not something we want to make transparent to the user, because it's only a 
> matter of time before it fails.


That is true, but on the other hand, you would only use hw breakpoints on those 
pieces of memory where you really need to instead of everywhere, which means 
(at least for the use case I have in mind) it be used very rarely. Of course, 
we would have to be careful do differentiate between reasons why setting a sw 
breakpoint failed (is it because the memory is RO, or some other reason like 
there is no memory at that address).

However, with this approach, it's still not clear to me how will the user know 
that he has to enable this setting? Will he get some sort of an error pointing 
here when the sw breakpoint fails? Or will you just enable this setting by 
default for targets where you know this is an issue?


https://reviews.llvm.org/D54221



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54221: Add setting to require hardware breakpoints.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 173191.
JDevlieghere added a comment.

Update tests to use SB API.


https://reviews.llvm.org/D54221

Files:
  include/lldb/API/SBBreakpoint.h
  include/lldb/API/SBThreadPlan.h
  include/lldb/Breakpoint/Breakpoint.h
  include/lldb/Target/Target.h
  include/lldb/Target/Thread.h
  include/lldb/Target/ThreadPlan.h
  include/lldb/Target/ThreadPlanPython.h
  include/lldb/Target/ThreadPlanShouldStopHere.h
  include/lldb/Target/ThreadPlanStepInRange.h
  include/lldb/Target/ThreadPlanStepInstruction.h
  include/lldb/Target/ThreadPlanStepOut.h
  include/lldb/Target/ThreadPlanStepThrough.h
  include/lldb/Target/ThreadPlanStepUntil.h
  include/lldb/lldb-private-interfaces.h
  
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/Makefile
  
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py
  
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/main.c
  
packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
  scripts/interface/SBBreakpoint.i
  source/API/SBBreakpoint.cpp
  source/API/SBThread.cpp
  source/API/SBThreadPlan.cpp
  source/Breakpoint/Breakpoint.cpp
  source/Commands/CommandObjectThread.cpp
  
source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
  source/Target/Process.cpp
  source/Target/StopInfo.cpp
  source/Target/Target.cpp
  source/Target/Thread.cpp
  source/Target/ThreadPlan.cpp
  source/Target/ThreadPlanCallOnFunctionExit.cpp
  source/Target/ThreadPlanPython.cpp
  source/Target/ThreadPlanRunToAddress.cpp
  source/Target/ThreadPlanShouldStopHere.cpp
  source/Target/ThreadPlanStepInRange.cpp
  source/Target/ThreadPlanStepInstruction.cpp
  source/Target/ThreadPlanStepOut.cpp
  source/Target/ThreadPlanStepOverRange.cpp
  source/Target/ThreadPlanStepRange.cpp
  source/Target/ThreadPlanStepThrough.cpp
  source/Target/ThreadPlanStepUntil.cpp

Index: source/Target/ThreadPlanStepUntil.cpp
===
--- source/Target/ThreadPlanStepUntil.cpp
+++ source/Target/ThreadPlanStepUntil.cpp
@@ -57,7 +57,10 @@
   m_return_addr = return_frame_sp->GetStackID().GetPC();
   Breakpoint *return_bp =
   target_sp->CreateBreakpoint(m_return_addr, true, false).get();
+
   if (return_bp != nullptr) {
+if (return_bp->IsHardware() && !return_bp->HasResolvedLocations())
+  m_could_not_resolve_hw_bp = true;
 return_bp->SetThreadID(thread_id);
 m_return_bp_id = return_bp->GetID();
 return_bp->SetBreakpointKind("until-return-backstop");
@@ -97,6 +100,7 @@
 }
   }
   m_until_points.clear();
+  m_could_not_resolve_hw_bp = false;
 }
 
 void ThreadPlanStepUntil::GetDescription(Stream *s,
@@ -127,9 +131,16 @@
 }
 
 bool ThreadPlanStepUntil::ValidatePlan(Stream *error) {
-  if (m_return_bp_id == LLDB_INVALID_BREAK_ID)
+  if (m_could_not_resolve_hw_bp) {
+if (error)
+  error->PutCString(
+  "Could not create hardware breakpoint for thread plan.");
+return false;
+  } else if (m_return_bp_id == LLDB_INVALID_BREAK_ID) {
+if (error)
+  error->PutCString("Could not create return breakpoint.");
 return false;
-  else {
+  } else {
 until_collection::iterator pos, end = m_until_points.end();
 for (pos = m_until_points.begin(); pos != end; pos++) {
   if (!LLDB_BREAK_ID_IS_VALID((*pos).second))
Index: source/Target/ThreadPlanStepThrough.cpp
===
--- source/Target/ThreadPlanStepThrough.cpp
+++ source/Target/ThreadPlanStepThrough.cpp
@@ -62,7 +62,10 @@
   ->GetTarget()
   .CreateBreakpoint(m_backstop_addr, true, false)
   .get();
+
   if (return_bp != nullptr) {
+if (return_bp->IsHardware() && !return_bp->HasResolvedLocations())
+  m_could_not_resolve_hw_bp = true;
 return_bp->SetThreadID(m_thread.GetID());
 m_backstop_bkpt_id = return_bp->GetID();
 return_bp->SetBreakpointKind("step-through-backstop");
@@ -139,7 +142,26 @@
 }
 
 bool ThreadPlanStepThrough::ValidatePlan(Stream *error) {
-  return m_sub_plan_sp.get() != nullptr;
+  if (m_could_not_resolve_hw_bp) {
+if (error)
+  error->PutCString(
+  "Could not create hardware breakpoint for thread plan.");
+return false;
+  }
+
+  if (m_backstop_bkpt_id == LLDB_INVALID_BREAK_ID) {
+if (error)
+  error->PutCString("Could not create backstop breakpoint.");
+return false;
+  }
+
+  if (!m_sub_plan_sp.get()) {
+if (error)
+  error->PutCString("Does not have a subplan.");
+return false;
+  }
+
+  return true;
 }
 
 bool ThreadPlanStepThrough::DoPlanExplainsStop(Event *event_ptr) {
@@ -215,6 +237,7 @@
   if (m_backstop_bkpt_id != LLDB_INVALID_BREAK_ID) {
 

[Lldb-commits] [PATCH] D54221: Add setting to require hardware breakpoints.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 173112.
JDevlieghere marked 18 inline comments as done.
JDevlieghere added a comment.

Feedback from Jim


https://reviews.llvm.org/D54221

Files:
  include/lldb/API/SBBreakpoint.h
  include/lldb/API/SBThreadPlan.h
  include/lldb/Breakpoint/Breakpoint.h
  include/lldb/Target/Target.h
  include/lldb/Target/Thread.h
  include/lldb/Target/ThreadPlan.h
  include/lldb/Target/ThreadPlanPython.h
  include/lldb/Target/ThreadPlanShouldStopHere.h
  include/lldb/Target/ThreadPlanStepInRange.h
  include/lldb/Target/ThreadPlanStepInstruction.h
  include/lldb/Target/ThreadPlanStepOut.h
  include/lldb/Target/ThreadPlanStepThrough.h
  include/lldb/Target/ThreadPlanStepUntil.h
  include/lldb/lldb-private-interfaces.h
  
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/Makefile
  
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py
  
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/main.c
  
packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
  scripts/interface/SBBreakpoint.i
  source/API/SBBreakpoint.cpp
  source/API/SBThread.cpp
  source/API/SBThreadPlan.cpp
  source/Breakpoint/Breakpoint.cpp
  source/Commands/CommandObjectThread.cpp
  
source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
  source/Target/Process.cpp
  source/Target/StopInfo.cpp
  source/Target/Target.cpp
  source/Target/Thread.cpp
  source/Target/ThreadPlan.cpp
  source/Target/ThreadPlanCallOnFunctionExit.cpp
  source/Target/ThreadPlanPython.cpp
  source/Target/ThreadPlanRunToAddress.cpp
  source/Target/ThreadPlanShouldStopHere.cpp
  source/Target/ThreadPlanStepInRange.cpp
  source/Target/ThreadPlanStepInstruction.cpp
  source/Target/ThreadPlanStepOut.cpp
  source/Target/ThreadPlanStepOverRange.cpp
  source/Target/ThreadPlanStepRange.cpp
  source/Target/ThreadPlanStepThrough.cpp
  source/Target/ThreadPlanStepUntil.cpp

Index: source/Target/ThreadPlanStepUntil.cpp
===
--- source/Target/ThreadPlanStepUntil.cpp
+++ source/Target/ThreadPlanStepUntil.cpp
@@ -57,7 +57,10 @@
   m_return_addr = return_frame_sp->GetStackID().GetPC();
   Breakpoint *return_bp =
   target_sp->CreateBreakpoint(m_return_addr, true, false).get();
+
   if (return_bp != nullptr) {
+if (return_bp->IsHardware() && !return_bp->HasResolvedLocations())
+  m_could_not_resolve_hw_bp = true;
 return_bp->SetThreadID(thread_id);
 m_return_bp_id = return_bp->GetID();
 return_bp->SetBreakpointKind("until-return-backstop");
@@ -97,6 +100,7 @@
 }
   }
   m_until_points.clear();
+  m_could_not_resolve_hw_bp = false;
 }
 
 void ThreadPlanStepUntil::GetDescription(Stream *s,
@@ -127,9 +131,16 @@
 }
 
 bool ThreadPlanStepUntil::ValidatePlan(Stream *error) {
-  if (m_return_bp_id == LLDB_INVALID_BREAK_ID)
+  if (m_could_not_resolve_hw_bp) {
+if (error)
+  error->PutCString(
+  "Could not create hardware breakpoint for thread plan.");
+return false;
+  } else if (m_return_bp_id == LLDB_INVALID_BREAK_ID) {
+if (error)
+  error->PutCString("Could not create return breakpoint.");
 return false;
-  else {
+  } else {
 until_collection::iterator pos, end = m_until_points.end();
 for (pos = m_until_points.begin(); pos != end; pos++) {
   if (!LLDB_BREAK_ID_IS_VALID((*pos).second))
Index: source/Target/ThreadPlanStepThrough.cpp
===
--- source/Target/ThreadPlanStepThrough.cpp
+++ source/Target/ThreadPlanStepThrough.cpp
@@ -62,7 +62,10 @@
   ->GetTarget()
   .CreateBreakpoint(m_backstop_addr, true, false)
   .get();
+
   if (return_bp != nullptr) {
+if (return_bp->IsHardware() && !return_bp->HasResolvedLocations())
+  m_could_not_resolve_hw_bp = true;
 return_bp->SetThreadID(m_thread.GetID());
 m_backstop_bkpt_id = return_bp->GetID();
 return_bp->SetBreakpointKind("step-through-backstop");
@@ -139,7 +142,26 @@
 }
 
 bool ThreadPlanStepThrough::ValidatePlan(Stream *error) {
-  return m_sub_plan_sp.get() != nullptr;
+  if (m_could_not_resolve_hw_bp) {
+if (error)
+  error->PutCString(
+  "Could not create hardware breakpoint for thread plan.");
+return false;
+  }
+
+  if (m_backstop_bkpt_id == LLDB_INVALID_BREAK_ID) {
+if (error)
+  error->PutCString("Could not create backstop breakpoint.");
+return false;
+  }
+
+  if (!m_sub_plan_sp.get()) {
+if (error)
+  error->PutCString("Does not have a subplan.");
+return false;
+  }
+
+  return true;
 }
 
 bool ThreadPlanStepThrough::DoPlanExplainsStop(Event *event_ptr) {
@@ -215,6 +237,7 @@
   if (m_backstop_bkpt_id != LLDB_INVALID_BREAK_ID) {
 

[Lldb-commits] [PATCH] D54221: Add setting to require hardware breakpoints.

[Jim Ingham via Phabricator via lldb-commits]

jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.

This is pretty good, but in all the places where some plan tries to find a 
sub-plan to do its job, you are losing the text of the error when that job 
fails.  So the controlling plan can't present a good error message.  You need 
to hold onto the Status object and return that, either as the error from 
ValidatePlan if it happens when the plan is getting created, or in the plan's 
stop description so that StopInfoThreadPlan::GetDescription can print it 
properly.




Comment at: include/lldb/Target/Thread.h:643
   //--
-  virtual lldb::ThreadPlanSP QueueFundamentalPlan(bool abort_other_plans);
+  virtual lldb::ThreadPlanSP QueueFundamentalPlan(Status ,
+  bool abort_other_plans);

You need to change the HeaderDoc to describe the status parameter.  Looks like 
you have to do this for all the QueueThreadPlan... functions.



Comment at: include/lldb/Target/ThreadPlanBase.h:55
   friend lldb::ThreadPlanSP
-  Thread::QueueFundamentalPlan(bool abort_other_plans);
+  Thread::QueueFundamentalPlan(Status , bool abort_other_plans);
 

I don't think it is useful to pass an error in this case.  The "Fundamental 
plan" just fields unhandled responses from other plans, and queuing it can 
never fail.  



Comment at: include/lldb/Target/ThreadPlanRunToAddress.h:66
  // using to stop us at m_address.
+  bool m_could_not_resolve_hw_bp;
 

Looks like you are adding this to most of the plans.  Would it make sense to 
add it to the ThreadPlan base class?  This is just an error flag, so it would 
stay false except is a derived plan wants to set it.



Comment at: 
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py:33-43
+exe = self.getBuildArtifact("a.out")
+target = self.dbg.CreateTarget(exe)
+
+breakpoint = target.BreakpointCreateByLocation("main.c", 1)
+
+self.runCmd("run")
+

This can all be done with:

(target, process, stepping_thread) = 
lldbutil.run_to_line_breakpoint(SBFileSpec("main.c"), 1)




Comment at: 
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py:47
+
+self.expect("thread step-in")
+self.expect("thread step-in", error=True)

Can you do this using "SBThread.StepInto" and check the error return in the 
case where it fails?  Since we are treating the possibility of step's failing, 
we need to make sure that the SB API's return errors everywhere and that the 
errors look right.  So it would be good to test that.

Ditto for the other stepping tests.



Comment at: source/API/SBThread.cpp:733
 new_plan_sp = thread->QueueThreadPlanForStepSingleInstruction(
-false, abort_other_plans, stop_other_threads);
+new_plan_status, false, abort_other_plans, stop_other_threads);
   }

Shouldn't new_plan_status get reflected in the "error" parameter passed into 
SBThread::StepInto?



Comment at: source/API/SBThread.cpp:767
+  new_plan_status, abort_other_plans, NULL, false, stop_other_threads,
+  eVoteYes, eVoteNoOpinion, 0, avoid_no_debug));
 

Same here.  If new_plan_status comes back with an error, we probably don't want 
to call ResumeNewPlan, and we want to report the error from queueing the plan.



Comment at: source/API/SBThread.cpp:818
 
+  Status new_plan_status;
   ThreadPlanSP new_plan_sp(thread->QueueThreadPlanForStepOut(

Same comment here.



Comment at: source/API/SBThread.cpp:847
   Thread *thread = exe_ctx.GetThreadPtr();
-  ThreadPlanSP new_plan_sp(
-  thread->QueueThreadPlanForStepSingleInstruction(step_over, true, true));
+  Status new_plan_status;
+  ThreadPlanSP new_plan_sp(thread->QueueThreadPlanForStepSingleInstruction(

And here.



Comment at: source/API/SBThread.cpp:881
 
+  Status new_plan_status;
   ThreadPlanSP new_plan_sp(thread->QueueThreadPlanForRunToAddress(

And here.



Comment at: source/API/SBThreadPlan.cpp:155
 start_address->CalculateSymbolContext();
+Status plan_status;
 return SBThreadPlan(

Can you add a variant of these calls that takes an SBError &?  The scripted 
thread plans will need to have a way to report this error when they try to 
queue a plan and it fails.  And below as well.



Comment at: source/Commands/CommandObjectThread.cpp:802
 } else {
+  result.SetError(new_plan_status);
   result.AppendError("Couldn't find thread 

[Lldb-commits] [PATCH] D54221: Add setting to require hardware breakpoints.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 173021.
JDevlieghere added a comment.

Remove `ValidatePlan` implementation for `ThreadPlanPython` as I believe it's 
bogus.


https://reviews.llvm.org/D54221

Files:
  include/lldb/API/SBBreakpoint.h
  include/lldb/Breakpoint/Breakpoint.h
  include/lldb/Target/Target.h
  include/lldb/Target/Thread.h
  include/lldb/Target/ThreadPlanBase.h
  include/lldb/Target/ThreadPlanRunToAddress.h
  include/lldb/Target/ThreadPlanStepInRange.h
  include/lldb/Target/ThreadPlanStepInstruction.h
  include/lldb/Target/ThreadPlanStepOut.h
  include/lldb/Target/ThreadPlanStepRange.h
  include/lldb/Target/ThreadPlanStepThrough.h
  include/lldb/Target/ThreadPlanStepUntil.h
  
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/Makefile
  
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py
  
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/main.c
  
packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
  scripts/interface/SBBreakpoint.i
  source/API/SBBreakpoint.cpp
  source/API/SBThread.cpp
  source/API/SBThreadPlan.cpp
  source/Breakpoint/Breakpoint.cpp
  source/Commands/CommandObjectThread.cpp
  
source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
  source/Target/Process.cpp
  source/Target/StopInfo.cpp
  source/Target/Target.cpp
  source/Target/Thread.cpp
  source/Target/ThreadPlanCallOnFunctionExit.cpp
  source/Target/ThreadPlanPython.cpp
  source/Target/ThreadPlanRunToAddress.cpp
  source/Target/ThreadPlanShouldStopHere.cpp
  source/Target/ThreadPlanStepInRange.cpp
  source/Target/ThreadPlanStepInstruction.cpp
  source/Target/ThreadPlanStepOut.cpp
  source/Target/ThreadPlanStepOverRange.cpp
  source/Target/ThreadPlanStepRange.cpp
  source/Target/ThreadPlanStepThrough.cpp
  source/Target/ThreadPlanStepUntil.cpp

Index: source/Target/ThreadPlanStepUntil.cpp
===
--- source/Target/ThreadPlanStepUntil.cpp
+++ source/Target/ThreadPlanStepUntil.cpp
@@ -39,7 +39,8 @@
   m_return_bp_id(LLDB_INVALID_BREAK_ID),
   m_return_addr(LLDB_INVALID_ADDRESS), m_stepped_out(false),
   m_should_stop(false), m_ran_analyze(false), m_explains_stop(false),
-  m_until_points(), m_stop_others(stop_others) {
+  m_until_points(), m_stop_others(stop_others),
+  m_could_not_resolve_hw_bp(false) {
   // Stash away our "until" addresses:
   TargetSP target_sp(m_thread.CalculateTarget());
 
@@ -57,7 +58,10 @@
   m_return_addr = return_frame_sp->GetStackID().GetPC();
   Breakpoint *return_bp =
   target_sp->CreateBreakpoint(m_return_addr, true, false).get();
+
   if (return_bp != nullptr) {
+if (return_bp->IsHardware() && !return_bp->HasResolvedLocations())
+  m_could_not_resolve_hw_bp = true;
 return_bp->SetThreadID(thread_id);
 m_return_bp_id = return_bp->GetID();
 return_bp->SetBreakpointKind("until-return-backstop");
@@ -97,6 +101,7 @@
 }
   }
   m_until_points.clear();
+  m_could_not_resolve_hw_bp = false;
 }
 
 void ThreadPlanStepUntil::GetDescription(Stream *s,
@@ -127,9 +132,16 @@
 }
 
 bool ThreadPlanStepUntil::ValidatePlan(Stream *error) {
-  if (m_return_bp_id == LLDB_INVALID_BREAK_ID)
+  if (m_could_not_resolve_hw_bp) {
+if (error)
+  error->PutCString(
+  "Could not create hardware breakpoint for thread plan.");
+return false;
+  } else if (m_return_bp_id == LLDB_INVALID_BREAK_ID) {
+if (error)
+  error->PutCString("Could not create return breakpoint.");
 return false;
-  else {
+  } else {
 until_collection::iterator pos, end = m_until_points.end();
 for (pos = m_until_points.begin(); pos != end; pos++) {
   if (!LLDB_BREAK_ID_IS_VALID((*pos).second))
Index: source/Target/ThreadPlanStepThrough.cpp
===
--- source/Target/ThreadPlanStepThrough.cpp
+++ source/Target/ThreadPlanStepThrough.cpp
@@ -40,7 +40,7 @@
  eVoteNoOpinion, eVoteNoOpinion),
   m_start_address(0), m_backstop_bkpt_id(LLDB_INVALID_BREAK_ID),
   m_backstop_addr(LLDB_INVALID_ADDRESS), m_return_stack_id(m_stack_id),
-  m_stop_others(stop_others) {
+  m_stop_others(stop_others), m_could_not_resolve_hw_bp(false) {
   LookForPlanToStepThroughFromCurrentPC();
 
   // If we don't get a valid step through plan, don't bother to set up a
@@ -62,7 +62,10 @@
   ->GetTarget()
   .CreateBreakpoint(m_backstop_addr, true, false)
   .get();
+
   if (return_bp != nullptr) {
+if (return_bp->IsHardware() && !return_bp->HasResolvedLocations())
+  m_could_not_resolve_hw_bp = true;
 return_bp->SetThreadID(m_thread.GetID());
 m_backstop_bkpt_id = return_bp->GetID();
 

[Lldb-commits] [PATCH] D54221: Add setting to require hardware breakpoints.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere updated this revision to Diff 173015.
JDevlieghere added a comment.

Fix bug for `thread until` and add test case.


https://reviews.llvm.org/D54221

Files:
  include/lldb/API/SBBreakpoint.h
  include/lldb/Breakpoint/Breakpoint.h
  include/lldb/Target/Target.h
  include/lldb/Target/Thread.h
  include/lldb/Target/ThreadPlanBase.h
  include/lldb/Target/ThreadPlanRunToAddress.h
  include/lldb/Target/ThreadPlanStepInRange.h
  include/lldb/Target/ThreadPlanStepInstruction.h
  include/lldb/Target/ThreadPlanStepOut.h
  include/lldb/Target/ThreadPlanStepRange.h
  include/lldb/Target/ThreadPlanStepThrough.h
  include/lldb/Target/ThreadPlanStepUntil.h
  
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/Makefile
  
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py
  
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/main.c
  
packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py
  scripts/interface/SBBreakpoint.i
  source/API/SBBreakpoint.cpp
  source/API/SBThread.cpp
  source/API/SBThreadPlan.cpp
  source/Breakpoint/Breakpoint.cpp
  source/Commands/CommandObjectThread.cpp
  
source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
  source/Target/Process.cpp
  source/Target/StopInfo.cpp
  source/Target/Target.cpp
  source/Target/Thread.cpp
  source/Target/ThreadPlanCallOnFunctionExit.cpp
  source/Target/ThreadPlanPython.cpp
  source/Target/ThreadPlanRunToAddress.cpp
  source/Target/ThreadPlanShouldStopHere.cpp
  source/Target/ThreadPlanStepInRange.cpp
  source/Target/ThreadPlanStepInstruction.cpp
  source/Target/ThreadPlanStepOut.cpp
  source/Target/ThreadPlanStepOverRange.cpp
  source/Target/ThreadPlanStepRange.cpp
  source/Target/ThreadPlanStepThrough.cpp
  source/Target/ThreadPlanStepUntil.cpp

Index: source/Target/ThreadPlanStepUntil.cpp
===
--- source/Target/ThreadPlanStepUntil.cpp
+++ source/Target/ThreadPlanStepUntil.cpp
@@ -39,7 +39,8 @@
   m_return_bp_id(LLDB_INVALID_BREAK_ID),
   m_return_addr(LLDB_INVALID_ADDRESS), m_stepped_out(false),
   m_should_stop(false), m_ran_analyze(false), m_explains_stop(false),
-  m_until_points(), m_stop_others(stop_others) {
+  m_until_points(), m_stop_others(stop_others),
+  m_could_not_resolve_hw_bp(false) {
   // Stash away our "until" addresses:
   TargetSP target_sp(m_thread.CalculateTarget());
 
@@ -57,7 +58,10 @@
   m_return_addr = return_frame_sp->GetStackID().GetPC();
   Breakpoint *return_bp =
   target_sp->CreateBreakpoint(m_return_addr, true, false).get();
+
   if (return_bp != nullptr) {
+if (return_bp->IsHardware() && !return_bp->HasResolvedLocations())
+  m_could_not_resolve_hw_bp = true;
 return_bp->SetThreadID(thread_id);
 m_return_bp_id = return_bp->GetID();
 return_bp->SetBreakpointKind("until-return-backstop");
@@ -97,6 +101,7 @@
 }
   }
   m_until_points.clear();
+  m_could_not_resolve_hw_bp = false;
 }
 
 void ThreadPlanStepUntil::GetDescription(Stream *s,
@@ -127,9 +132,16 @@
 }
 
 bool ThreadPlanStepUntil::ValidatePlan(Stream *error) {
-  if (m_return_bp_id == LLDB_INVALID_BREAK_ID)
+  if (m_could_not_resolve_hw_bp) {
+if (error)
+  error->PutCString(
+  "Could not create hardware breakpoint for thread plan.");
+return false;
+  } else if (m_return_bp_id == LLDB_INVALID_BREAK_ID) {
+if (error)
+  error->PutCString("Could not create return breakpoint.");
 return false;
-  else {
+  } else {
 until_collection::iterator pos, end = m_until_points.end();
 for (pos = m_until_points.begin(); pos != end; pos++) {
   if (!LLDB_BREAK_ID_IS_VALID((*pos).second))
Index: source/Target/ThreadPlanStepThrough.cpp
===
--- source/Target/ThreadPlanStepThrough.cpp
+++ source/Target/ThreadPlanStepThrough.cpp
@@ -40,7 +40,7 @@
  eVoteNoOpinion, eVoteNoOpinion),
   m_start_address(0), m_backstop_bkpt_id(LLDB_INVALID_BREAK_ID),
   m_backstop_addr(LLDB_INVALID_ADDRESS), m_return_stack_id(m_stack_id),
-  m_stop_others(stop_others) {
+  m_stop_others(stop_others), m_could_not_resolve_hw_bp(false) {
   LookForPlanToStepThroughFromCurrentPC();
 
   // If we don't get a valid step through plan, don't bother to set up a
@@ -62,7 +62,10 @@
   ->GetTarget()
   .CreateBreakpoint(m_backstop_addr, true, false)
   .get();
+
   if (return_bp != nullptr) {
+if (return_bp->IsHardware() && !return_bp->HasResolvedLocations())
+  m_could_not_resolve_hw_bp = true;
 return_bp->SetThreadID(m_thread.GetID());
 m_backstop_bkpt_id = return_bp->GetID();
 return_bp->SetBreakpointKind("step-through-backstop");
@@ -139,7 +142,26 @@
 }
 
 

[Lldb-commits] [PATCH] D54221: Add setting to require hardware breakpoints.

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.

On NetBSD one has to check PaX MPROTECT property of a traced process.

Something like:

  bool IsMPROTECT(pid_t pid) {
  #if defined(__NetBSD__)
int mib[3];
int paxflags;
size_t len = sizeof(paxflags);
  
mib[0] = CTL_PROC;
mib[1] = pid;
mib[2] = PROC_PID_PAXFLAGS;
  
if (sysctl(mib, 3, , , NULL, 0) != 0)
  err(EXIT_FAILURE, "sysctl"); /* or return true */
  
return !!(paxflags & CTL_PROC_PAXFLAGS_MPROTECT);
  #else
return false;
  #endif
  }

If IsMPROTECT is true, then we must use hardware assisted/emulated breakpoints.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54221



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54221: Add setting to require hardware breakpoints.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I recall something about linux on arm having a magic unmodifiable (even by 
ptrace) page of memory, so this could be useful there too. However, it's not 
clear to me how a user is going to figure out that he needs to enable this 
setting. Would it make sense to automatically try setting a hardware breakpoint 
if software breakpoint fails?


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54221



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D54221: Add setting to require hardware breakpoints.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: jingham, jasonmolenda.
JDevlieghere added a project: LLDB.
Herald added subscribers: teemperor, abidh.

When debugging read-only memory we cannot use software breakpoint. We already 
have support for hardware breakpoints and users can specify them with `-H`. 
However, there's no option to force hardware breakpoints even while stepping.

This patch adds a setting `target.require-hardware-breakpoint` that forces LLDB 
to always use hardware breakpoints. Because hardware breakpoints are a limited 
resource and therefore can fail to resolve, this patch also extends error 
handling in thread plans, where breakpoints are used for stepping.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54221

Files:
  include/lldb/API/SBBreakpoint.h
  include/lldb/Breakpoint/Breakpoint.h
  include/lldb/Target/Target.h
  include/lldb/Target/Thread.h
  include/lldb/Target/ThreadPlanBase.h
  include/lldb/Target/ThreadPlanStepInRange.h
  include/lldb/Target/ThreadPlanStepInstruction.h
  include/lldb/Target/ThreadPlanStepOut.h
  include/lldb/Target/ThreadPlanStepRange.h
  include/lldb/Target/ThreadPlanStepThrough.h
  include/lldb/Target/ThreadPlanStepUntil.h
  
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/Makefile
  
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/TestRequireHWBreakpoints.py
  
packages/Python/lldbsuite/test/functionalities/breakpoint/require_hw_breakpoints/main.c
  scripts/interface/SBBreakpoint.i
  source/API/SBBreakpoint.cpp
  source/API/SBThread.cpp
  source/API/SBThreadPlan.cpp
  source/Breakpoint/Breakpoint.cpp
  source/Commands/CommandObjectThread.cpp
  
source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
  source/Target/Process.cpp
  source/Target/StopInfo.cpp
  source/Target/Target.cpp
  source/Target/Thread.cpp
  source/Target/ThreadPlanCallOnFunctionExit.cpp
  source/Target/ThreadPlanShouldStopHere.cpp
  source/Target/ThreadPlanStepInRange.cpp
  source/Target/ThreadPlanStepInstruction.cpp
  source/Target/ThreadPlanStepOut.cpp
  source/Target/ThreadPlanStepOverRange.cpp
  source/Target/ThreadPlanStepRange.cpp
  source/Target/ThreadPlanStepThrough.cpp
  source/Target/ThreadPlanStepUntil.cpp

Index: source/Target/ThreadPlanStepUntil.cpp
===
--- source/Target/ThreadPlanStepUntil.cpp
+++ source/Target/ThreadPlanStepUntil.cpp
@@ -39,7 +39,8 @@
   m_return_bp_id(LLDB_INVALID_BREAK_ID),
   m_return_addr(LLDB_INVALID_ADDRESS), m_stepped_out(false),
   m_should_stop(false), m_ran_analyze(false), m_explains_stop(false),
-  m_until_points(), m_stop_others(stop_others) {
+  m_until_points(), m_stop_others(stop_others),
+  m_could_not_resolve_hw_bp(false) {
   // Stash away our "until" addresses:
   TargetSP target_sp(m_thread.CalculateTarget());
 
@@ -57,7 +58,10 @@
   m_return_addr = return_frame_sp->GetStackID().GetPC();
   Breakpoint *return_bp =
   target_sp->CreateBreakpoint(m_return_addr, true, false).get();
+
   if (return_bp != nullptr) {
+if (return_bp->IsHardware() && !return_bp->HasResolvedLocations())
+  m_could_not_resolve_hw_bp = true;
 return_bp->SetThreadID(thread_id);
 m_return_bp_id = return_bp->GetID();
 return_bp->SetBreakpointKind("until-return-backstop");
@@ -97,6 +101,7 @@
 }
   }
   m_until_points.clear();
+  m_could_not_resolve_hw_bp = false;
 }
 
 void ThreadPlanStepUntil::GetDescription(Stream *s,
@@ -127,9 +132,16 @@
 }
 
 bool ThreadPlanStepUntil::ValidatePlan(Stream *error) {
-  if (m_return_bp_id == LLDB_INVALID_BREAK_ID)
+  if (m_could_not_resolve_hw_bp) {
+if (error)
+  error->PutCString(
+  "Could not create hardware breakpoint for thread plan.");
+return false;
+  } else if (m_return_bp_id == LLDB_INVALID_BREAK_ID) {
+if (error)
+  error->PutCString("Could not create return breakpoint.");
 return false;
-  else {
+  } else {
 until_collection::iterator pos, end = m_until_points.end();
 for (pos = m_until_points.begin(); pos != end; pos++) {
   if (!LLDB_BREAK_ID_IS_VALID((*pos).second))
Index: source/Target/ThreadPlanStepThrough.cpp
===
--- source/Target/ThreadPlanStepThrough.cpp
+++ source/Target/ThreadPlanStepThrough.cpp
@@ -40,7 +40,7 @@
  eVoteNoOpinion, eVoteNoOpinion),
   m_start_address(0), m_backstop_bkpt_id(LLDB_INVALID_BREAK_ID),
   m_backstop_addr(LLDB_INVALID_ADDRESS), m_return_stack_id(m_stack_id),
-  m_stop_others(stop_others) {
+  m_stop_others(stop_others), m_could_not_resolve_hw_bp(false) {
   LookForPlanToStepThroughFromCurrentPC();
 
   // If we don't get a valid step through plan, don't bother to set up a
@@ -62,7 +62,10 @@
   ->GetTarget()