From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26754 invoked by alias); 6 Apr 2002 02:10:20 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 26742 invoked from network); 6 Apr 2002 02:10:18 -0000 Received: from unknown (HELO cygnus.com) (205.180.230.5) by sources.redhat.com with SMTP; 6 Apr 2002 02:10:18 -0000 Received: from redhat.com (notinuse.cygnus.com [205.180.231.12]) by runyon.cygnus.com (8.8.7-cygnus/8.8.7) with ESMTP id SAA11600; Fri, 5 Apr 2002 18:06:34 -0800 (PST) Message-ID: <3CAE5578.10B51383@redhat.com> Date: Fri, 05 Apr 2002 18:10:00 -0000 From: Michael Snyder Organization: Red Hat, Inc. X-Accept-Language: en MIME-Version: 1.0 To: "Martin M. Hunt" CC: gdb-patches@sources.redhat.com, eliz@is.elta.co.il Subject: Re: [RFA] breakpoints.c clear_command fix References: <200203270001.g2R01sJ08019@localhost.localdomain> Content-Type: multipart/mixed; boundary="------------CC21840F94FC53E5AC8F156F" X-SW-Source: 2002-04/txt/msg00207.txt.bz2 This is a multi-part message in MIME format. --------------CC21840F94FC53E5AC8F156F Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Content-length: 1131 "Martin M. Hunt" wrote: > > The clear command improperly detects overlays and fails > to clear breakpoints if overlays are not disabled. > > Tested with linux-x-mips (overlays enabled) and linux x86 native. Martin, you really made me think with this one. Sorry it took so long. I had to go back eleven years in the code base to understand what this code was trying to do -- which made me realize that it's painfully obsolete. It has two inner loops with identical control conditions (except that they've gotten out of sync), just because they didn't have ALL_BREAKPOINTS_SAFE when this code was written. So I rewrote the whole damn function. ;-) If you'd like to check the attached, to make sure that it preserves the intent of your change? I take it your intentions were: 1) Make sure that the two inner loops have the same control conditions. Instead of that, I combined them. Having two was just bad, it allowed them to get out of sync in the first place. 2) Allowing an overlay breakpoint to be cleared if overlay debugging is disabled. I modified that part of your change slightly. --------------CC21840F94FC53E5AC8F156F Content-Type: text/plain; charset=us-ascii; name="clear.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="clear.patch" Content-length: 7192 2002-04-05 Michael Snyder * breakpoint.c (clear_command): Rewrite middle section to combine two loops with identical control conditions. Add a cleanup to eliminate a memory leak. Index: breakpoint.c =================================================================== RCS file: /cvs/src/src/gdb/breakpoint.c,v retrieving revision 1.70 diff -p -r1.70 breakpoint.c *** breakpoint.c 2002/04/05 19:16:15 1.70 --- breakpoint.c 2002/04/06 01:47:45 *************** tcatch_command (char *arg, int from_tty) *** 6442,6456 **** catch_command_1 (arg, 1, from_tty); } static void clear_command (char *arg, int from_tty) { ! register struct breakpoint *b, *b1; int default_match; struct symtabs_and_lines sals; struct symtab_and_line sal; - register struct breakpoint *found; int i; if (arg) --- 6442,6456 ---- catch_command_1 (arg, 1, from_tty); } + /* Delete breakpoints by address or line. */ static void clear_command (char *arg, int from_tty) { ! struct breakpoint *b, *tmp, *prev, *found; int default_match; struct symtabs_and_lines sals; struct symtab_and_line sal; int i; if (arg) *************** clear_command (char *arg, int from_tty) *** 6462,6467 **** --- 6462,6468 ---- { sals.sals = (struct symtab_and_line *) xmalloc (sizeof (struct symtab_and_line)); + make_cleanup (xfree, sals.sals); INIT_SAL (&sal); /* initialize to zeroes */ sal.line = default_breakpoint_line; sal.symtab = default_breakpoint_symtab; *************** clear_command (char *arg, int from_tty) *** 6476,6488 **** } /* For each line spec given, delete bps which correspond ! to it. We do this in two loops: the first loop looks at ! the initial bp(s) in the chain which should be deleted, ! the second goes down the rest of the chain looking ahead ! one so it can take those bps off the chain without messing ! up the chain. */ ! for (i = 0; i < sals.nelts; i++) { /* If exact pc given, clear bpts at that pc. --- 6477,6487 ---- } /* For each line spec given, delete bps which correspond ! to it. Do it in two passes, solely to preserve the current ! behavior that from_tty is forced true if we delete more than ! one breakpoint. */ ! found = NULL; for (i = 0; i < sals.nelts; i++) { /* If exact pc given, clear bpts at that pc. *************** clear_command (char *arg, int from_tty) *** 6498,6578 **** 1 0 */ sal = sals.sals[i]; ! found = (struct breakpoint *) 0; ! ! ! while (breakpoint_chain ! /* Why don't we check here that this is not ! a watchpoint, etc., as we do below? ! I can't make it fail, but don't know ! what's stopping the failure: a watchpoint ! of the same address as "sal.pc" should ! wind up being deleted. */ ! ! && (((sal.pc && (breakpoint_chain->address == sal.pc)) ! && (!overlay_debugging ! || breakpoint_chain->section == sal.section)) ! || ((default_match || (0 == sal.pc)) ! && breakpoint_chain->source_file != NULL ! && sal.symtab != NULL ! && STREQ (breakpoint_chain->source_file, sal.symtab->filename) ! && breakpoint_chain->line_number == sal.line))) ! ! { ! b1 = breakpoint_chain; ! breakpoint_chain = b1->next; ! b1->next = found; ! found = b1; ! } ! ! ALL_BREAKPOINTS (b) ! while (b->next ! && b->next->type != bp_none ! && b->next->type != bp_watchpoint ! && b->next->type != bp_hardware_watchpoint ! && b->next->type != bp_read_watchpoint ! && b->next->type != bp_access_watchpoint ! && (((sal.pc && (b->next->address == sal.pc)) ! && (!overlay_debugging || b->next->section == sal.section)) ! || ((default_match || (0 == sal.pc)) ! && b->next->source_file != NULL ! && sal.symtab != NULL ! && STREQ (b->next->source_file, sal.symtab->filename) ! && b->next->line_number == sal.line))) ! ! ! { ! b1 = b->next; ! b->next = b1->next; ! b1->next = found; ! found = b1; ! } ! if (found == 0) { ! if (arg) ! error ("No breakpoint at %s.", arg); else ! error ("No breakpoint at this line."); } ! if (found->next) ! from_tty = 1; /* Always report if deleted more than one */ ! if (from_tty) ! printf_unfiltered ("Deleted breakpoint%s ", found->next ? "s" : ""); ! breakpoints_changed (); ! while (found) ! { ! if (from_tty) ! printf_unfiltered ("%d ", found->number); ! b1 = found->next; ! delete_breakpoint (found); ! found = b1; ! } if (from_tty) ! putchar_unfiltered ('\n'); } ! xfree (sals.sals); } /* Delete breakpoint in BS if they are `delete' breakpoints and --- 6497,6571 ---- 1 0 */ sal = sals.sals[i]; ! prev = NULL; ! /* Find all matching breakpoints, remove them from the ! breakpoint chain, and add them to the 'found' chain. */ ! ALL_BREAKPOINTS_SAFE (b, tmp) { ! /* Are we going to delete b? */ ! if (b->type != bp_none ! && b->type != bp_watchpoint ! && b->type != bp_hardware_watchpoint ! && b->type != bp_read_watchpoint ! && b->type != bp_access_watchpoint ! /* Not if b is a watchpoint of any sort... */ ! && (((sal.pc && (b->address == sal.pc)) ! && (!section_is_overlay (b->section) ! || b->section == sal.section)) ! /* Yes, if sal.pc matches b (modulo overlays). */ ! || ((default_match || (0 == sal.pc)) ! && b->source_file != NULL ! && sal.symtab != NULL ! && STREQ (b->source_file, sal.symtab->filename) ! && b->line_number == sal.line))) ! /* Yes, if sal source file and line matches b. */ ! { ! /* Remove it from breakpoint_chain... */ ! if (b == breakpoint_chain) ! { ! /* b is at the head of the list */ ! breakpoint_chain = b->next; ! } ! else ! { ! prev->next = b->next; ! } ! /* And add it to 'found' chain. */ ! b->next = found; ! found = b; ! } else ! { ! /* Keep b, and keep a pointer to it. */ ! prev = b; ! } } + } + /* Now go thru the 'found' chain and delete them. */ + if (found == 0) + { + if (arg) + error ("No breakpoint at %s.", arg); + else + error ("No breakpoint at this line."); + } ! if (found->next) ! from_tty = 1; /* Always report if deleted more than one */ ! if (from_tty) ! printf_unfiltered ("Deleted breakpoint%s ", found->next ? "s" : ""); ! breakpoints_changed (); ! while (found) ! { if (from_tty) ! printf_unfiltered ("%d ", found->number); ! tmp = found->next; ! delete_breakpoint (found); ! found = tmp; } ! if (from_tty) ! putchar_unfiltered ('\n'); } /* Delete breakpoint in BS if they are `delete' breakpoints and --------------CC21840F94FC53E5AC8F156F--