[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)

via lldb-commits lldb-commits at lists.llvm.org
Fri Jul 12 12:28:43 PDT 2024


================
@@ -633,171 +613,142 @@ StopInfoSP StopInfoMachException::CreateStopReasonWithMachException(
     }
     break;
 
+  // [exc_type, exc_code, exc_sub_code, exc_sub_sub_code]
+  //
+  // Instruction step:
+  //   [6, 1, 0]
+  //   Intel KDP [6, 3, ??]
+  //   armv7 [6, 0x102, <stop-pc>]  Same as software breakpoint!
+  //
+  // Software breakpoint:
+  //   x86 [6, 2, 0]
+  //   Intel KDP [6, 2, <bp-addr + 1>]
+  //   arm64 [6, 1, <bp-addr>]
+  //   armv7 [6, 0x102, <bp-addr>]  Same as instruction step!
+  //
+  // Hardware breakpoint:
+  //   x86 [6, 1, <bp-addr>, 0]
+  //   x86/Rosetta not implemented, see software breakpoint
+  //   arm64 [6, 1, <bp-addr>]
+  //   armv7 not implemented, see software breakpoint
+  //
+  // Hardware watchpoint:
+  //   x86 [6, 1, <accessed-addr>, 0] (both Intel hw and Rosetta)
+  //   arm64 [6, 0x102, <accessed-addr>, 0]
+  //   armv7 [6, 0x102, <accessed-addr>, 0]
+  //
+  // arm64 BRK instruction (imm arg not reflected in the ME)
+  //   [ 6, 1, <addr-of-BRK-insn>]
+  //
+  // In order of codes mach exceptions:
+  //   [6, 1, 0] - instruction step
+  //   [6, 1, <bp-addr>] - hardware breakpoint or watchpoint
+  //
+  //   [6, 2, 0] - software breakpoint
+  //   [6, 2, <bp-addr + 1>] - software breakpoint
+  //
+  //   [6, 3] - instruction step
+  //
+  //   [6, 0x102, <stop-pc>] armv7 instruction step
+  //   [6, 0x102, <bp-addr>] armv7 software breakpoint
+  //   [6, 0x102, <accessed-addr>, 0] arm64/armv7 watchpoint
   case 6: // EXC_BREAKPOINT
   {
-    bool is_actual_breakpoint = false;
-    bool is_trace_if_actual_breakpoint_missing = false;
-    switch (cpu) {
-    case llvm::Triple::x86:
-    case llvm::Triple::x86_64:
-      if (exc_code == 1) // EXC_I386_SGL
-      {
-        if (!exc_sub_code) {
-          // This looks like a plain trap.
-          // Have to check if there is a breakpoint here as well.  When you
-          // single-step onto a trap, the single step stops you not to trap.
-          // Since we also do that check below, let's just use that logic.
-          is_actual_breakpoint = true;
-          is_trace_if_actual_breakpoint_missing = true;
-        } else {
-          if (StopInfoSP stop_info =
-                  GetStopInfoForHardwareBP(thread, target, exc_data_count,
-                                           exc_sub_code, exc_sub_sub_code))
-            return stop_info;
-        }
-      } else if (exc_code == 2 || // EXC_I386_BPT
-                 exc_code == 3)   // EXC_I386_BPTFLT
-      {
-        // KDP returns EXC_I386_BPTFLT for trace breakpoints
-        if (exc_code == 3)
-          is_trace_if_actual_breakpoint_missing = true;
-
-        is_actual_breakpoint = true;
-        if (!pc_already_adjusted)
-          pc_decrement = 1;
-      }
-      break;
-
-    case llvm::Triple::arm:
-    case llvm::Triple::thumb:
-      if (exc_code == 0x102) // EXC_ARM_DA_DEBUG
-      {
-        // LWP_TODO: We need to find the WatchpointResource that matches
-        // the address, and evaluate its Watchpoints.
-
-        // It's a watchpoint, then, if the exc_sub_code indicates a
-        // known/enabled data break address from our watchpoint list.
-        lldb::WatchpointSP wp_sp;
-        if (target)
-          wp_sp = target->GetWatchpointList().FindByAddress(
-              (lldb::addr_t)exc_sub_code);
-        if (wp_sp && wp_sp->IsEnabled()) {
-          return StopInfo::CreateStopReasonWithWatchpointID(thread,
-                                                            wp_sp->GetID());
-        } else {
-          is_actual_breakpoint = true;
-          is_trace_if_actual_breakpoint_missing = true;
-        }
-      } else if (exc_code == 1) // EXC_ARM_BREAKPOINT
-      {
-        is_actual_breakpoint = true;
-        is_trace_if_actual_breakpoint_missing = true;
-      } else if (exc_code == 0) // FIXME not EXC_ARM_BREAKPOINT but a kernel
-                                // is currently returning this so accept it
-                                // as indicating a breakpoint until the
-                                // kernel is fixed
-      {
-        is_actual_breakpoint = true;
-        is_trace_if_actual_breakpoint_missing = true;
-      }
-      break;
+    bool stopped_by_hitting_breakpoint = false;
+    bool stopped_by_completing_stepi = false;
+    bool stopped_watchpoint = false;
+    std::optional<addr_t> value;
+
+    // exc_code 1
+    if (exc_code == 1 && exc_sub_code == 0)
+      stopped_by_completing_stepi = true;
+    if (exc_code == 1 && exc_sub_code != 0) {
+      stopped_by_hitting_breakpoint = true;
+      stopped_watchpoint = true;
+      value = exc_sub_code;
+    }
 
-    case llvm::Triple::aarch64_32:
-    case llvm::Triple::aarch64: {
-      // xnu describes three things with type EXC_BREAKPOINT:
-      //
-      //   exc_code 0x102 [EXC_ARM_DA_DEBUG], exc_sub_code addr-of-insn
-      //      Watchpoint access.  exc_sub_code is the address of the
-      //      instruction which trigged the watchpoint trap.
-      //      debugserver may add the watchpoint number that was triggered
-      //      in exc_sub_sub_code.
-      //
-      //   exc_code 1 [EXC_ARM_BREAKPOINT], exc_sub_code 0
-      //      Instruction step has completed.
-      //
-      //   exc_code 1 [EXC_ARM_BREAKPOINT], exc_sub_code address-of-instruction
-      //      Software breakpoint instruction executed.
-
-      if (exc_code == 1 && exc_sub_code == 0) // EXC_ARM_BREAKPOINT
-      {
-        // This is hit when we single instruction step aka MDSCR_EL1 SS bit 0
-        // is set
-        is_actual_breakpoint = true;
-        is_trace_if_actual_breakpoint_missing = true;
-        if (thread.GetTemporaryResumeState() != eStateStepping)
-          not_stepping_but_got_singlestep_exception = true;
-      }
-      if (exc_code == 0x102) // EXC_ARM_DA_DEBUG
-      {
-        // LWP_TODO: We need to find the WatchpointResource that matches
-        // the address, and evaluate its Watchpoints.
-
-        // It's a watchpoint, then, if the exc_sub_code indicates a
-        // known/enabled data break address from our watchpoint list.
-        lldb::WatchpointSP wp_sp;
-        if (target)
-          wp_sp = target->GetWatchpointList().FindByAddress(
-              (lldb::addr_t)exc_sub_code);
-        if (wp_sp && wp_sp->IsEnabled()) {
-          return StopInfo::CreateStopReasonWithWatchpointID(thread,
-                                                            wp_sp->GetID());
-        }
-        // EXC_ARM_DA_DEBUG seems to be reused for EXC_BREAKPOINT as well as
-        // EXC_BAD_ACCESS
-        if (thread.GetTemporaryResumeState() == eStateStepping)
-          return StopInfo::CreateStopReasonToTrace(thread);
-      }
-      // It looks like exc_sub_code has the 4 bytes of the instruction that
-      // triggered the exception, i.e. our breakpoint opcode
-      is_actual_breakpoint = exc_code == 1;
-      break;
+    // exc_code 2
+    if (exc_code == 2 && exc_sub_code == 0)
+      stopped_by_hitting_breakpoint = true;
+    if (exc_code == 2 && exc_sub_code != 0) {
+      stopped_by_hitting_breakpoint = true;
+      // Intel KDP software breakpoint
+      if (!pc_already_adjusted)
+        pc_decrement = 1;
     }
 
-    default:
-      break;
+    // exc_code 3
+    if (exc_code == 3)
+      stopped_by_completing_stepi = true;
+
+    // exc_code 0x102
+    if (exc_code == 0x102 && exc_sub_code != 0) {
+      if (cpu == llvm::Triple::arm || cpu == llvm::Triple::thumb) {
+        stopped_by_hitting_breakpoint = true;
+        stopped_by_completing_stepi = true;
+      }
+      stopped_watchpoint = true;
+      value = exc_sub_code;
     }
 
-    if (is_actual_breakpoint) {
-      RegisterContextSP reg_ctx_sp(thread.GetRegisterContext());
-      addr_t pc = reg_ctx_sp->GetPC() - pc_decrement;
+    // Go through the reasons why we stopped, starting
+    // with the easiest to detect unambiguously.  We
+    // may have multiple possible reasons set.
 
-      ProcessSP process_sp(thread.CalculateProcess());
+    if (stopped_by_hitting_breakpoint) {
+      addr_t pc = LLDB_INVALID_ADDRESS;
+      if (reg_ctx_sp)
----------------
jimingham wrote:

I find this logic confusing.  At the start, if you had a register context, you calculated the bp_site_sp from the register context pc, but not using the decrement.  Now you do it again, but prioritizing using the value's version of the pc, and overwriting the bp_site_sp that was found at the beginning of the function.  But if you don't have a value, you either use the breakpoint site you calculated above w/o the decrement if that was found, or look it up with the decrement here.
This at least needs a comment explaining what you are doing.

https://github.com/llvm/llvm-project/pull/96260


More information about the lldb-commits mailing list