Change lldb breakpoint and stepping algorithm

cross-project-tests/debuginfo-tests/dexter/dex/debugger/lldb/LLDB.py

@@ -206,6 +206,33 @@ def launch(self, cmdline):
 
  def step(self):
  self._thread.StepInto()
+ stop_reason = self._thread.GetStopReason()
+ # If we (1) completed a step and (2) are sitting at a breakpoint,
+ # but (3) the breakpoint is not reported as the stop reason, then
+ # we'll need to step once more to hit the breakpoint.
+ #
+ # dexter sets breakpoints on every source line, then steps
+ # each source line. Older lldb's would overwrite the stop
+ # reason with "breakpoint hit" when we stopped at a breakpoint,
+ # even if the breakpoint hadn't been exectued yet. One
+ # step per source line, hitting a breakpoint each time.
+ #
+ # But a more accurate behavior is that the step completes
+ # with step-completed stop reason, then when we step again,
+ # we execute the breakpoint and stop (with the pc the same) and
+ # a breakpoint-hit stop reason. So we need to step twice per line.
+ if stop_reason == self._interface.eStopReasonPlanComplete:
+ stepped_to_breakpoint = False
+ pc = self._thread.GetFrameAtIndex(0).GetPC()
+ for bp in self._target.breakpoints:
+ for bploc in bp.locations:
+ if (
+ bploc.IsEnabled()
+ and bploc.GetAddress().GetLoadAddress(self._target) == pc
+ ):
+ stepped_to_breakpoint = True
+ if stepped_to_breakpoint:
+ self._thread.StepInto()
 
  def go(self) -> ReturnCode:
  self._process.Continue()

lldb/include/lldb/Target/Thread.h

@@ -129,6 +129,7 @@ class Thread : public std::enable_shared_from_this<Thread>,
  register_backup_sp; // You need to restore the registers, of course...
  uint32_t current_inlined_depth;
  lldb::addr_t current_inlined_pc;
+ lldb::addr_t stopped_at_unexecuted_bp;
  };
 
  /// Constructor
@@ -378,6 +379,26 @@ class Thread : public std::enable_shared_from_this<Thread>,
 
  virtual void SetQueueLibdispatchQueueAddress(lldb::addr_t dispatch_queue_t) {}
 
+ /// When a thread stops at an enabled BreakpointSite that has not executed,
+ /// the Process plugin should call SetThreadStoppedAtUnexecutedBP(pc).
+ /// If that BreakpointSite was actually triggered (the instruction was
+ /// executed, for a software breakpoint), regardless of whether the
+ /// breakpoint is valid for this thread, SetThreadHitBreakpointSite()
+ /// should be called to record that fact.
+ ///
+ /// Depending on the structure of the Process plugin, it may be easiest
+ /// to call SetThreadStoppedAtUnexecutedBP(pc) unconditionally when at
+ /// a BreakpointSite, and later when it is known that it was triggered,
+ /// SetThreadHitBreakpointSite() can be called. These two methods
+ /// overwrite the same piece of state in the Thread, the last one
+ /// called on a Thread wins.
+ void SetThreadStoppedAtUnexecutedBP(lldb::addr_t pc) {
+ m_stopped_at_unexecuted_bp = pc;
+ }
+ void SetThreadHitBreakpointSite() {
+ m_stopped_at_unexecuted_bp = LLDB_INVALID_ADDRESS;
+ }
+
  /// Whether this Thread already has all the Queue information cached or not
  ///
  /// A Thread may be associated with a libdispatch work Queue at a given
@@ -1364,6 +1385,9 @@ class Thread : public std::enable_shared_from_this<Thread>,
  bool m_should_run_before_public_stop; // If this thread has "stop others" 
  // private work to do, then it will
  // set this.
+ lldb::addr_t m_stopped_at_unexecuted_bp; // Set to the address of a breakpoint
+ // instruction that we have not yet
+ // hit, but will hit when we resume.
  const uint32_t m_index_id; ///< A unique 1 based index assigned to each thread
  /// for easy UI/command line access.
  lldb::RegisterContextSP m_reg_context_sp; ///< The register context for this

lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp

@@ -319,9 +319,12 @@ void NativeProcessFreeBSD::MonitorSIGTRAP(lldb::pid_t pid) {
  info.pl_siginfo.si_addr);
 
  if (thread) {
+ auto &regctx = static_cast<NativeRegisterContextFreeBSD &>(
+ thread->GetRegisterContext());
  auto thread_info =
  m_threads_stepping_with_breakpoint.find(thread->GetID());
- if (thread_info != m_threads_stepping_with_breakpoint.end()) {
+ if (thread_info != m_threads_stepping_with_breakpoint.end() &&
+ thread_info->second == regctx.GetPC()) {
  thread->SetStoppedByTrace();
  Status brkpt_error = RemoveBreakpoint(thread_info->second);
  if (brkpt_error.Fail())

lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp

@@ -829,8 +829,11 @@ void NativeProcessLinux::MonitorBreakpoint(NativeThreadLinux &thread) {
  thread.SetStoppedByBreakpoint();
  FixupBreakpointPCAsNeeded(thread);
 
- if (m_threads_stepping_with_breakpoint.find(thread.GetID()) !=
- m_threads_stepping_with_breakpoint.end())
+ NativeRegisterContextLinux &reg_ctx = thread.GetRegisterContext();
+ auto stepping_with_bp_it =
+ m_threads_stepping_with_breakpoint.find(thread.GetID());
+ if (stepping_with_bp_it != m_threads_stepping_with_breakpoint.end() &&
+ stepping_with_bp_it->second == reg_ctx.GetPC())
  thread.SetStoppedByTrace();
 
  StopRunningThreads(thread.GetID());