From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4009 invoked by alias); 18 Apr 2011 21:22:50 -0000 Received: (qmail 3987 invoked by uid 22791); 18 Apr 2011 21:22:48 -0000 X-SWARE-Spam-Status: No, hits=-0.5 required=5.0 tests=AWL,BAYES_20,SPF_SOFTFAIL,TW_GJ,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from e1.ny.us.ibm.com (HELO e1.ny.us.ibm.com) (32.97.182.141) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 18 Apr 2011 21:22:32 +0000 Received: from d01relay03.pok.ibm.com (d01relay03.pok.ibm.com [9.56.227.235]) by e1.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id p3ILBrds000827 for ; Mon, 18 Apr 2011 17:11:53 -0400 Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay03.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p3ILMULm339778 for ; Mon, 18 Apr 2011 17:22:30 -0400 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p3ILMUSj007099 for ; Mon, 18 Apr 2011 18:22:30 -0300 Received: from [9.12.229.135] ([9.12.229.135]) by d01av02.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with ESMTP id p3ILMS1Y006655; Mon, 18 Apr 2011 18:22:29 -0300 Subject: [RFA 2/3] Demote to sw watchpoint only in update_watchpoint From: Thiago Jung Bauermann To: Ulrich Weigand Cc: gdb-patches ml In-Reply-To: <201102171440.p1HEeIBJ012343@d06av02.portsmouth.uk.ibm.com> References: <201102171440.p1HEeIBJ012343@d06av02.portsmouth.uk.ibm.com> Content-Type: text/plain; charset="UTF-8" Date: Mon, 18 Apr 2011 21:22:00 -0000 Message-ID: <1303161745.1969.220.camel@hactar> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit 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: 2011-04/txt/msg00292.txt.bz2 Hi, watch_command_1 duplicates logic from update_watchpoint to decide whether the newly created watchpoint should be a software watchpoint or a hardware watchpoint, and also to error out if trying to set up a read or access watchpoint without having debug registers available. I was trying to adapt the logic in both places for masked watchpoints, but it's hard to anticipate in watch_command_1 what update_watchpoint will ultimately do without duplicating a good portion of the latter. The code in watch_command_1 was getting complicated and hard to get right in all situations for something which would be done again later anyway. I decided that it was cleaner if watch_command_1 just delegated everything to update_watchpoint. This patch makes it create a watchpoint, call update_watchpoint and then delete it if some error is hit. The only drawback I can think of is that the aborted watchpoint will "consume" one breakpoint number, and thus GDB will skip one number when creating the next breakpoint/watchpoint. This doesn't seem important to me. Tested without regressions on ppc-linux, ppc64-linux and i386-linux. Ok? -- []'s Thiago Jung Bauermann IBM Linux Technology Center 2011-04-18 Thiago Jung Bauermann * breakpoint.c (update_watchpoint): Change between software and hardware watchpoint for all kinds of watchpoints, not just read/write ones. Determine b->exact value here instead of in watch_command_1. Error out if there are not enough resources for a read or access hardware watchpoint. (watch_command_1): Remove logic of checking whether there are enough resources available, since update_watchpoint will do that work now. Don't set b->exact here. Catch exceptions thrown by update_watchpoint and delete the watchpoint. (can_use_hardware_watchpoint): Remove exact_watchpoints argument. Use target_exact_watchpoints instead. (delete_breakpoint): Add announce argument to control whether observers are notified of the deletion. Update all callers. diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 744057a..2bfdfb0 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -101,7 +101,7 @@ static void clear_command (char *, int); static void catch_command (char *, int); -static int can_use_hardware_watchpoint (struct value *, int); +static int can_use_hardware_watchpoint (struct value *); static void break_command_1 (char *, int, int); @@ -1404,19 +1404,22 @@ update_watchpoint (struct breakpoint *b, int reparse) an ordinary watchpoint depending on the hardware support and free hardware slots. REPARSE is set when the inferior is started. */ - if ((b->type == bp_watchpoint || b->type == bp_hardware_watchpoint) - && reparse) + if (reparse) { int reg_cnt; enum bp_loc_type loc_type; struct bp_location *bl; - reg_cnt = can_use_hardware_watchpoint (val_chain, b->exact); + reg_cnt = can_use_hardware_watchpoint (val_chain); if (reg_cnt) { int i, target_resources_ok, other_type_used; + /* Use an exact watchpoint when there's only one memory region to be + watched, and only one debug register is needed to watch it. */ + b->exact = target_exact_watchpoints && reg_cnt == 1; + /* We need to determine how many resources are already used for all other hardware watchpoints plus this one to see if we still have enough resources to also fit @@ -1424,7 +1427,8 @@ update_watchpoint (struct breakpoint *b, int reparse) hw_watchpoint_used_count call below counts this watchpoint, make sure that it is marked as a hardware watchpoint. */ - b->type = bp_hardware_watchpoint; + if (b->type == bp_watchpoint) + b->type = bp_hardware_watchpoint; i = hw_watchpoint_used_count (bp_hardware_watchpoint, &other_type_used); @@ -1432,8 +1436,22 @@ update_watchpoint (struct breakpoint *b, int reparse) target_resources_ok = target_can_use_hardware_watchpoint (bp_hardware_watchpoint, i, other_type_used); if (target_resources_ok <= 0) - b->type = bp_watchpoint; + { + if (target_resources_ok == 0 + && b->type != bp_hardware_watchpoint) + error (_("Target does not support this type of " + "hardware watchpoint.")); + else if (target_resources_ok < 0 + && b->type != bp_hardware_watchpoint) + error (_("Target can only support one kind " + "of HW watchpoint at a time.")); + else + b->type = bp_watchpoint; + } } + else if (b->type != bp_hardware_watchpoint) + error (_("Expression cannot be implemented with " + "read/access watchpoint.")); else b->type = bp_watchpoint; @@ -1796,7 +1814,7 @@ breakpoint_program_space_exit (struct program_space *pspace) ALL_BREAKPOINTS_SAFE (b, b_temp) { if (b->pspace == pspace) - delete_breakpoint (b); + delete_breakpoint (b, 1); } /* Breakpoints set through other program spaces could have locations @@ -2382,14 +2400,14 @@ update_breakpoints_after_exec (void) /* Solib breakpoints must be explicitly reset after an exec(). */ if (b->type == bp_shlib_event) { - delete_breakpoint (b); + delete_breakpoint (b, 1); continue; } /* JIT breakpoints must be explicitly reset after an exec(). */ if (b->type == bp_jit_event) { - delete_breakpoint (b); + delete_breakpoint (b, 1); continue; } @@ -2399,14 +2417,14 @@ update_breakpoints_after_exec (void) || b->type == bp_longjmp_master || b->type == bp_std_terminate_master || b->type == bp_exception_master) { - delete_breakpoint (b); + delete_breakpoint (b, 1); continue; } /* Step-resume breakpoints are meaningless after an exec(). */ if (b->type == bp_step_resume) { - delete_breakpoint (b); + delete_breakpoint (b, 1); continue; } @@ -2415,7 +2433,7 @@ update_breakpoints_after_exec (void) if (b->type == bp_longjmp || b->type == bp_longjmp_resume || b->type == bp_exception || b->type == bp_exception_resume) { - delete_breakpoint (b); + delete_breakpoint (b, 1); continue; } @@ -2464,7 +2482,7 @@ update_breakpoints_after_exec (void) a.out. */ if (b->addr_string == NULL) { - delete_breakpoint (b); + delete_breakpoint (b, 1); continue; } } @@ -2736,7 +2754,7 @@ breakpoint_init_inferior (enum inf_context context) (gdb) tar rem :9999 # remote Windows gdbserver. */ - delete_breakpoint (b); + delete_breakpoint (b, 1); break; case bp_watchpoint: @@ -2746,7 +2764,7 @@ breakpoint_init_inferior (enum inf_context context) /* Likewise for watchpoints on local expressions. */ if (b->exp_valid_block != NULL) - delete_breakpoint (b); + delete_breakpoint (b, 1); else if (context == inf_starting) { /* Reset val field to force reread of starting value in @@ -5970,7 +5988,7 @@ delete_longjmp_breakpoint (int thread) if (b->type == bp_longjmp || b->type == bp_exception) { if (b->thread == thread) - delete_breakpoint (b); + delete_breakpoint (b, 1); } } @@ -6026,7 +6044,7 @@ delete_std_terminate_breakpoint (void) ALL_BREAKPOINTS_SAFE (b, b_tmp) if (b->type == bp_std_terminate) - delete_breakpoint (b); + delete_breakpoint (b, 1); } struct breakpoint * @@ -6054,7 +6072,7 @@ remove_thread_event_breakpoints (void) ALL_BREAKPOINTS_SAFE (b, b_tmp) if (b->type == bp_thread_event && b->loc->pspace == current_program_space) - delete_breakpoint (b); + delete_breakpoint (b, 1); } struct lang_and_radix @@ -6085,7 +6103,7 @@ remove_jit_event_breakpoints (void) ALL_BREAKPOINTS_SAFE (b, b_tmp) if (b->type == bp_jit_event && b->loc->pspace == current_program_space) - delete_breakpoint (b); + delete_breakpoint (b, 1); } void @@ -6096,7 +6114,7 @@ remove_solib_event_breakpoints (void) ALL_BREAKPOINTS_SAFE (b, b_tmp) if (b->type == bp_shlib_event && b->loc->pspace == current_program_space) - delete_breakpoint (b); + delete_breakpoint (b, 1); } struct breakpoint * @@ -8789,6 +8807,7 @@ static void watch_command_1 (char *arg, int accessflag, int from_tty, int just_location, int internal) { + volatile struct gdb_exception e; struct breakpoint *b, *scope_breakpoint = NULL; struct expression *exp; struct block *exp_valid_block = NULL, *cond_exp_valid_block = NULL; @@ -8800,9 +8819,7 @@ watch_command_1 (char *arg, int accessflag, int from_tty, int toklen; char *cond_start = NULL; char *cond_end = NULL; - int i, other_type_used, target_resources_ok = 0; enum bptype bp_type; - int reg_cnt = 0; int thread = -1; int pc = 0; @@ -8932,28 +8949,6 @@ watch_command_1 (char *arg, int accessflag, int from_tty, else bp_type = bp_hardware_watchpoint; - reg_cnt = can_use_hardware_watchpoint (val, target_exact_watchpoints); - if (reg_cnt == 0 && bp_type != bp_hardware_watchpoint) - error (_("Expression cannot be implemented with read/access watchpoint.")); - if (reg_cnt != 0) - { - i = hw_watchpoint_used_count (bp_type, &other_type_used); - target_resources_ok = - target_can_use_hardware_watchpoint (bp_type, i + reg_cnt, - other_type_used); - if (target_resources_ok == 0 && bp_type != bp_hardware_watchpoint) - error (_("Target does not support this type of hardware watchpoint.")); - - if (target_resources_ok < 0 && bp_type != bp_hardware_watchpoint) - error (_("Target can only support one kind " - "of HW watchpoint at a time.")); - } - - /* Change the type of breakpoint to an ordinary watchpoint if a - hardware watchpoint could not be set. */ - if (!reg_cnt || target_resources_ok <= 0) - bp_type = bp_watchpoint; - frame = block_innermost_frame (exp_valid_block); /* If the expression is "local", then set up a "watchpoint scope" @@ -9022,10 +9017,6 @@ watch_command_1 (char *arg, int accessflag, int from_tty, b->val_valid = 1; b->ops = &watchpoint_breakpoint_ops; - /* Use an exact watchpoint when there's only one memory region to be - watched, and only one debug register is needed to watch it. */ - b->exact = target_exact_watchpoints && reg_cnt == 1; - if (cond_start) b->cond_string = savestring (cond_start, cond_end - cond_start); else @@ -9053,9 +9044,18 @@ watch_command_1 (char *arg, int accessflag, int from_tty, if (!just_location) value_free_to_mark (mark); - /* Finally update the new watchpoint. This creates the locations - that should be inserted. */ - update_watchpoint (b, 1); + TRY_CATCH (e, RETURN_MASK_ALL) + { + /* Finally update the new watchpoint. This creates the locations + that should be inserted. */ + update_watchpoint (b, 1); + } + if (e.reason < 0) + { + delete_breakpoint (b, 0); + throw_exception (e); + } + if (internal) /* Do not mention breakpoints with a negative number, but do notify observers. */ @@ -9066,14 +9066,10 @@ watch_command_1 (char *arg, int accessflag, int from_tty, } /* Return count of debug registers needed to watch the given expression. - If EXACT_WATCHPOINTS is 1, then consider that only the address of - the start of the watched region will be monitored (i.e., all accesses - will be aligned). This uses less debug registers on some targets. - If the watchpoint cannot be handled in hardware return zero. */ static int -can_use_hardware_watchpoint (struct value *v, int exact_watchpoints) +can_use_hardware_watchpoint (struct value *v) { int found_memory_cnt = 0; struct value *head = v; @@ -9129,7 +9125,7 @@ can_use_hardware_watchpoint (struct value *v, int exact_watchpoints) int len; int num_regs; - len = (exact_watchpoints + len = (target_exact_watchpoints && is_scalar_type_recursive (vtype))? 1 : TYPE_LENGTH (value_type (v)); @@ -9245,9 +9241,9 @@ until_break_command_continuation (void *arg) { struct until_break_command_continuation_args *a = arg; - delete_breakpoint (a->breakpoint); + delete_breakpoint (a->breakpoint, 1); if (a->breakpoint2) - delete_breakpoint (a->breakpoint2); + delete_breakpoint (a->breakpoint2, 1); delete_longjmp_breakpoint (a->thread_num); } @@ -9976,7 +9972,7 @@ clear_command (char *arg, int from_tty) { if (from_tty) printf_unfiltered ("%d ", b->number); - delete_breakpoint (b); + delete_breakpoint (b, 1); } if (from_tty) putchar_unfiltered ('\n'); @@ -9995,12 +9991,12 @@ breakpoint_auto_delete (bpstat bs) if (bs->breakpoint_at && bs->breakpoint_at->disposition == disp_del && bs->stop) - delete_breakpoint (bs->breakpoint_at); + delete_breakpoint (bs->breakpoint_at, 1); ALL_BREAKPOINTS_SAFE (b, b_tmp) { if (b->disposition == disp_del_at_next_stop) - delete_breakpoint (b); + delete_breakpoint (b, 1); } } @@ -10441,10 +10437,11 @@ bpstat_remove_breakpoint_callback (struct thread_info *th, void *data) } /* Delete a breakpoint and clean up all traces of it in the data - structures. */ + structures. If ANNOUNCE is 1, then notifies observers that + the breakpoint was deleted. */ void -delete_breakpoint (struct breakpoint *bpt) +delete_breakpoint (struct breakpoint *bpt, int announce) { struct breakpoint *b; @@ -10487,7 +10484,8 @@ delete_breakpoint (struct breakpoint *bpt) bpt->related_breakpoint = bpt; } - observer_notify_breakpoint_deleted (bpt->number); + if (announce) + observer_notify_breakpoint_deleted (bpt->number); if (breakpoint_chain == bpt) breakpoint_chain = bpt->next; @@ -10544,7 +10542,7 @@ delete_breakpoint (struct breakpoint *bpt) static void do_delete_breakpoint_cleanup (void *b) { - delete_breakpoint (b); + delete_breakpoint (b, 1); } struct cleanup * @@ -10559,7 +10557,7 @@ make_cleanup_delete_breakpoint (struct breakpoint *b) static void do_delete_breakpoint (struct breakpoint *b, void *ignore) { - delete_breakpoint (b); + delete_breakpoint (b, 1); } void @@ -10610,7 +10608,7 @@ delete_command (char *arg, int from_tty) && b->type != bp_std_terminate_master && b->type != bp_exception_master && b->number >= 0) - delete_breakpoint (b); + delete_breakpoint (b, 1); } } } @@ -11073,7 +11071,7 @@ breakpoint_re_set_one (void *bint) if (b->addr_string == NULL) { /* Anything without a string can't be re-set. */ - delete_breakpoint (b); + delete_breakpoint (b, 1); return 0; } @@ -11127,7 +11125,7 @@ breakpoint_re_set_one (void *bint) case bp_longjmp_master: case bp_std_terminate_master: case bp_exception_master: - delete_breakpoint (b); + delete_breakpoint (b, 1); break; /* This breakpoint is special, it's set up when the inferior @@ -12114,7 +12112,7 @@ delete_trace_command (char *arg, int from_tty) { if (is_tracepoint (b) && b->number >= 0) - delete_breakpoint (b); + delete_breakpoint (b, 1); } } } diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index 7a9c2d4..bf827e5 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -953,7 +953,7 @@ extern void breakpoint_init_inferior (enum inf_context); extern struct cleanup *make_cleanup_delete_breakpoint (struct breakpoint *); -extern void delete_breakpoint (struct breakpoint *); +extern void delete_breakpoint (struct breakpoint *, int); extern void breakpoint_auto_delete (bpstat); diff --git a/gdb/elfread.c b/gdb/elfread.c index 2d589a4..277a093 100644 --- a/gdb/elfread.c +++ b/gdb/elfread.c @@ -1036,7 +1036,7 @@ elf_gnu_ifunc_resolver_return_stop (struct breakpoint *b) case bp_gnu_ifunc_resolver: break; case bp_gnu_ifunc_resolver_return: - delete_breakpoint (b); + delete_breakpoint (b, 1); break; default: internal_error (__FILE__, __LINE__, diff --git a/gdb/infcmd.c b/gdb/infcmd.c index 3dc13e3..f0e9d96 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -1501,7 +1501,7 @@ finish_command_continuation (void *arg) that the *stopped notification includes the return value. */ if (bs != NULL && tp->control.proceed_to_finish) observer_notify_normal_stop (bs, 1 /* print frame */); - delete_breakpoint (a->breakpoint); + delete_breakpoint (a->breakpoint, 1); delete_longjmp_breakpoint (inferior_thread ()->num); } diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c index 0c21bfc..4450e60 100644 --- a/gdb/python/py-breakpoint.c +++ b/gdb/python/py-breakpoint.c @@ -302,7 +302,7 @@ bppy_delete_breakpoint (PyObject *self, PyObject *args) BPPY_REQUIRE_VALID (self_bp); - delete_breakpoint (self_bp->bp); + delete_breakpoint (self_bp->bp, 1); Py_RETURN_NONE; } diff --git a/gdb/thread.c b/gdb/thread.c index 6ad1807..929c335 100644 --- a/gdb/thread.c +++ b/gdb/thread.c @@ -87,7 +87,7 @@ delete_step_resume_breakpoint (struct thread_info *tp) { if (tp && tp->control.step_resume_breakpoint) { - delete_breakpoint (tp->control.step_resume_breakpoint); + delete_breakpoint (tp->control.step_resume_breakpoint, 1); tp->control.step_resume_breakpoint = NULL; } } @@ -97,7 +97,7 @@ delete_exception_resume_breakpoint (struct thread_info *tp) { if (tp && tp->control.exception_resume_breakpoint) { - delete_breakpoint (tp->control.exception_resume_breakpoint); + delete_breakpoint (tp->control.exception_resume_breakpoint, 1); tp->control.exception_resume_breakpoint = NULL; } }