From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8831 invoked by alias); 24 Mar 2010 21:22:51 -0000 Received: (qmail 8821 invoked by uid 22791); 24 Mar 2010 21:22:49 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 24 Mar 2010 21:22:44 +0000 Received: (qmail 22661 invoked from network); 24 Mar 2010 21:22:42 -0000 Received: from unknown (HELO orlando.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 24 Mar 2010 21:22:42 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: Teach gdbserver to step over internal breakpoints Date: Wed, 24 Mar 2010 21:22:00 -0000 User-Agent: KMail/1.12.2 (Linux/2.6.31-20-generic; KDE/4.3.2; x86_64; ; ) References: <201003161839.52618.pedro@codesourcery.com> <201003240005.38983.pedro@codesourcery.com> <201003241213.31420.pedro@codesourcery.com> In-Reply-To: <201003241213.31420.pedro@codesourcery.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201003242122.39450.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-03/txt/msg00827.txt.bz2 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 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. */