From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25473 invoked by alias); 12 Nov 2013 04:07:23 -0000 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 Received: (qmail 25463 invoked by uid 89); 12 Nov 2013 04:07:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.9 required=5.0 tests=AWL,BAYES_20,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,RDNS_NONE,SPF_PASS autolearn=no version=3.3.2 X-HELO: mail-pb0-f45.google.com Received: from Unknown (HELO mail-pb0-f45.google.com) (209.85.160.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Tue, 12 Nov 2013 04:06:34 +0000 Received: by mail-pb0-f45.google.com with SMTP id mc8so2469602pbc.18 for ; Mon, 11 Nov 2013 20:06:26 -0800 (PST) X-Received: by 10.66.136.71 with SMTP id py7mr34361033pab.2.1384229186439; Mon, 11 Nov 2013 20:06:26 -0800 (PST) Received: from seba.sebabeach.org.gmail.com (173-13-178-50-sfba.hfc.comcastbusiness.net. [173.13.178.50]) by mx.google.com with ESMTPSA id fk4sm40178069pab.23.2013.11.11.20.06.24 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 11 Nov 2013 20:06:25 -0800 (PST) From: Doug Evans To: gdb-patches@sourceware.org Subject: [PATCH] simplify bpstat_check_breakpoint_conditions Date: Tue, 12 Nov 2013 07:58:00 -0000 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2013-11/txt/msg00307.txt.bz2 Hi. This patch simplifies bpstat_check_breakpoint_conditions a bit. Regression tested on amd64-linux. Ok to check in? [Appended at the end is the same patch with diff's -b option added to make the actual changes easier to see.] 2013-11-11 Doug Evans * breakpoint.c (bpstat_check_breakpoint_conditions): Assert bs->stop != 0 on entry. Update function comment. Simplify early exit for frame mismatch. Reindent rest of function. diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index ffe73fd..36252ee 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -5099,8 +5099,8 @@ bpstat_check_watchpoint (bpstat bs) } } - -/* Check conditions (condition proper, frame, thread and ignore count) +/* For breakpoints that are currently marked as telling gdb to stop, + check conditions (condition proper, frame, thread and ignore count) of breakpoint referred to by BS. If we should not stop for this breakpoint, set BS->stop to 0. */ @@ -5110,6 +5110,10 @@ bpstat_check_breakpoint_conditions (bpstat bs, ptid_t ptid) int thread_id = pid_to_thread_id (ptid); const struct bp_location *bl; struct breakpoint *b; + int value_is_zero = 0; + struct expression *cond; + + gdb_assert (bs->stop); /* BS is built for existing struct breakpoint. */ bl = bs->bp_location_at; @@ -5123,109 +5127,106 @@ bpstat_check_breakpoint_conditions (bpstat bs, ptid_t ptid) if (frame_id_p (b->frame_id) && !frame_id_eq (b->frame_id, get_stack_frame_id (get_current_frame ()))) - bs->stop = 0; - else if (bs->stop) { - int value_is_zero = 0; - struct expression *cond; + bs->stop = 0; + return; + } - /* Evaluate Python breakpoints that have a "stop" - method implemented. */ - if (b->py_bp_object) - bs->stop = gdbpy_should_stop (b->py_bp_object); + /* Evaluate Python breakpoints that have a "stop" method implemented. */ + if (b->py_bp_object) + bs->stop = gdbpy_should_stop (b->py_bp_object); - if (is_watchpoint (b)) - { - struct watchpoint *w = (struct watchpoint *) b; + if (is_watchpoint (b)) + { + struct watchpoint *w = (struct watchpoint *) b; - cond = w->cond_exp; - } + cond = w->cond_exp; + } + else + cond = bl->cond; + + if (cond && b->disposition != disp_del_at_next_stop) + { + int within_current_scope = 1; + struct watchpoint * w; + + /* We use value_mark and value_free_to_mark because it could + be a long time before we return to the command level and + call free_all_values. We can't call free_all_values + because we might be in the middle of evaluating a + function call. */ + struct value *mark = value_mark (); + + if (is_watchpoint (b)) + w = (struct watchpoint *) b; else - cond = bl->cond; + w = NULL; - if (cond && b->disposition != disp_del_at_next_stop) + /* Need to select the frame, with all that implies so that + the conditions will have the right context. Because we + use the frame, we will not see an inlined function's + variables when we arrive at a breakpoint at the start + of the inlined function; the current frame will be the + call site. */ + if (w == NULL || w->cond_exp_valid_block == NULL) + select_frame (get_current_frame ()); + else { - int within_current_scope = 1; - struct watchpoint * w; - - /* We use value_mark and value_free_to_mark because it could - be a long time before we return to the command level and - call free_all_values. We can't call free_all_values - because we might be in the middle of evaluating a - function call. */ - struct value *mark = value_mark (); - - if (is_watchpoint (b)) - w = (struct watchpoint *) b; - else - w = NULL; - - /* Need to select the frame, with all that implies so that - the conditions will have the right context. Because we - use the frame, we will not see an inlined function's - variables when we arrive at a breakpoint at the start - of the inlined function; the current frame will be the - call site. */ - if (w == NULL || w->cond_exp_valid_block == NULL) - select_frame (get_current_frame ()); - else - { - struct frame_info *frame; - - /* For local watchpoint expressions, which particular - instance of a local is being watched matters, so we - keep track of the frame to evaluate the expression - in. To evaluate the condition however, it doesn't - really matter which instantiation of the function - where the condition makes sense triggers the - watchpoint. This allows an expression like "watch - global if q > 10" set in `func', catch writes to - global on all threads that call `func', or catch - writes on all recursive calls of `func' by a single - thread. We simply always evaluate the condition in - the innermost frame that's executing where it makes - sense to evaluate the condition. It seems - intuitive. */ - frame = block_innermost_frame (w->cond_exp_valid_block); - if (frame != NULL) - select_frame (frame); - else - within_current_scope = 0; - } - if (within_current_scope) - value_is_zero - = catch_errors (breakpoint_cond_eval, cond, - "Error in testing breakpoint condition:\n", - RETURN_MASK_ALL); + struct frame_info *frame; + + /* For local watchpoint expressions, which particular + instance of a local is being watched matters, so we + keep track of the frame to evaluate the expression + in. To evaluate the condition however, it doesn't + really matter which instantiation of the function + where the condition makes sense triggers the + watchpoint. This allows an expression like "watch + global if q > 10" set in `func', catch writes to + global on all threads that call `func', or catch + writes on all recursive calls of `func' by a single + thread. We simply always evaluate the condition in + the innermost frame that's executing where it makes + sense to evaluate the condition. It seems + intuitive. */ + frame = block_innermost_frame (w->cond_exp_valid_block); + if (frame != NULL) + select_frame (frame); else - { - warning (_("Watchpoint condition cannot be tested " - "in the current scope")); - /* If we failed to set the right context for this - watchpoint, unconditionally report it. */ - value_is_zero = 0; - } - /* FIXME-someday, should give breakpoint #. */ - value_free_to_mark (mark); - } - - if (cond && value_is_zero) - { - bs->stop = 0; + within_current_scope = 0; } - else if (b->thread != -1 && b->thread != thread_id) + if (within_current_scope) + value_is_zero + = catch_errors (breakpoint_cond_eval, cond, + "Error in testing breakpoint condition:\n", + RETURN_MASK_ALL); + else { - bs->stop = 0; + warning (_("Watchpoint condition cannot be tested " + "in the current scope")); + /* If we failed to set the right context for this + watchpoint, unconditionally report it. */ + value_is_zero = 0; } - else if (b->ignore_count > 0) - { - b->ignore_count--; - bs->stop = 0; - /* Increase the hit count even though we don't stop. */ - ++(b->hit_count); - observer_notify_breakpoint_modified (b); - } + /* FIXME-someday, should give breakpoint #. */ + value_free_to_mark (mark); + } + + if (cond && value_is_zero) + { + bs->stop = 0; + } + else if (b->thread != -1 && b->thread != thread_id) + { + bs->stop = 0; } + else if (b->ignore_count > 0) + { + b->ignore_count--; + bs->stop = 0; + /* Increase the hit count even though we don't stop. */ + ++(b->hit_count); + observer_notify_breakpoint_modified (b); + } } ----- with -b added ----- --- breakpoint.c~ 2013-11-02 13:42:45.321034724 -0700 +++ breakpoint.c 2013-11-11 19:18:21.525399880 -0800 @@ -5099,8 +5099,8 @@ bpstat_check_watchpoint (bpstat bs) } } - -/* Check conditions (condition proper, frame, thread and ignore count) +/* For breakpoints that are currently marked as telling gdb to stop, + check conditions (condition proper, frame, thread and ignore count) of breakpoint referred to by BS. If we should not stop for this breakpoint, set BS->stop to 0. */ @@ -5110,6 +5110,10 @@ bpstat_check_breakpoint_conditions (bpst int thread_id = pid_to_thread_id (ptid); const struct bp_location *bl; struct breakpoint *b; + int value_is_zero = 0; + struct expression *cond; + + gdb_assert (bs->stop); /* BS is built for existing struct breakpoint. */ bl = bs->bp_location_at; @@ -5123,14 +5127,12 @@ bpstat_check_breakpoint_conditions (bpst if (frame_id_p (b->frame_id) && !frame_id_eq (b->frame_id, get_stack_frame_id (get_current_frame ()))) - bs->stop = 0; - else if (bs->stop) { - int value_is_zero = 0; - struct expression *cond; + bs->stop = 0; + return; + } - /* Evaluate Python breakpoints that have a "stop" - method implemented. */ + /* Evaluate Python breakpoints that have a "stop" method implemented. */ if (b->py_bp_object) bs->stop = gdbpy_should_stop (b->py_bp_object); @@ -5225,7 +5227,6 @@ bpstat_check_breakpoint_conditions (bpst ++(b->hit_count); observer_notify_breakpoint_modified (b); } - } }