From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1504 invoked by alias); 11 Apr 2012 09:07:27 -0000 Received: (qmail 1489 invoked by uid 22791); 11 Apr 2012 09:07:24 -0000 X-SWARE-Spam-Status: No, hits=-3.3 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,TW_BL,TW_XZ X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 11 Apr 2012 09:07:11 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1SHtWA-0000Xd-Fu from Hui_Zhu@mentor.com for gdb-patches@sourceware.org; Wed, 11 Apr 2012 02:07:10 -0700 Received: from SVR-ORW-FEM-02.mgc.mentorg.com ([147.34.96.206]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Wed, 11 Apr 2012 02:06:54 -0700 Received: from [127.0.0.1] (147.34.91.1) by svr-orw-fem-02.mgc.mentorg.com (147.34.96.168) with Microsoft SMTP Server id 14.1.289.1; Wed, 11 Apr 2012 02:07:09 -0700 Message-ID: <4F8549BC.3050003@mentor.com> Date: Wed, 11 Apr 2012 09:32:00 -0000 From: Hui Zhu User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:11.0) Gecko/20120329 Thunderbird/11.0.1 MIME-Version: 1.0 To: Subject: [PATCH] breakpoint remove fail handle bug fix Content-Type: multipart/mixed; boundary="------------060900040802040904000802" 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: 2012-04/txt/msg00235.txt.bz2 --------------060900040802040904000802 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Content-length: 3488 Hi, I sent this patch with the autoload-breakpoints patches because I found this issue when I test the autoload-breakpoints. But I think sent it together with a feature patch list is not a good idea. So I move it out for review. This is bug was found when I when I test autoload-breakpint code. And I found that it affect target-async too. It can be reproduce: (gdb) set target-async on (gdb) start Temporary breakpoint 1 at 0x4004c8: file 1.c, line 4. Starting program: /home/teawater/tmp/a.out Temporary breakpoint 1, main () at 1.c:4 4 sleep (20); (gdb) disassemble Dump of assembler code for function main: 0x00000000004004c4 <+0>: push %rbp 0x00000000004004c5 <+1>: mov %rsp,%rbp => 0x00000000004004c8 <+4>: mov $0x14,%edi 0x00000000004004cd <+9>: mov $0x0,%eax 0x00000000004004d2 <+14>: callq 0x4003d0 0x00000000004004d7 <+19>: mov $0x0,%eax 0x00000000004004dc <+24>: pop %rbp 0x00000000004004dd <+25>: retq End of assembler dump. (gdb) list 1 int 2 main() 3 { 4 sleep (20); 5 6 return 0; 7 } 8 (gdb) b 6 Breakpoint 2 at 0x4004d7: file 1.c, line 6. (gdb) c& Continuing. (gdb) d Delete all breakpoints? (y or n) y warning: Error removing breakpoint 2 (gdb) Program received signal SIGTRAP, Trace/breakpoint trap. 0x00000000004004d8 in main () at 1.c:6 6 return 0; c Continuing. Program received signal SIGSEGV, Segmentation fault. 0x00000000004004d8 in main () at 1.c:6 6 return 0; (gdb) info reg pc pc: 0x4004d8 (gdb) disassemble main Dump of assembler code for function main: 0x00000000004004c4 <+0>: push %rbp 0x00000000004004c5 <+1>: mov %rsp,%rbp 0x00000000004004c8 <+4>: mov $0x14,%edi 0x00000000004004cd <+9>: mov $0x0,%eax 0x00000000004004d2 <+14>: callq 0x4003d0 0x00000000004004d7 <+19>: int3 => 0x00000000004004d8 <+20>: add %al,(%rax) 0x00000000004004da <+22>: add %al,(%rax) 0x00000000004004dc <+24>: pop %rbp 0x00000000004004dd <+25>: retq End of assembler dump. This because is when GDB got fail when it remove the breakpoint, it give up the control of this breakpoint. There are 2 issues about it: 1. When the GDB stop, this breakpoint is not be removed. 2. If inferior is stoped by this breakpoint, adjust_pc_after_break didn't know this stop is beauce the breakpoint, so the pc address will not be adjust to the right value. I add a list called bp_location_remove_fail_chain, when GDB got fail with remove a breakpoint, add it to this list. When adjust_pc_after_break check if this address is the breakpint, check this list too. And when gdb remve all breakpoints, try remove breakpint in this list. Thanks, Hui 2012-04-11 Hui Zhu * breakpoint.c (ALL_BP_LOCATION_REMOV_FAIL): New macro. (ALL_BP_LOCATION_REMOV_FAIL_SAFE): New macro. (bp_location_remove_fail_chain_inserted_here_p): New function. (bp_location_remove_fail_chain_insert): New function. (bp_location_remove_fail_chain_remove): New function. (remove_breakpoints): Call remove_breakpoint with the bp_locations inside the ALL_BP_LOCATION_REMOV_FAIL_SAFE. (software_breakpoint_inserted_here_p): Call bp_location_remove_fail_chain_inserted_here_p. (update_global_location_list): Call bp_location_remove_fail_chain_insert. * breakpoint.h (bp_location_remove_fail_chain_remove): New extern. * target.c (target_kill): Call bp_location_remove_fail_chain_remove. (target_detach): Ditto. (target_disconnect): Ditto. --------------060900040802040904000802 Content-Type: text/plain; charset="us-ascii"; name="break-remove-error-change.txt" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="break-remove-error-change.txt" Content-length: 5910 --- breakpoint.c | 132 +++++++++++++++++++++++++++++++++++++++++++++++++++++++---- breakpoint.h | 2 target.c | 15 +++++- 3 files changed, 139 insertions(+), 10 deletions(-) --- a/breakpoint.c +++ b/breakpoint.c @@ -569,6 +569,100 @@ static struct cmd_list_element *breakpoi static struct cmd_list_element *breakpoint_show_cmdlist; struct cmd_list_element *save_cmdlist; +/* Chains of bp_location that have remove issue. */ +static struct bp_location *bp_location_remove_fail_chain = NULL; + +#define ALL_BP_LOCATION_REMOV_FAIL(BP) \ + for (BP = bp_location_remove_fail_chain; \ + BP; \ + BP = BP->next) + +#define ALL_BP_LOCATION_REMOV_FAIL_SAFE(BP,TMP) \ + for (BP = bp_location_remove_fail_chain; \ + BP ? (TMP=BP->next, 1): 0; \ + BP = TMP) + +static int +bp_location_remove_fail_chain_inserted_here_p (struct address_space *aspace, + CORE_ADDR pc) +{ + struct bp_location *bl; + + ALL_BP_LOCATION_REMOV_FAIL (bl) + { + if (bl->loc_type != bp_loc_software_breakpoint) + continue; + + if (bl->inserted + && breakpoint_address_match (bl->pspace->aspace, bl->address, + aspace, pc)) + { + if (overlay_debugging + && section_is_overlay (bl->section) + && !section_is_mapped (bl->section)) + continue; /* unmapped overlay -- can't be a match */ + else + return 1; + } + } + + return 0; +} + +static void +bp_location_remove_fail_chain_insert (struct bp_location *bl) +{ + struct bp_location *bltmp; + struct breakpoint *b; + + if (!bl->owner) + return; + + ALL_BP_LOCATION_REMOV_FAIL (bltmp) + { + if (bltmp == bl) + return; + } + + b = (struct breakpoint *) xzalloc (sizeof (struct breakpoint)); + *b = *bl->owner; + bl->owner = b; + + incref_bp_location (bl); + bl->next = bp_location_remove_fail_chain; + bp_location_remove_fail_chain = bl; +} + +void +bp_location_remove_fail_chain_remove (struct bp_location *bl) +{ + struct bp_location *bltmp, *bltmp2, *blprev = NULL; + + ALL_BP_LOCATION_REMOV_FAIL_SAFE (bltmp, bltmp2) + { + if (!bl || bltmp == bl) + { + if (bl) + { + if (blprev) + blprev->next = bl->next; + else + bp_location_remove_fail_chain = bl->next; + } + xfree (bltmp->owner); + bltmp->owner = NULL; + decref_bp_location (&bltmp); + if (bl) + break; + } + else + blprev = bltmp; + } + + if (!bl) + bp_location_remove_fail_chain = NULL; +} + /* Return whether a breakpoint is an active enabled breakpoint. */ static int breakpoint_enabled (struct breakpoint *b) @@ -2594,7 +2688,7 @@ You may have requested too many hardware int remove_breakpoints (void) { - struct bp_location *bl, **blp_tmp; + struct bp_location *bl, *bltmp, **blp_tmp; int val = 0; ALL_BP_LOCATIONS (bl, blp_tmp) @@ -2602,6 +2696,20 @@ remove_breakpoints (void) if (bl->inserted && !is_tracepoint (bl->owner)) val |= remove_breakpoint (bl, mark_uninserted); } + + ALL_BP_LOCATION_REMOV_FAIL_SAFE (bl, bltmp) + { + volatile struct gdb_exception e; + int ret; + + TRY_CATCH (e, RETURN_MASK_ERROR) + { + ret = remove_breakpoint (bl, mark_uninserted); + } + if (!ret && e.reason >= 0) + bp_location_remove_fail_chain_remove (bl); + } + return val; } @@ -3536,6 +3644,9 @@ software_breakpoint_inserted_here_p (str if (single_step_breakpoint_inserted_here_p (aspace, pc)) return 1; + if (bp_location_remove_fail_chain_inserted_here_p (aspace, pc)) + return 1; + return 0; } @@ -11650,7 +11761,14 @@ update_global_location_list (int should_ if (!keep_in_target) { - if (remove_breakpoint (old_loc, mark_uninserted)) + volatile struct gdb_exception e; + int ret; + + TRY_CATCH (e, RETURN_MASK_ERROR) + { + ret = remove_breakpoint (old_loc, mark_uninserted); + } + if (ret || e.reason < 0) { /* This is just about all we can do. We could keep this location on the global list, and try to @@ -11663,8 +11781,11 @@ update_global_location_list (int should_ printf_filtered (_("warning: Error removing " "breakpoint %d\n"), old_loc->owner->number); + removed = 0; + bp_location_remove_fail_chain_insert (old_loc); } - removed = 1; + else + removed = 1; } } @@ -11725,10 +11846,7 @@ update_global_location_list (int should_ VEC_safe_push (bp_location_p, moribund_locations, old_loc); } else - { - old_loc->owner = NULL; - decref_bp_location (&old_loc); - } + decref_bp_location (&old_loc); } } --- a/breakpoint.h +++ b/breakpoint.h @@ -1480,4 +1480,6 @@ extern struct gdbarch *get_sal_arch (str extern void handle_solib_event (void); +extern void bp_location_remove_fail_chain_remove (struct bp_location *bl); + #endif /* !defined (BREAKPOINT_H) */ --- a/target.c +++ b/target.c @@ -454,6 +454,9 @@ target_kill (void) fprintf_unfiltered (gdb_stdlog, "target_kill ()\n"); t->to_kill (t); + + bp_location_remove_fail_chain_remove (NULL); + return; } @@ -2568,9 +2571,13 @@ target_detach (char *args, int from_tty) disconnection from the target. */ ; else - /* If we're in breakpoints-always-inserted mode, have to remove - them before detaching. */ - remove_breakpoints_pid (PIDGET (inferior_ptid)); + { + /* If we're in breakpoints-always-inserted mode, have to remove + them before detaching. */ + remove_breakpoints_pid (PIDGET (inferior_ptid)); + + bp_location_remove_fail_chain_remove (NULL); + } prepare_for_detach (); @@ -2599,6 +2606,8 @@ target_disconnect (char *args, int from_ disconnecting. */ remove_breakpoints (); + bp_location_remove_fail_chain_remove (NULL); + for (t = current_target.beneath; t != NULL; t = t->beneath) if (t->to_disconnect != NULL) { --------------060900040802040904000802--