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());