From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 526 invoked by alias); 28 Mar 2011 14:37:36 -0000 Received: (qmail 363 invoked by uid 22791); 28 Mar 2011 14:37:34 -0000 X-SWARE-Spam-Status: No, hits=-1.6 required=5.0 tests=AWL,BAYES_00,SPF_SOFTFAIL,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, 28 Mar 2011 14:37:28 +0000 Received: from d01dlp02.pok.ibm.com (d01dlp02.pok.ibm.com [9.56.224.85]) by e1.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id p2SER5fv004343 for ; Mon, 28 Mar 2011 10:27:05 -0400 Received: from d01relay06.pok.ibm.com (d01relay06.pok.ibm.com [9.56.227.116]) by d01dlp02.pok.ibm.com (Postfix) with ESMTP id E0D956E803C for ; Mon, 28 Mar 2011 10:37:17 -0400 (EDT) Received: from d01av04.pok.ibm.com (d01av04.pok.ibm.com [9.56.224.64]) by d01relay06.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p2SEbG5h1335306 for ; Mon, 28 Mar 2011 10:37:16 -0400 Received: from d01av04.pok.ibm.com (loopback [127.0.0.1]) by d01av04.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p2SEbGLL024901 for ; Mon, 28 Mar 2011 10:37:16 -0400 Received: from [9.12.224.105] ([9.12.224.105]) by d01av04.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with ESMTP id p2SEbEb4024823 for ; Mon, 28 Mar 2011 10:37:15 -0400 Subject: [RFA] Refactor breakpoint_re_set_one From: Thiago Jung Bauermann To: gdb-patches ml Content-Type: text/plain; charset="UTF-8" Date: Mon, 28 Mar 2011 15:18:00 -0000 Message-ID: <1301322874.2433.0.camel@hactar> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-Content-Scanned: Fidelis XPS MAILER 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-03/txt/msg01129.txt.bz2 Hi, For the PowerPC BookE ranged breakpoints patch I had to break breakpoint_re_set_one into two functions. I thought it would be clearer and easier to review if I put that refactoring in a separate patch. This patch doesn't change any functionality, just moves code around. It applies on top of: http://sourceware.org/ml/gdb-patches/2011-03/msg00978.html I could also make this patch not depend on the above patch (which is just a cleanup after all, I don't strictly need that). There are no regressions in i386-linux, ppc-linux and ppc64-linux. Ok? -- []'s Thiago Jung Bauermann IBM Linux Technology Center 2011-03-27 Thiago Jung Bauermann * breakpoint.c (breakpoint_re_set_one): Factor out breakpoint-resetting code from here ... (reset_breakpoint): ... to here ... (addr_string_to_sals): ... and here. diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 548b775..df8bb99 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -10484,6 +10484,113 @@ update_breakpoint_locations (struct breakpoint *b, update_global_location_list (1); } +/* Find the SaL locations corresponding to the given addr_string. */ + +static struct symtabs_and_lines +addr_string_to_sals (struct breakpoint *b, char *addr_string, int *found) +{ + char *s; + int marker_spec, not_found; + struct symtabs_and_lines sals; + struct gdb_exception e; + + s = addr_string; + marker_spec = b->type == bp_static_tracepoint && is_marker_spec (s); + + TRY_CATCH (e, RETURN_MASK_ERROR) + { + if (marker_spec) + { + sals = decode_static_tracepoint_spec (&s); + if (sals.nelts > b->static_trace_marker_id_idx) + { + sals.sals[0] = sals.sals[b->static_trace_marker_id_idx]; + sals.nelts = 1; + } + else + error (_("marker %s not found"), b->static_trace_marker_id); + } + else + sals = decode_line_1 (&s, 1, (struct symtab *) NULL, 0, + NULL, ¬_found); + } + + /* For pending breakpoints, it's expected that parsing will + fail until the right shared library is loaded. User has + already told to create pending breakpoints and don't need + extra messages. If breakpoint is in bp_shlib_disabled + state, then user already saw the message about that + breakpoint being disabled, and don't want to see more + errors. */ + if (e.reason < 0 && (!not_found || (!b->condition_not_parsed + && !(b->loc && b->loc->shlib_disabled) + && b->enable_state != bp_disabled))) + { + /* We surely don't want to warn about the same breakpoint + 10 times. One solution, implemented here, is disable + the breakpoint on error. Another solution would be to + have separate 'warning emitted' flag. Since this + happens only when a binary has changed, I don't know + which approach is better. */ + b->enable_state = bp_disabled; + throw_exception (e); + } + + if (!not_found) + { + gdb_assert (sals.nelts == 1); + + resolve_sal_pc (&sals.sals[0]); + if (b->condition_not_parsed && s && s[0]) + { + char *cond_string = 0; + int thread = -1; + int task = 0; + + find_condition_and_thread (s, sals.sals[0].pc, + &cond_string, &thread, &task); + if (cond_string) + b->cond_string = cond_string; + b->thread = thread; + b->task = task; + b->condition_not_parsed = 0; + } + + if (b->type == bp_static_tracepoint && !marker_spec) + sals.sals[0] = update_static_tracepoint (b, sals.sals[0]); + } + + *found = !not_found; + + return sals; +} + +/* Reset a hardware or software breakpoint. */ + +static void +reset_breakpoint (struct breakpoint *b) +{ + int found; + struct symtabs_and_lines sals; + struct symtabs_and_lines expanded = {0}; + struct cleanup *cleanups = make_cleanup (null_cleanup, NULL); + + input_radix = b->input_radix; + save_current_space_and_thread (); + switch_to_program_space_and_thread (b->pspace); + set_language (b->language); + + sals = addr_string_to_sals (b, b->addr_string, &found); + if (found) + { + make_cleanup (xfree, sals.sals); + expanded = expand_line_sal_maybe (sals.sals[0]); + } + + update_breakpoint_locations (b, expanded); + do_cleanups (cleanups); +} + /* Reset a breakpoint given it's struct breakpoint * BINT. The value we return ends up being the return value from catch_errors. Unused in this case. */ @@ -10493,14 +10600,6 @@ breakpoint_re_set_one (void *bint) { /* Get past catch_errs. */ struct breakpoint *b = (struct breakpoint *) bint; - int not_found = 0; - int *not_found_ptr = ¬_found; - struct symtabs_and_lines sals = {0}; - struct symtabs_and_lines expanded = {0}; - char *s; - struct gdb_exception e; - struct cleanup *cleanups = make_cleanup (null_cleanup, NULL); - int marker_spec = 0; switch (b->type) { @@ -10524,82 +10623,7 @@ breakpoint_re_set_one (void *bint) return 0; } - input_radix = b->input_radix; - s = b->addr_string; - - save_current_space_and_thread (); - switch_to_program_space_and_thread (b->pspace); - - marker_spec = b->type == bp_static_tracepoint && is_marker_spec (s); - - set_language (b->language); - TRY_CATCH (e, RETURN_MASK_ERROR) - { - if (marker_spec) - { - sals = decode_static_tracepoint_spec (&s); - if (sals.nelts > b->static_trace_marker_id_idx) - { - sals.sals[0] = sals.sals[b->static_trace_marker_id_idx]; - sals.nelts = 1; - } - else - error (_("marker %s not found"), b->static_trace_marker_id); - } - else - sals = decode_line_1 (&s, 1, (struct symtab *) NULL, 0, - NULL, not_found_ptr); - } - - /* For pending breakpoints, it's expected that parsing will - fail until the right shared library is loaded. User has - already told to create pending breakpoints and don't need - extra messages. If breakpoint is in bp_shlib_disabled - state, then user already saw the message about that - breakpoint being disabled, and don't want to see more - errors. */ - if (e.reason < 0 && (!not_found || (!b->condition_not_parsed - && !(b->loc && b->loc->shlib_disabled) - && b->enable_state != bp_disabled))) - { - /* We surely don't want to warn about the same breakpoint - 10 times. One solution, implemented here, is disable - the breakpoint on error. Another solution would be to - have separate 'warning emitted' flag. Since this - happens only when a binary has changed, I don't know - which approach is better. */ - b->enable_state = bp_disabled; - throw_exception (e); - } - - if (!not_found) - { - gdb_assert (sals.nelts == 1); - - resolve_sal_pc (&sals.sals[0]); - if (b->condition_not_parsed && s && s[0]) - { - char *cond_string = 0; - int thread = -1; - int task = 0; - - find_condition_and_thread (s, sals.sals[0].pc, - &cond_string, &thread, &task); - if (cond_string) - b->cond_string = cond_string; - b->thread = thread; - b->task = task; - b->condition_not_parsed = 0; - } - - if (b->type == bp_static_tracepoint && !marker_spec) - sals.sals[0] = update_static_tracepoint (b, sals.sals[0]); - - expanded = expand_line_sal_maybe (sals.sals[0]); - } - - make_cleanup (xfree, sals.sals); - update_breakpoint_locations (b, expanded); + reset_breakpoint (b); break; case bp_watchpoint: @@ -10679,7 +10703,6 @@ breakpoint_re_set_one (void *bint) break; } - do_cleanups (cleanups); return 0; }