This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: Teach gdbserver to step over internal breakpoints
On Wednesday 24 March 2010 12:13:31, Pedro Alves wrote:
> On Wednesday 24 March 2010 00:05:38, Pedro Alves wrote:
> > I've now tested this on arm-none-linux-gnueabi,
> > and there were no regressions.
>
> I busted this. I forgot to force a thread event
> breakpoint to exercise the code paths on ARM. I redid the testing
> overnight, and it revealed a bunch of regressions; it turns out
> I trimmed the original patch too much --- there are some needed
> missing bits I left behind. I'm working on fixing this.
Here's one of the bits. The first issue one trips on with
current baseline, is that delete_breakpoint never worked with
more than one breakpoint in the breakpoint list correctly;
it can easily loop forever, and in fact, it's what was happening
on my testing:
delete_breakpoint:
while (cur->next)
{
if (cur->next == bp)
{
cur->next = bp->next;
(*the_target->write_memory) (bp->pc, bp->old_data,
breakpoint_len);
free (bp);
return;
}
}
The other issue is that `delete_reinsert_breakpoints' was never
deleting anything, as it was removing the breakpoint from the
list before calling delete_breakpoint. Then, delete_breakpoint
would not find the breakpoint to delete in the breakpoint
list. :-) Also, we now need to distinguish the types of
breakpoints. Presently, "reinsert breakpoints", something akin
of software single-step breakpoints, and "other breakpoints",
where thread event breakpoints and in the future tracepoints
will go to. There will be yet another type in the future
"gdb breakpoints".
This fixes most of the regressions forcing a thread event
breakpoint on ARM revealed. The next patch will fix the
rest. Also tested on x86_64, with and without a thread
event breakpoint, and also with a had that makes x86-64
use reinsert breakpoints. I'll show that for the archives
shortly.
Applied.
--
Pedro Alves
2010-03-24 Pedro Alves <pedro@codesourcery.com>
gdb/gdbserver/
* mem-break.c (enum bkpt_type): New.
(struct breakpoint): New field `type'.
(set_breakpoint_at): Change return type to struct breakpoint
pointer. Set type to `other_breakpoint' by default.
(delete_breakpoint): Rewrite, supporting more than one breakpoint
in the breakpoint list.
(delete_reinsert_breakpoints): Only delete reinsert breakpoints.
(reinsert_breakpoint): Rename to ...
(reinsert_raw_breakpoint): ... this.
(reinsert_breakpoints_at): Adjust.
* mem-break.h (struct breakpoint): Declare.
(set_breakpoint_at): Change return type to struct breakpoint
pointer.
---
gdb/gdbserver/mem-break.c | 85 +++++++++++++++++++++++++++++++++-------------
gdb/gdbserver/mem-break.h | 5 +-
2 files changed, 64 insertions(+), 26 deletions(-)
Index: src/gdb/gdbserver/mem-break.c
===================================================================
--- src.orig/gdb/gdbserver/mem-break.c 2010-03-24 11:32:21.000000000 +0000
+++ src/gdb/gdbserver/mem-break.c 2010-03-24 11:59:00.000000000 +0000
@@ -26,6 +26,17 @@ int breakpoint_len;
#define MAX_BREAKPOINT_LEN 8
+/* The type of a breakpoint. */
+enum bkpt_type
+ {
+ /* A basic-software-single-step breakpoint. */
+ reinsert_breakpoint,
+
+ /* Any other breakpoint type that doesn't require specific
+ treatment goes here. E.g., an event breakpoint. */
+ other_breakpoint,
+ };
+
struct breakpoint
{
struct breakpoint *next;
@@ -36,11 +47,16 @@ struct breakpoint
inferior. */
int inserted;
+ /* The breakpoint's type. */
+ enum bkpt_type type;
+
/* Function to call when we hit this breakpoint. If it returns 1,
the breakpoint shall be deleted; 0, it will be left inserted. */
int (*handler) (CORE_ADDR);
};
+static void uninsert_breakpoint (struct breakpoint *bp);
+
static struct breakpoint *
set_raw_breakpoint_at (CORE_ADDR where)
{
@@ -86,7 +102,7 @@ set_raw_breakpoint_at (CORE_ADDR where)
return bp;
}
-void
+struct breakpoint *
set_breakpoint_at (CORE_ADDR where, int (*handler) (CORE_ADDR))
{
struct process_info *proc = current_process ();
@@ -97,42 +113,45 @@ set_breakpoint_at (CORE_ADDR where, int
if (bp == NULL)
{
/* warn? */
- return;
+ return NULL;
}
bp = xcalloc (1, sizeof (struct breakpoint));
+ bp->type = other_breakpoint;
bp->handler = handler;
bp->next = proc->breakpoints;
proc->breakpoints = bp;
+
+ return bp;
}
static void
-delete_breakpoint (struct breakpoint *bp)
+delete_breakpoint (struct breakpoint *todel)
{
struct process_info *proc = current_process ();
- struct breakpoint *cur;
+ struct breakpoint *bp, **bp_link;
- if (proc->breakpoints == bp)
- {
- proc->breakpoints = bp->next;
- (*the_target->write_memory) (bp->pc, bp->old_data,
- breakpoint_len);
- free (bp);
- return;
- }
- cur = proc->breakpoints;
- while (cur->next)
+ bp = proc->breakpoints;
+ bp_link = &proc->breakpoints;
+
+ while (bp)
{
- if (cur->next == bp)
+ if (bp == todel)
{
- cur->next = bp->next;
- (*the_target->write_memory) (bp->pc, bp->old_data,
- breakpoint_len);
+ *bp_link = bp->next;
+
+ uninsert_breakpoint (bp);
free (bp);
return;
}
+ else
+ {
+ bp_link = &bp->next;
+ bp = *bp_link;
+ }
}
+
warning ("Could not find breakpoint in list.");
}
@@ -163,7 +182,11 @@ delete_breakpoint_at (CORE_ADDR addr)
void
set_reinsert_breakpoint (CORE_ADDR stop_at)
{
- set_breakpoint_at (stop_at, NULL);
+ struct breakpoint *bp;
+
+ bp = set_breakpoint_at (stop_at, NULL);
+
+ bp->type = reinsert_breakpoint;
}
void
@@ -177,9 +200,23 @@ delete_reinsert_breakpoints (void)
while (bp)
{
- *bp_link = bp->next;
- delete_breakpoint (bp);
- bp = *bp_link;
+ if (bp->type == reinsert_breakpoint)
+ {
+ *bp_link = bp->next;
+
+ /* If something goes wrong, maybe this is a shared library
+ breakpoint, and the shared library has been unmapped.
+ Assume the breakpoint is gone anyway. */
+ uninsert_breakpoint (bp);
+ free (bp);
+
+ bp = *bp_link;
+ }
+ else
+ {
+ bp_link = &bp->next;
+ bp = *bp_link;
+ }
}
}
@@ -228,7 +265,7 @@ uninsert_breakpoints_at (CORE_ADDR pc)
}
static void
-reinsert_breakpoint (struct breakpoint *bp)
+reinsert_raw_breakpoint (struct breakpoint *bp)
{
int err;
@@ -263,7 +300,7 @@ reinsert_breakpoints_at (CORE_ADDR pc)
return;
}
- reinsert_breakpoint (bp);
+ reinsert_raw_breakpoint (bp);
}
void
Index: src/gdb/gdbserver/mem-break.h
===================================================================
--- src.orig/gdb/gdbserver/mem-break.h 2010-03-24 11:47:23.000000000 +0000
+++ src/gdb/gdbserver/mem-break.h 2010-03-24 11:47:43.000000000 +0000
@@ -23,6 +23,7 @@
#define MEM_BREAK_H
/* Breakpoints are opaque. */
+struct breakpoint;
/* Returns TRUE if breakpoints are supported on this target. */
@@ -41,8 +42,8 @@ int breakpoint_inserted_here (CORE_ADDR
it is hit. HANDLER should return 1 if the breakpoint
should be deleted, 0 otherwise. */
-void set_breakpoint_at (CORE_ADDR where,
- int (*handler) (CORE_ADDR));
+struct breakpoint *set_breakpoint_at (CORE_ADDR where,
+ int (*handler) (CORE_ADDR));
/* Delete a breakpoint previously inserted at ADDR with
set_breakpoint_at. */