From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9850 invoked by alias); 28 Nov 2012 20:32:32 -0000 Received: (qmail 9841 invoked by uid 22791); 28 Nov 2012 20:32:31 -0000 X-SWARE-Spam-Status: No, hits=-7.8 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_CP,TW_QX X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 28 Nov 2012 20:32:19 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id qASKWHi4012972 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 28 Nov 2012 15:32:17 -0500 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id qASKWDL3008820; Wed, 28 Nov 2012 15:32:14 -0500 Message-ID: <50B674CC.6050009@redhat.com> Date: Wed, 28 Nov 2012 20:32:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/17.0 Thunderbird/17.0 MIME-Version: 1.0 To: markus.t.metzger@intel.com CC: gdb-patches@sourceware.org, markus.t.metzger@gmail.com, jan.kratochvil@redhat.com, palves@redhat.com, tromey@redhat.com, kettenis@gnu.org Subject: Re: [patch v4 09/13] gdbserver, btrace: add generic btrace support References: <1354013351-14791-1-git-send-email-markus.t.metzger@intel.com> <1354013351-14791-10-git-send-email-markus.t.metzger@intel.com> In-Reply-To: <1354013351-14791-10-git-send-email-markus.t.metzger@intel.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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/msg00827.txt.bz2 On 11/27/2012 10:49 AM, markus.t.metzger@intel.com wrote: > From: Markus Metzger > > Add support to gdbserver to understand branch trace related packages. > > 2012-11-27 Markus Metzger > > gdbserver/ > * target.h (struct target_ops): Add btrace ops. > (target_supports_btrace): New macro. > (target_btrace_has_changed): New macro. > (target_enable_btrace): New macro. > (target_disable_btrace): New macro. > (target_read_btrace): New macro. > * gdbthread.h (struct thread_info): Add btrace field. > * server.c (handle_btrace_general_set): New function. > (handle_btrace_enable): New function. > (handle_btrace_disable): New function. > (handle_general_set): Call handle_btrace_general_set. > (handle_qxfer_btrace): New function. > (struct qxfer qxfer_packets[]): Add btrace entry. > (handle_btrace_query): New function. > (handle_query): Add btrace to supported query, call handle_btrace_query. > * inferiors.c (remove_thread): Disable btrace. > > > --- > gdb/gdbserver/gdbthread.h | 5 ++ > gdb/gdbserver/inferiors.c | 3 + > gdb/gdbserver/server.c | 167 +++++++++++++++++++++++++++++++++++++++++++++ > gdb/gdbserver/target.h | 33 +++++++++ > 4 files changed, 208 insertions(+), 0 deletions(-) > > diff --git a/gdb/gdbserver/gdbthread.h b/gdb/gdbserver/gdbthread.h > index adc23da..6238ed2 100644 > --- a/gdb/gdbserver/gdbthread.h > +++ b/gdb/gdbserver/gdbthread.h > @@ -22,6 +22,8 @@ > > #include "server.h" > > +struct btrace_target_info; > + > struct thread_info > { > struct inferior_list_entry entry; > @@ -58,6 +60,9 @@ struct thread_info > Each item in the list holds the current step of the while-stepping > action. */ > struct wstep_state *while_stepping; > + > + /* Branch trace target information for this thread. */ > + struct btrace_target_info *btrace; > }; > > extern struct inferior_list all_threads; > diff --git a/gdb/gdbserver/inferiors.c b/gdb/gdbserver/inferiors.c > index 76abaf5..76f5731 100644 > --- a/gdb/gdbserver/inferiors.c > +++ b/gdb/gdbserver/inferiors.c > @@ -161,6 +161,9 @@ free_one_thread (struct inferior_list_entry *inf) > void > remove_thread (struct thread_info *thread) > { > + if (thread->btrace) > + target_disable_btrace (thread->btrace); > + > remove_inferior (&all_threads, (struct inferior_list_entry *) thread); > free_one_thread (&thread->entry); > } > diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c > index fae9199..2758d40 100644 > --- a/gdb/gdbserver/server.c > +++ b/gdb/gdbserver/server.c > @@ -442,6 +442,82 @@ write_qxfer_response (char *buf, const void *data, int len, int is_more) > > /* Handle all of the extended 'Q' packets. */ This comment above should be kept close to the corresponding function. > > +/* Handle btrace enabling. */ > +static int > +handle_btrace_enable (char *ptid_str) > +{ > + ptid_t ptid = read_ptid (ptid_str, NULL); > + struct thread_info *thread = find_thread_ptid (ptid); > + int err = 1; > + > + if (!thread) > + fprintf (stderr, "No such thread: %s.\n", ptid_str); > + else if (thread->btrace) > + fprintf (stderr, "Btrace already enabled for %s.\n", ptid_str); > + else if (!the_target->enable_btrace) > + fprintf (stderr, "Target does not support branch tracing."); > + else > + { > + thread->btrace = target_enable_btrace (thread->entry.id); > + if (!thread->btrace) > + fprintf (stderr, "Could not enable btrace for %s.\n", ptid_str); > + else > + err = 0; > + } > + > + return err; Instead of printing those errors to stderr, you can pass them down to GDB, by sending "E.ERRTXT" instead of E01. (See packet_check_result in remote.c). > +} > + > +/* Handle btrace disabling. */ > +static int > +handle_btrace_disable (char *ptid_str) > +{ > + ptid_t ptid = read_ptid (ptid_str, NULL); > + struct thread_info *thread = find_thread_ptid (ptid); > + int err = 1; > + > + if (!thread) > + fprintf (stderr, "No such thread: %s.\n", ptid_str); > + else if (!thread->btrace) > + fprintf (stderr, "Branch tracing not enabled for %s.\n", ptid_str); > + else if (!the_target->disable_btrace) > + fprintf (stderr, "Target does not support branch tracing."); > + else if (target_disable_btrace (thread->btrace) != 0) > + fprintf (stderr, "Could not disable branch tracing for %s.\n", ptid_str); > + else > + { > + thread->btrace = NULL; > + err = 0; > + } > + > + return err; > +} Same comment about handle_btrace_enable/handle_btrace_disable being almost identical. > + > +/* Handle the "Qbtrace" packet. */ > +static int > +handle_btrace_general_set (char *own_buf) > +{ > + char *operation = own_buf + strlen ("Qbtrace:"); > + int err = 1; > + > + if (strncmp ("Qbtrace:", own_buf, strlen ("Qbtrace:")) != 0) > + return 0; > + > + if (strncmp ("on:", operation, strlen ("on:")) == 0) > + err = handle_btrace_enable (operation + strlen ("on:")); > + else if (strncmp ("off:", operation, strlen ("off:")) == 0) > + err = handle_btrace_disable (operation + strlen ("off:")); > + else > + fprintf (stderr, "Unknown btrace operation: %s. Use on or off.\n", own_buf); > + > + if (err) > + write_enn (own_buf); > + else > + write_ok (own_buf); > + > + return 1; > +} > + > static void > handle_general_set (char *own_buf) > { > @@ -596,6 +672,9 @@ handle_general_set (char *own_buf) > return; > } > > + if (handle_btrace_general_set (own_buf)) > + return; > + > /* Otherwise we didn't know what packet it was. Say we didn't > understand it. */ > own_buf[0] = 0; > @@ -1295,6 +1374,55 @@ handle_qxfer_fdpic (const char *annex, gdb_byte *readbuf, > return (*the_target->read_loadmap) (annex, offset, readbuf, len); > } > > +/* Handle branch trace data transfer. */ > +static int > +handle_qxfer_btrace (const char *annex, > + gdb_byte *readbuf, const gdb_byte *writebuf, > + ULONGEST offset, LONGEST len) > +{ > + static struct buffer cache; > + struct thread_info *thread; > + ptid_t ptid; > + > + if (!the_target->read_btrace || writebuf) > + return -2; > + > + if (!target_running () || !annex) > + return -1; > + > + ptid = read_ptid ((char *) annex, NULL); > + thread = find_thread_ptid (ptid); > + if (!thread) > + { > + fprintf (stderr, "No such thread: %s\n", annex); > + return -1; > + } > + else if (!thread->btrace) > + { > + fprintf (stderr, "Btrace not enabled for %s\n", annex); > + return -1; > + } > + > + if (!offset) > + { > + buffer_free (&cache); > + > + target_read_btrace (thread->btrace, &cache); > + } > + else if (offset >= cache.used_size) > + { > + buffer_free (&cache); > + return 0; I think this should be -1. > + } > + > + if (len > (cache.used_size - offset)) > + len = cache.used_size - offset; Unnecessary parens. > + > + memcpy (readbuf, cache.buffer + offset, len); > + > + return len; > +} > + > static const struct qxfer qxfer_packets[] = > { > { "auxv", handle_qxfer_auxv }, > @@ -1308,6 +1436,7 @@ static const struct qxfer qxfer_packets[] = > { "statictrace", handle_qxfer_statictrace }, > { "threads", handle_qxfer_threads }, > { "traceframe-info", handle_qxfer_traceframe_info }, > + { "btrace", handle_qxfer_btrace }, > }; Please keep the list sorted. -- Pedro Alves