From 4c31b8af05f5f388c1eea328a0ec69a6acb3a7f8 Mon Sep 17 00:00:00 2001 From: Matt Hunter Date: Sun, 24 Aug 2025 07:10:14 -0400 Subject: 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 --- debugger.c | 48 ++++++++++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 18 deletions(-) (limited to 'debugger.c') 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; -- cgit v1.2.3