[PATCH][PR threads/15824] when get threads name failed from info threads with linux kernel version earlier than 2.6.33
Will Huang
will.huang@aliyun-inc.com
Fri Aug 16 05:32:00 GMT 2013
Thanks for Yao Qi's review.
Patch is update.
Test OS:Linux 2.6.32-220.23.2.ali1089.el5.x86_64
GNU gdb (GDB) 7.6
Under linux kernel 2.6.32( or earlier version) there is no proc file named /proc/$pid/task/$tid/comm.
GDB init their threads' name from this proc file. So info threads can't print correct thread names when
this proc file not exist. This can be solve by using procfs /proc/$pid/task/$tid/status instead if comm
file not exists. Status file has a Name line which starts by "Name:".
diff -uNr ./gdb.org/ChangeLog ./gdb/ChangeLog
--- ./gdb.org/ChangeLog 2013-04-26 19:43:30.000000000 +0800
+++ ./gdb/ChangeLog 2013-08-16 11:48:20.000000000 +0800
@@ -1,3 +1,11 @@
+2013-08-14 Will Huang <will.huang@aliyun-inc.com>
+
+ PR threads/15824
+ * linux-nat.c (linux_nat_thread_name): Add get thread name from
+ procfs "/proc/$pid/task/$tid/status".
+ * commo/linux-procfs.c (linux_proc_get_thread_name
+ linux_proc_get_string): New.
+
2013-04-26 Joel Brobecker <brobecker@adacore.com>
* NEWS: Change "since GDB 7.5" into "in GDB 7.6".
diff -uNr ./gdb.org/common/linux-procfs.c ./gdb/common/linux-procfs.c
--- ./gdb.org/common/linux-procfs.c 2013-01-01 14:32:54.000000000 +0800
+++ ./gdb/common/linux-procfs.c 2013-08-16 11:48:49.000000000 +0800
@@ -55,6 +55,55 @@
return retval;
}
+/* Return the string length of FILED /proc/LWPID/task/TID/status.
+ Return -1 if not found. */
+
+static int
+linux_proc_get_string (pid_t lwpid, pid_t tid, char *target,
+ size_t t_size, const char *field)
+{
+ size_t field_len = strlen (field);
+ FILE *status_file;
+ char buf[100];
+ int retval = -1;
+
+ if (tid > 0)
+ snprintf (buf, sizeof (buf), "/proc/%d/task/%d/status",
+ (int) lwpid, (int) tid);
+ else
+ {
+ /*if TID is zero, ingnore it*/
+ snprintf (buf, sizeof (buf), "/proc/%d/status", (int) lwpid);
+ }
+
+ status_file = fopen (buf, "r");
+ if (status_file == NULL)
+ {
+ warning (_("unable to open /proc file '%s'"), buf);
+ return -1;
+ }
+
+ while (fgets (buf, sizeof (buf), status_file))
+ if (strncmp (buf, field, field_len) == 0 && buf[field_len] == ':')
+ {
+ size_t pos = field_len + 1;
+ while((buf[pos] == ' ' || buf[pos] == '\t')
+ && (pos < sizeof (buf) - 1))
+ pos++;
+
+ strncpy (target, &buf[pos], t_size);
+ target[t_size - 1] = '\0';
+ retval = strlen (target);
+ if(retval > 0 && target[retval - 1] == '\n')
+ target[retval - 1] = '\0';
+
+ break;
+ }
+
+ fclose (status_file);
+ return retval;
+}
+
/* Return the TGID of LWPID from /proc/pid/status. Returns -1 if not
found. */
@@ -118,3 +167,13 @@
{
return linux_proc_pid_has_state (pid, "Z (zombie)");
}
+
+/* Save the name of thread PID/TID into buffer NAME. SIZE is the size
+ of buffer NAME. Return the length of the name if success,
+ otherwise return -1. */
+
+int
+linux_proc_get_thread_name (pid_t pid, pid_t tid, char *name, size_t size)
+{
+ return linux_proc_get_string (pid, tid, name, size, "Name");
+}
diff -uNr ./gdb.org/common/linux-procfs.h ./gdb/common/linux-procfs.h
--- ./gdb.org/common/linux-procfs.h 2013-01-01 14:32:54.000000000 +0800
+++ ./gdb/common/linux-procfs.h 2013-08-16 11:48:49.000000000 +0800
@@ -40,4 +40,12 @@
extern int linux_proc_pid_is_zombie (pid_t pid);
+
+/* Save the name of thread PID/TID into buffer NAME. SIZE is the size
+ of buffer NAME. Return the length of the name if success,
+ otherwise return -1. */
+
+extern int linux_proc_get_thread_name (pid_t pid, pid_t tid,
+ char *name, size_t size);
+
#endif /* COMMON_LINUX_PROCFS_H */
diff -uNr ./gdb.org/linux-nat.c ./gdb/linux-nat.c
--- ./gdb.org/linux-nat.c 2013-02-13 22:59:49.000000000 +0800
+++ ./gdb/linux-nat.c 2013-08-16 13:22:31.000000000 +0800
@@ -4294,6 +4294,14 @@
fclose (comm_file);
}
+ else
+ {
+ static char comm[COMM_LEN + 1];
+ int size = linux_proc_get_thread_name (pid, lwp, comm, COMM_LEN + 1);
+ if (size > 0)
+ result = comm;
+
+ }
#undef COMM_LEN
#undef FORMAT
diff -uNr ./gdb.org/testsuite/ChangeLog ./gdb/testsuite/ChangeLog
--- ./gdb.org/testsuite/ChangeLog 2013-04-25 20:22:26.000000000 +0800
+++ ./gdb/testsuite/ChangeLog 2013-08-16 11:49:02.000000000 +0800
@@ -1,3 +1,9 @@
+2013-08-14 Will Huang <will.huang@aliyun-inc.com>
+
+ PR threads/15824
+ * gdb.threads/manythreads.exp: Add test for get thread
+ original name
+
2013-04-25 Sergio Durigan Junior <sergiodj@redhat.com>
* gdb.arch/arm-bl-branch-dest.c: New file.
diff -uNr ./gdb.org/testsuite/gdb.threads/manythreads.exp ./gdb/testsuite/gdb.threads/manythreads.exp
--- ./gdb.org/testsuite/gdb.threads/manythreads.exp 2013-01-01 14:41:27.000000000 +0800
+++ ./gdb/testsuite/gdb.threads/manythreads.exp 2013-08-16 11:49:26.000000000 +0800
@@ -93,6 +93,7 @@
}
}
+gdb_test "info threads" ".*manythreads.*" "check thread original name"
gdb_test_no_output "thread name zardoz" "give a name to the thread"
gdb_test "info threads" ".*zardoz.*" "check thread name"
Thanks,
Will Huang
> -----Original Message-----
> From: Yao Qi [mailto:yao@codesourcery.com]
> Sent: Friday, August 16, 2013 10:26 AM
> To: Will Huang
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH][PR threads/15824] when get threads name failed from info
> threads with linux kernel version earlier than 2.6.33
>
> Hi Will,
> I am not the 'right' person to review this patch, however, I find some thing
> obviously incorrect. I point them out here.
>
> On 08/14/2013 12:26 PM, Will Huang wrote:
> > diff -ruN ./gdb.org/ChangeLog ./gdb/ChangeLog
> > --- ./gdb.org/ChangeLog 2013-04-26 19:43:30.000000000 +0800
> > +++ ./gdb/ChangeLog 2013-08-14 11:09:08.000000000 +0800
> > @@ -1,3 +1,11 @@
> > +2013-08-14 Will Huang<will.huang@aliyun-inc.com>
> > +
> > + PR threads/15824
>
> Each line should be prefixed with 'TAB' instead of 8 spaces.
>
> > + * linux-nat.c (linux_nat_thread_name): Add get thread name from
> > + procfs "/proc/$pid/task/$tid/status".
> > + * commo/linux-procfs.c (linux_proc_get_thread_name): New.
> > + * commo/linux-procfs.c (linux_proc_get_string): New.
> ^^^^^^^^^^^^^^^^^^^^^^^ Duplicated.
>
> Only have to mention the same file once.
>
> > +
> > 2013-04-26 Joel Brobecker<brobecker@adacore.com>
> >
> > * NEWS: Change "since GDB 7.5" into "in GDB 7.6".
> > diff -ruN ./gdb.org/common/linux-procfs.c ./gdb/common/linux-procfs.c
> > --- ./gdb.org/common/linux-procfs.c 2013-01-01 14:32:54.000000000 +0800
> > +++ ./gdb/common/linux-procfs.c 2013-08-12 16:51:33.000000000 +0800
> > @@ -55,6 +55,51 @@
> > return retval;
> > }
> >
> > +/* Return the string length of Name from/proc/$pid/task/$tid/status.
>
> Space after "from" is needed.
> s/Name/FIELD
> s/$pid/LWPID
> s/$tid/TID
>
> > + Returns -1 if not found. */
>
> s/Returns/Return
>
> > +
> > +static int
> > +linux_proc_get_string (pid_t lwpid, pid_t tid,
> > + char *target, size_t t_size, const char
> > +*field)
>
> "char *target" should go to the previous line.
>
> > +{
> > + size_t field_len = strlen (field);
> > + FILE *status_file;
> > + char buf[100];
> > + int retval = -1;
> > +
> > + if (tid > 0)
> > + snprintf (buf, sizeof (buf),
> > + "/proc/%d/task/%d/status", (int) lwpid, (int) tid);
>
> Use xsnprintf, and the indentation looks wrong.
>
> > + else
> > + snprintf (buf, sizeof (buf), "/proc/%d/status", (int) lwpid);
> > +
>
> Likewise. Is it a dead path? Can 'tid' be negative?
>
> > + status_file = fopen (buf, "r");
>
> Use gdb_fopen_cloexec?
>
> > + if (status_file == NULL)
> > + {
> > + warning (_("unable to open /proc file '%s'"), buf);
> > + return -1;
> > + }
> > +
> > + while (fgets (buf, sizeof (buf), status_file))
> > + if (strncmp (buf, field, field_len) == 0 && buf[field_len] == ':')
> > + {
> > + size_t pos = field_len + 1;
> > + while((buf[pos] == ' ' || buf[pos] == '\t') && (pos < sizeof
> > + (buf) -
> > 1))
>
> Your mailer wrap this line...
>
> > + pos++;
> > +
> > + strncpy (target, &buf[pos], t_size);
> > + target[t_size - 1] = '\0';
> > + retval = strlen (target);
> > + if(retval > 0 && target[retval - 1] == '\n')
> > + target[retval - 1] = '\0';
> > +
> > + break;
> > + }
> > +
> > + fclose (status_file);
> > + return retval;
> > +}
> > +
> > /* Return the TGID of LWPID from /proc/pid/status. Returns -1 if not
> > found. */
> >
> > @@ -118,3 +163,11 @@
> > {
> > return linux_proc_pid_has_state (pid, "Z (zombie)");
> > }
> > +
> > +/*Get thread name identified by PID & TID*/
>
> I am not good at comments either, but I suggest
>
> /* Save the name of thread PID/TID into buffer NAME. SIZE is the size
> of buffer NAME. Return the length of the name if success,
> otherwise return -1. */
>
> > +
> > +int
> > +linux_proc_get_thread_name (pid_t pid, pid_t tid, char *name, size_t
> > +size) {
> > + return linux_proc_get_string (pid, tid, name, size, "Name"); }
> > diff -ruN ./gdb.org/common/linux-procfs.h ./gdb/common/linux-procfs.h
> > --- ./gdb.org/common/linux-procfs.h 2013-01-01 14:32:54.000000000
> +0800
> > +++ ./gdb/common/linux-procfs.h 2013-08-12 16:50:39.000000000 +0800
> > @@ -40,4 +40,10 @@
> >
> > extern int linux_proc_pid_is_zombie (pid_t pid);
> >
> > +/* Return -1 if thread Name of tid didn't found;
>
> Comments should be updated as what I suggested above...
>
> > + "char *name" for thread name (need be allocated by caller). */
> > +
> > +extern int linux_proc_get_thread_name (pid_t pid, pid_t tid,
> > + char *name, size_t size);
> > +
> > #endif /* COMMON_LINUX_PROCFS_H */
> > diff -ruN ./gdb.org/linux-nat.c ./gdb/linux-nat.c
> > --- ./gdb.org/linux-nat.c 2013-02-13 22:59:49.000000000 +0800
> > +++ ./gdb/linux-nat.c 2013-08-12 16:47:26.000000000 +0800
> > @@ -4294,6 +4294,14 @@
> >
> > fclose (comm_file);
> > }
> > + else
> > + {
> > + static char comm[COMM_LEN + 1];
> > + int size = linux_proc_get_thread_name (pid, lwp, comm,
> > + COMM_LEN+1);
>
> A blank line is needed. Space is needed before and after "+".
>
> The type of pid is int and type of lwp is long, but the types of parameters of
> linux_proc_get_thread_name are pid_t and pid_t. It is inconsistent.
>
> --
> Yao (齐尧)
More information about the Gdb-patches
mailing list