From patchwork Thu Nov 26 15:52:00 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 9831 Received: (qmail 63449 invoked by alias); 26 Nov 2015 15:52:06 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 63435 invoked by uid 89); 26 Nov 2015 15:52:05 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: usplmg21.ericsson.net Received: from usplmg21.ericsson.net (HELO usplmg21.ericsson.net) (198.24.6.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Thu, 26 Nov 2015 15:52:03 +0000 Received: from EUSAAHC008.ericsson.se (Unknown_Domain [147.117.188.96]) by usplmg21.ericsson.net (Symantec Mail Security) with SMTP id 68.0E.32102.F9A27565; Thu, 26 Nov 2015 16:52:00 +0100 (CET) Received: from [142.133.110.144] (147.117.188.8) by smtp-am.internal.ericsson.com (147.117.188.98) with Microsoft SMTP Server id 14.3.248.2; Thu, 26 Nov 2015 10:52:00 -0500 Subject: Re: [PATCH v2 2/3] Display names of remote threads To: Pedro Alves , References: <1448488138-2360-1-git-send-email-simon.marchi@ericsson.com> <1448488138-2360-3-git-send-email-simon.marchi@ericsson.com> <5656EDB8.8030803@redhat.com> From: Simon Marchi Message-ID: <56572AA0.9070904@ericsson.com> Date: Thu, 26 Nov 2015 10:52:00 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <5656EDB8.8030803@redhat.com> X-IsSubscribed: yes On 15-11-26 06:32 AM, Pedro Alves wrote: > You were probably already doing this, but since your submission process doesn't > show names in the ChangeLog entry, we can't tell -- if this was based on the > patch from Daniel, please make sure to also credit him in the ChangeLog. Yes of course. I don't put the name and date line of the ChangeLog entry, because the date is meaningless anyway. In those cases where it's not just me, I could include it to be explicit. >> +auxilliary information. > > "auxiliary", I think. Right, fixed. >> + if (info != NULL && info->priv != NULL) > > I don't think info can be NULL (though info->priv can). The linux-nat.c > version already assumes non-NULL. I think other callbacks have that > extra check because they take a ptid as argument. Ok, removed the info != NULL check. >> + return info->priv->name; >> + >> + return NULL; >> +} > > Otherwise looks good to me. > > Thanks, > Pedro Alves > Thanks, pushed with these fixes. From 79efa585c51f0657b319beb1e213d5721eaacdcc Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Thu, 26 Nov 2015 09:49:04 -0500 Subject: [PATCH] Display names of remote threads This patch adds support for thread names in the remote protocol, and updates gdb/gdbserver to use it. The information is added to the XML description sent in response to the qXfer:threads:read packet. gdb/ChangeLog: * linux-nat.c (linux_nat_thread_name): Replace implementation by call to linux_proc_tid_get_name. * nat/linux-procfs.c (linux_proc_tid_get_name): New function, implementation inspired by linux_nat_thread_name. * nat/linux-procfs.h (linux_proc_tid_get_name): New declaration. * remote.c (struct private_thread_info) : New field. (free_private_thread_info): Free name field. (remote_thread_name): New function. (thread_item_t) : New field. (clear_threads_listing_context): Free name field. (start_thread): Get name xml attribute. (thread_attributes): Add "name" attribute. (remote_update_thread_list): Copy name field. (init_remote_ops): Assign remote_thread_name callback. * target.h (target_thread_name): Update comment. * NEWS: Mention remote thread name support. gdb/gdbserver/ChangeLog: * linux-low.c (linux_target_ops): Use linux_proc_tid_get_name. * server.c (handle_qxfer_threads_worker): Refactor to include thread name in reply. * target.h (struct target_ops) : New field. (target_thread_name): New macro. gdb/doc/ChangeLog: * gdb.texinfo (Thread List Format): Mention thread names. --- gdb/ChangeLog | 20 ++++++++++++++++++++ gdb/NEWS | 5 +++++ gdb/doc/ChangeLog | 4 ++++ gdb/doc/gdb.texinfo | 8 +++++--- gdb/gdbserver/ChangeLog | 9 +++++++++ gdb/gdbserver/linux-low.c | 3 ++- gdb/gdbserver/server.c | 16 +++++++++------- gdb/gdbserver/target.h | 8 ++++++++ gdb/linux-nat.c | 33 +-------------------------------- gdb/nat/linux-procfs.c | 42 ++++++++++++++++++++++++++++++++++++++++++ gdb/nat/linux-procfs.h | 7 +++++++ gdb/remote.c | 29 ++++++++++++++++++++++++++++- gdb/target.h | 4 ++-- 13 files changed, 142 insertions(+), 46 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 1d9cb4f..0871500 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,23 @@ +2015-11-26 Daniel Colascione +2015-11-26 Simon Marchi + + * linux-nat.c (linux_nat_thread_name): Replace implementation by call + to linux_proc_tid_get_name. + * nat/linux-procfs.c (linux_proc_tid_get_name): New function, + implementation inspired by linux_nat_thread_name. + * nat/linux-procfs.h (linux_proc_tid_get_name): New declaration. + * remote.c (struct private_thread_info) : New field. + (free_private_thread_info): Free name field. + (remote_thread_name): New function. + (thread_item_t) : New field. + (clear_threads_listing_context): Free name field. + (start_thread): Get name xml attribute. + (thread_attributes): Add "name" attribute. + (remote_update_thread_list): Copy name field. + (init_remote_ops): Assign remote_thread_name callback. + * target.h (target_thread_name): Update comment. + * NEWS: Mention remote thread name support. + 2015-11-26 Simon Marchi * linux-nat.c (linux_nat_thread_name): Constify return value. diff --git a/gdb/NEWS b/gdb/NEWS index 31072b7..5f704fe 100644 --- a/gdb/NEWS +++ b/gdb/NEWS @@ -94,6 +94,11 @@ set remote exec-event-feature-packet show remote exec-event-feature-packet Set/show the use of the remote exec event feature. + * Thread names in remote protocol + + The reply to qXfer:threads:read may now include a name attribute for each + thread. + *** Changes in GDB 7.10 * Support for process record-replay and reverse debugging on aarch64*-linux* diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog index 80df9ad..7e747dc 100644 --- a/gdb/doc/ChangeLog +++ b/gdb/doc/ChangeLog @@ -1,3 +1,7 @@ +2015-11-26 Simon Marchi + + * gdb.texinfo (Thread List Format): Mention thread names. + 2015-11-24 Pedro Alves PR 17539 diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo index 1917008..c4d8a18 100644 --- a/gdb/doc/gdb.texinfo +++ b/gdb/doc/gdb.texinfo @@ -39542,7 +39542,7 @@ the following structure: @smallexample - + ... description ... @@ -39551,8 +39551,10 @@ the following structure: Each @samp{thread} element must have the @samp{id} attribute that identifies the thread (@pxref{thread-id syntax}). The @samp{core} attribute, if present, specifies which processor core -the thread was last executing on. The content of the of @samp{thread} -element is interpreted as human-readable auxilliary information. +the thread was last executing on. The @samp{name} attribute, if +present, specifies the human-readable name of the thread. The content +of the of @samp{thread} element is interpreted as human-readable +auxiliary information. @node Traceframe Info Format @section Traceframe Info Format diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog index e265798..6e2a95d 100644 --- a/gdb/gdbserver/ChangeLog +++ b/gdb/gdbserver/ChangeLog @@ -1,3 +1,12 @@ +2015-11-26 Daniel Colascione +2015-11-26 Simon Marchi + + * linux-low.c (linux_target_ops): Use linux_proc_tid_get_name. + * server.c (handle_qxfer_threads_worker): Refactor to include thread + name in reply. + * target.h (struct target_ops) : New field. + (target_thread_name): New macro. + 2015-11-23 Joel Brobecker * regcache.h (regcache_invalidate_pid): Add declaration. diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index e3a56a7..a70868c 100644 --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -7043,7 +7043,8 @@ static struct target_ops linux_target_ops = { linux_mntns_unlink, linux_mntns_readlink, linux_breakpoint_kind_from_pc, - linux_sw_breakpoint_from_kind + linux_sw_breakpoint_from_kind, + linux_proc_tid_get_name, }; static void diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c index 7d6c9cc..0105b99 100644 --- a/gdb/gdbserver/server.c +++ b/gdb/gdbserver/server.c @@ -1456,20 +1456,22 @@ handle_qxfer_threads_worker (struct inferior_list_entry *inf, void *arg) char ptid_s[100]; int core = target_core_of_thread (ptid); char core_s[21]; + const char *name = target_thread_name (ptid); write_ptid (ptid_s, ptid); + buffer_xml_printf (buffer, "\n", - ptid_s, core_s); - } - else - { - buffer_xml_printf (buffer, "\n", - ptid_s); + buffer_xml_printf (buffer, " core=\"%s\"", core_s); } + + if (name != NULL) + buffer_xml_printf (buffer, " name=\"%s\"", name); + + buffer_xml_printf (buffer, "/>\n"); } /* Helper for handle_qxfer_threads. */ diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h index 358a8ab..8903da5 100644 --- a/gdb/gdbserver/target.h +++ b/gdb/gdbserver/target.h @@ -453,6 +453,10 @@ struct target_ops specific meaning like the Z0 kind parameter. SIZE is set to the software breakpoint's length in memory. */ const gdb_byte *(*sw_breakpoint_from_kind) (int kind, int *size); + + /* Return the thread's name, or NULL if the target is unable to determine it. + The returned value must not be freed by the caller. */ + const char *(*thread_name) (ptid_t thread); }; extern struct target_ops *the_target; @@ -663,6 +667,10 @@ ptid_t mywait (ptid_t ptid, struct target_waitstatus *ourstatus, int options, (the_target->core_of_thread ? (*the_target->core_of_thread) (ptid) \ : -1) +#define target_thread_name(ptid) \ + (the_target->thread_name ? (*the_target->thread_name) (ptid) \ + : NULL) + int read_inferior_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len); int write_inferior_memory (CORE_ADDR memaddr, const unsigned char *myaddr, diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c index 2421687..9bc1324 100644 --- a/gdb/linux-nat.c +++ b/gdb/linux-nat.c @@ -4100,38 +4100,7 @@ linux_nat_pid_to_str (struct target_ops *ops, ptid_t ptid) static const char * linux_nat_thread_name (struct target_ops *self, struct thread_info *thr) { - int pid = ptid_get_pid (thr->ptid); - long lwp = ptid_get_lwp (thr->ptid); -#define FORMAT "/proc/%d/task/%ld/comm" - char buf[sizeof (FORMAT) + 30]; - FILE *comm_file; - char *result = NULL; - - snprintf (buf, sizeof (buf), FORMAT, pid, lwp); - comm_file = gdb_fopen_cloexec (buf, "r"); - if (comm_file) - { - /* Not exported by the kernel, so we define it here. */ -#define COMM_LEN 16 - static char line[COMM_LEN + 1]; - - if (fgets (line, sizeof (line), comm_file)) - { - char *nl = strchr (line, '\n'); - - if (nl) - *nl = '\0'; - if (*line != '\0') - result = line; - } - - fclose (comm_file); - } - -#undef COMM_LEN -#undef FORMAT - - return result; + return linux_proc_tid_get_name (thr->ptid); } /* Accepts an integer PID; Returns a string representing a file that diff --git a/gdb/nat/linux-procfs.c b/gdb/nat/linux-procfs.c index 24bcb01..78cbb03 100644 --- a/gdb/nat/linux-procfs.c +++ b/gdb/nat/linux-procfs.c @@ -187,6 +187,48 @@ linux_proc_pid_is_zombie (pid_t pid) /* See linux-procfs.h. */ +const char * +linux_proc_tid_get_name (ptid_t ptid) +{ +#define TASK_COMM_LEN 16 /* As defined in the kernel's sched.h. */ + + static char comm_buf[TASK_COMM_LEN]; + char comm_path[100]; + FILE *comm_file; + const char *comm_val; + pid_t pid = ptid_get_pid (ptid); + pid_t tid = ptid_lwp_p (ptid) ? ptid_get_lwp (ptid) : ptid_get_pid (ptid); + + xsnprintf (comm_path, sizeof (comm_path), + "/proc/%ld/task/%ld/comm", (long) pid, (long) tid); + + comm_file = gdb_fopen_cloexec (comm_path, "r"); + if (comm_file == NULL) + return NULL; + + comm_val = fgets (comm_buf, sizeof (comm_buf), comm_file); + fclose (comm_file); + + if (comm_val != NULL) + { + int i; + + /* Make sure there is no newline at the end. */ + for (i = 0; i < sizeof (comm_buf); i++) + { + if (comm_buf[i] == '\n') + { + comm_buf[i] = '\0'; + break; + } + } + } + + return comm_val; +} + +/* See linux-procfs.h. */ + void linux_proc_attach_tgid_threads (pid_t pid, linux_proc_attach_lwp_func attach_lwp) diff --git a/gdb/nat/linux-procfs.h b/gdb/nat/linux-procfs.h index f9cad39..f28c1ba 100644 --- a/gdb/nat/linux-procfs.h +++ b/gdb/nat/linux-procfs.h @@ -54,6 +54,13 @@ extern int linux_proc_pid_is_zombie_nowarn (pid_t pid); extern int linux_proc_pid_is_gone (pid_t pid); +/* Return a string giving the thread's name or NULL if the + information is unavailable. The returned value points to a statically + allocated buffer. The value therefore becomes invalid at the next + linux_proc_tid_get_name call. */ + +extern const char *linux_proc_tid_get_name (ptid_t ptid); + /* Callback function for linux_proc_attach_tgid_threads. If the PTID thread is not yet known, try to attach to it and return true, otherwise return false. */ diff --git a/gdb/remote.c b/gdb/remote.c index 2bbab62..a80e548 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -437,6 +437,7 @@ struct remote_state struct private_thread_info { char *extra; + char *name; int core; }; @@ -444,6 +445,7 @@ static void free_private_thread_info (struct private_thread_info *info) { xfree (info->extra); + xfree (info->name); xfree (info); } @@ -2141,6 +2143,18 @@ remote_thread_alive (struct target_ops *ops, ptid_t ptid) return (rs->buf[0] == 'O' && rs->buf[1] == 'K'); } +/* Return a pointer to a thread name if we know it and NULL otherwise. + The thread_info object owns the memory for the name. */ + +static const char * +remote_thread_name (struct target_ops *ops, struct thread_info *info) +{ + if (info->priv != NULL) + return info->priv->name; + + return NULL; +} + /* About these extended threadlist and threadinfo packets. They are variable length packets but, the fields within them are often fixed length. They are redundent enough to send over UDP as is the @@ -2821,6 +2835,9 @@ typedef struct thread_item /* The thread's extra info. May be NULL. */ char *extra; + /* The thread's name. May be NULL. */ + char *name; + /* The core the thread was running on. -1 if not known. */ int core; } thread_item_t; @@ -2847,7 +2864,10 @@ clear_threads_listing_context (void *p) struct thread_item *item; for (i = 0; VEC_iterate (thread_item_t, context->items, i, item); ++i) - xfree (item->extra); + { + xfree (item->extra); + xfree (item->name); + } VEC_free (thread_item_t, context->items); } @@ -2951,6 +2971,9 @@ start_thread (struct gdb_xml_parser *parser, else item.core = -1; + attr = xml_find_attribute (attributes, "name"); + item.name = attr != NULL ? xstrdup (attr->value) : NULL; + item.extra = 0; VEC_safe_push (thread_item_t, data->items, &item); @@ -2971,6 +2994,7 @@ end_thread (struct gdb_xml_parser *parser, const struct gdb_xml_attribute thread_attributes[] = { { "id", GDB_XML_AF_NONE, NULL, NULL }, { "core", GDB_XML_AF_OPTIONAL, gdb_xml_parse_attr_ulongest, NULL }, + { "name", GDB_XML_AF_OPTIONAL, NULL, NULL }, { NULL, GDB_XML_AF_NONE, NULL, NULL } }; @@ -3149,6 +3173,8 @@ remote_update_thread_list (struct target_ops *ops) info->core = item->core; info->extra = item->extra; item->extra = NULL; + info->name = item->name; + item->name = NULL; } } } @@ -12738,6 +12764,7 @@ Specify the serial device it is connected to\n\ remote_ops.to_pass_signals = remote_pass_signals; remote_ops.to_program_signals = remote_program_signals; remote_ops.to_thread_alive = remote_thread_alive; + remote_ops.to_thread_name = remote_thread_name; remote_ops.to_update_thread_list = remote_update_thread_list; remote_ops.to_pid_to_str = remote_pid_to_str; remote_ops.to_extra_thread_info = remote_threads_extra_info; diff --git a/gdb/target.h b/gdb/target.h index ac28a41..65b717c 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -1820,8 +1820,8 @@ extern char *normal_pid_to_str (ptid_t ptid); #define target_extra_thread_info(TP) \ (current_target.to_extra_thread_info (¤t_target, TP)) -/* Return the thread's name. A NULL result means that the target - could not determine this thread's name. */ +/* Return the thread's name, or NULL if the target is unable to determine it. + The returned value must not be freed by the caller. */ extern const char *target_thread_name (struct thread_info *);