Update detect_breakpoint() to better handle single stepping

A stopped thread should sometimes restart, even if a breakpoint
interrupted a single step.  detect_breakpoint() and its interaction with
wait_thread() is updated such that any restart during a single step
scenario properly "requeues" the thread's run intent, by preserving the
doing/donext flags.

Furthermore, detect_breakpoint() ditches its call to get_breakpoint()
and considers any and all breakpoints impacting the current thread PC,
since they may each suggest different restarting requirements.  In
effect, the thread will only remain stopped if at least one relevant
breakpoint allows it to do so.

This reverts commit 5589a9e3afd5 ("Ignore breakpoints during
singlestep").

Signed-off-by: Matt Hunter <m@lfurio.us>

1 files changed, 30 insertions, 18 deletions

diff --git a/debugger.c b/debugger.c
index 06447c3..173acbd 100644
--- a/debugger.c
+++ b/debugger.c
@@ -109,8 +109,9 @@ static void uninstall_breakpoints(struct thread *th) {
 }
 
 static int detect_breakpoint(struct thread *th, int *restart) {
-    int ret = 0;
-    *restart = 0;
+    int is_bp = 0;      /* at least 1 effective breakpoint at this PC */
+    int is_user = 0;    /* at least 1 user-defined bp at this PC */
+    *restart = 0;       /* at least 1 bp which must stop at this PC */
 
     /* Hack: Need to manually fetch registers here, since capture_state() has
      * not yet run for this stop.  It is not guaranteed that we even want to
@@ -123,8 +124,27 @@ static int detect_breakpoint(struct thread *th, int *restart) {
     architecture_info(&archinfo, &ivregs);
 
     unsigned long breakpt_address = archinfo.progmctr - archinfo.bp_adjust;
-    struct breakpoint *b = get_breakpoint(th->proc, breakpt_address);
-    if (b && b->installed && th->doing != PTRACE_SINGLESTEP) {
+
+    struct list *breaks = &th->proc->breakpoints;
+    for (struct breakpoint *b = breaks->head; b != breaks->end; b = b->next) {
+        if (b->address == breakpt_address) {
+            if (b->installed /* && th->doing != PTRACE_SINGLESTEP*/) {
+                /* todo - issues with singlestep? */
+                /* todo - put conditional guard around `hits++` */
+                is_bp = 1;
+                b->hits++;
+                is_user |= b->user;
+
+                if ((b->stack == 0 || b->stack == archinfo.stackptr)
+                 && (b->tid == 0 || b->tid == th->id)
+                 && (b->enabled)) {
+                    *restart = 1;
+                }
+            }
+        }
+    }
+
+    if (is_bp) {
         /* restore actual program counter to breakpoint address */
         if (ivregs.iov_len == sizeof(struct user_regs_32)) {
             regs.PROGMCTR_32 = breakpt_address;
@@ -132,20 +152,10 @@ static int detect_breakpoint(struct thread *th, int *restart) {
             regs.PROGMCTR_64 = breakpt_address;
         }
         ptrace(PTRACE_SETREGSET, th->id, NT_PRSTATUS, &ivregs);
-
-        b->hits++; /* todo: consider whether this is firing too much */
-        ret = b->user;
-
-        if (b->stack != 0 && b->stack != archinfo.stackptr) {
-            *restart = 1;
-        } else if (b->tid != 0 && b->tid != th->id) {
-            *restart = 1;
-        } else if (!b->enabled) {
-            *restart = 1;
-        }
     }
 
-    return ret;
+    *restart = (is_bp ? !*restart : 0);
+    return is_user;
 }
 
 static void free_states(struct thread *th) {
@@ -441,8 +451,10 @@ static int wait_thread(struct thread *th) {
                 strcpy(th->status, (bp ? "BREAKPOINT" : "STEP"));
 
                 if (restart) {
-                    th->donext = th->doing;
-                    th->doing = (th->doing ? PTRACE_SINGLESTEP : 0);
+                    if (th->doing != PTRACE_SINGLESTEP) {
+                        th->donext = th->doing;
+                        th->doing = (th->doing ? PTRACE_SINGLESTEP : 0);
+                    }
                 } else {
                     th->doing = th->donext;
                     th->donext = 0;