From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27914 invoked by alias); 9 Nov 2012 01:09:30 -0000 Received: (qmail 27906 invoked by uid 22791); 9 Nov 2012 01:09:29 -0000 X-SWARE-Spam-Status: No, hits=-4.5 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL 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; Fri, 09 Nov 2012 01:09:22 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1TWd61-0000ci-16 from Yao_Qi@mentor.com ; Thu, 08 Nov 2012 17:09:21 -0800 Received: from SVR-ORW-FEM-05.mgc.mentorg.com ([147.34.97.43]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Thu, 8 Nov 2012 17:09:20 -0800 Received: from qiyao.dyndns.org (147.34.91.1) by svr-orw-fem-05.mgc.mentorg.com (147.34.97.43) with Microsoft SMTP Server id 14.1.289.1; Thu, 8 Nov 2012 17:09:19 -0800 Message-ID: <509C57B2.5020806@codesourcery.com> Date: Fri, 09 Nov 2012 01:09:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; Linux i686; rv:15.0) Gecko/20120911 Thunderbird/15.0.1 MIME-Version: 1.0 To: Pedro Alves CC: Subject: Re: [PATCH] Add breakpoint_created observer to update tracepoint_count. References: <1352249397-15044-1-git-send-email-yao@codesourcery.com> <509AA4C6.2030102@redhat.com> In-Reply-To: <509AA4C6.2030102@redhat.com> Content-Type: text/plain; charset="ISO-8859-1" 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: 2012-11/txt/msg00225.txt.bz2 On 11/08/2012 02:13 AM, Pedro Alves wrote: > No need to install an observer for a notification that is emitted in the same > module the new observer is in. This is internals of the breakpoints module. > All set_breakpoint_count's calls are centralized in install_breakpoint, through > set_breakpoint_number. All but break-range's, that is. I don't recall why > that doesn't use install_breakpoint. Maybe it should. > My original thought is to move tracepoint-related code from breakpoint.c to tracepoint.c, so register a breakpoint_created observer in tracepoint.c to update 'tracepoint_count'. Unfortunately, tracepoint commands use some breakpoint internal macros that prevent moving them to tracepoint.c, so I move the observer register code back to breakpoint.c. Yes, it is not necessary to register a observer to update some state in the same module. > We should be able to put 'if (is_tracepoint) set_tracepoint_count()' in > install_breakpoint too. Hmmm, that sounds the right place to update 'tracepoint_count'. How about patch below? A new test is added, without this fix, this test fails. -- Yao gdb: 2012-11-09 Yao Qi * breakpoint.c: Declare set_tracepoint_count. (install_breakpoint): Call set_tracepoint_count if B is a tracepoint. (trace_command): Don't call set_tracepoint_count. Re-indent. (strace_command, ftrace_command): (create_tracepoint_from_upload): Likewise. gdb/testsuite: 2012-11-09 Yao Qi * gdb.mi/mi-break.exp (test_abreak_creation): New. --- gdb/breakpoint.c | 75 ++++++++++++++++++------------------- gdb/testsuite/gdb.mi/mi-break.exp | 10 +++++ 2 files changed, 47 insertions(+), 38 deletions(-) diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 092d81e..3763a04 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -261,6 +261,8 @@ static void disable_trace_command (char *, int); static void trace_pass_command (char *, int); +static void set_tracepoint_count (int num); + static int is_masked_watchpoint (const struct breakpoint *b); static struct bp_location **get_first_locp_gte_addr (CORE_ADDR address); @@ -8319,6 +8321,8 @@ install_breakpoint (int internal, struct breakpoint *b, int update_gll) { add_to_breakpoint_chain (b); set_breakpoint_number (internal, b); + if (is_tracepoint (b)) + set_tracepoint_count (breakpoint_count); if (!internal) mention (b); observer_notify_breakpoint_created (b); @@ -14999,35 +15003,33 @@ trace_command (char *arg, int from_tty) else ops = &tracepoint_breakpoint_ops; - if (create_breakpoint (get_current_arch (), - arg, - NULL, 0, NULL, 1 /* parse arg */, - 0 /* tempflag */, - bp_tracepoint /* type_wanted */, - 0 /* Ignore count */, - pending_break_support, - ops, - from_tty, - 1 /* enabled */, - 0 /* internal */, 0)) - set_tracepoint_count (breakpoint_count); + create_breakpoint (get_current_arch (), + arg, + NULL, 0, NULL, 1 /* parse arg */, + 0 /* tempflag */, + bp_tracepoint /* type_wanted */, + 0 /* Ignore count */, + pending_break_support, + ops, + from_tty, + 1 /* enabled */, + 0 /* internal */, 0); } static void ftrace_command (char *arg, int from_tty) { - if (create_breakpoint (get_current_arch (), - arg, - NULL, 0, NULL, 1 /* parse arg */, - 0 /* tempflag */, - bp_fast_tracepoint /* type_wanted */, - 0 /* Ignore count */, - pending_break_support, - &tracepoint_breakpoint_ops, - from_tty, - 1 /* enabled */, - 0 /* internal */, 0)) - set_tracepoint_count (breakpoint_count); + create_breakpoint (get_current_arch (), + arg, + NULL, 0, NULL, 1 /* parse arg */, + 0 /* tempflag */, + bp_fast_tracepoint /* type_wanted */, + 0 /* Ignore count */, + pending_break_support, + &tracepoint_breakpoint_ops, + from_tty, + 1 /* enabled */, + 0 /* internal */, 0); } /* strace command implementation. Creates a static tracepoint. */ @@ -15044,18 +15046,17 @@ strace_command (char *arg, int from_tty) else ops = &tracepoint_breakpoint_ops; - if (create_breakpoint (get_current_arch (), - arg, - NULL, 0, NULL, 1 /* parse arg */, - 0 /* tempflag */, - bp_static_tracepoint /* type_wanted */, - 0 /* Ignore count */, - pending_break_support, - ops, - from_tty, - 1 /* enabled */, - 0 /* internal */, 0)) - set_tracepoint_count (breakpoint_count); + create_breakpoint (get_current_arch (), + arg, + NULL, 0, NULL, 1 /* parse arg */, + 0 /* tempflag */, + bp_static_tracepoint /* type_wanted */, + 0 /* Ignore count */, + pending_break_support, + ops, + from_tty, + 1 /* enabled */, + 0 /* internal */, 0); } /* Set up a fake reader function that gets command lines from a linked @@ -15124,8 +15125,6 @@ create_tracepoint_from_upload (struct uploaded_tp *utp) CREATE_BREAKPOINT_FLAGS_INSERTED)) return NULL; - set_tracepoint_count (breakpoint_count); - /* Get the tracepoint we just created. */ tp = get_tracepoint (tracepoint_count); gdb_assert (tp != NULL); diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp index 573f484..e40f94d 100644 --- a/gdb/testsuite/gdb.mi/mi-break.exp +++ b/gdb/testsuite/gdb.mi/mi-break.exp @@ -150,6 +150,14 @@ proc test_rbreak_creation_and_listing {} { "delete temp breakpoints" } +proc test_abreak_creation {} { + mi_gdb_test "522-break-insert -a main" \ + "522\\^done,bkpt=\{number=\"10\",type=\"tracepoint\".*\"\}" \ + "break-insert -a operation" + + mi_gdb_test "p \$tpnum" ".* = 10.*" "print \$tpnum" +} + proc test_ignore_count {} { global mi_gdb_prompt global line_callme_body @@ -256,5 +264,7 @@ test_disabled_creation test_breakpoint_commands +test_abreak_creation + mi_gdb_exit return 0 -- 1.7.7.6