From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14488 invoked by alias); 27 Jul 2009 20:17:55 -0000 Received: (qmail 14464 invoked by uid 22791); 27 Jul 2009 20:17:52 -0000 X-SWARE-Spam-Status: No, hits=-0.9 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_43,J_CHICKENPOX_63,J_CHICKENPOX_65,ZMIde_GENERICSPAM1 X-Spam-Check-By: sourceware.org Received: from qnxmail.qnx.com (HELO qnxmail.qnx.com) (209.226.137.76) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 27 Jul 2009 20:17:44 +0000 Received: from Nebula.ott.qnx.com (nebula.ott.qnx.com [10.42.3.30]) by hub.ott.qnx.com (8.9.3/8.9.3) with ESMTP id QAA29910; Mon, 27 Jul 2009 16:17:35 -0400 Received: from [127.0.0.1] ([10.42.161.192]) by Nebula.ott.qnx.com with Microsoft SMTPSVC(6.0.3790.3959); Mon, 27 Jul 2009 16:17:41 -0400 Message-ID: <4A6E0B5F.6090901@qnx.com> Date: Mon, 27 Jul 2009 20:55:00 -0000 From: Aleksandar Ristovski User-Agent: Thunderbird 2.0.0.22 (Windows/20090605) MIME-Version: 1.0 To: Pedro Alves CC: gdb-patches@sourceware.org Subject: Re: nto_extra_thread_info References: <200907241516.48134.pedro@codesourcery.com> In-Reply-To: <200907241516.48134.pedro@codesourcery.com> Content-Type: multipart/mixed; boundary="------------060503060103000405050101" 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: 2009-07/txt/msg00663.txt.bz2 This is a multi-part message in MIME format. --------------060503060103000405050101 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 4099 Hello Pedro, Pedro Alves wrote: > On Friday 12 June 2009 19:57:02, Aleksandar Ristovski wrote: > >> This patch introduces extra thread info for nto target, and >> fixes a bug in procfs_find_new_threads. > > >> Index: gdb/nto-procfs.c >> =================================================================== >> --- gdb/nto-procfs.c (revision 2) >> +++ gdb/nto-procfs.c (working copy) >> @@ -220,11 +220,94 @@ static int >> procfs_thread_alive (struct target_ops *ops, ptid_t ptid) >> { > >> + >> + status.tid = tid; >> + if ((err = devctl (ctl_fd, DCMD_PROC_TIDSTATUS, >> + &status, sizeof (status), 0)) != EOK) >> + return 0; >> + >> + /* Thread is alive or dead but not yet joined, >> + or dead and there is an alive (or dead unjoined) thread with >> + higher tid. We return tidinfo. > > ^ Missing space. > > This "We return tidinfo." looks out of place though. > I suspect this whole comment was copied from the devctl's docs or > implementation. Indeed. I copied it. (Ironically, I also wrote it where I copied it from). I changed wording to be more applicable to this place. > >> + >> + Client should check if the tid is the same as >> + requested; if not, requested tid is dead. */ >> + return (status.tid == tid) && (status.state != STATE_DEAD); >> +} >> + >> +static void >> +update_thread_private_data_name (struct thread_info *new_thread, >> + const char *newname) >> +{ > > >> + >> + if (!new_thread->private) >> + { >> + new_thread->private = xmalloc (sizeof (struct private_thread_info) >> + + newnamelen + 1); > > Note: pedanticaly, '+ 1' here isn't really needed, since the name field was > declared as name[1]. FYI, a standard practice is to use > offsetof(struct foo, flexiblearraymember) + arraysize instead of sizeof. Changed to "offsetof". > >> +static void >> +update_thread_private_data (struct thread_info *new_thread, >> + pthread_t tid, int state, int flags) >> +{ >> + struct private_thread_info *pti; >> + procfs_info pidinfo; >> + struct _thread_name *tn; >> + procfs_threadctl tctl; >> +#if _NTO_VERSION > 630 >> + gdb_assert (new_thread != NULL); >> + >> + if (devctl (ctl_fd, DCMD_PROC_INFO, &pidinfo, >> + sizeof(pidinfo), 0) != EOK) >> + return; > > Should this set the private data to a known state, or > release it, so it doesn't show stale data? (can this really > happen?) I don't think so; if devctl returned != EOK, we really don't know anything about the thread and it will be deleted from the thread list... even if it happens that gdb still prints info about this thread, I think printing name is still o.k. since status should now say "DEAD". > >> 2009-06-12 Aleksandar Ristovski >> >> * nto-tdep.c (nto_thread_state_str): New array. >> (nto_extra_thread_info): New function definition. >> * nto-tdep.h (gdbthread.h): New include. >> (private_thread_info): New struct. >> (nto_extra_thread_info): New declaration. >> * nto-procfs.c (procfs_thread_alive): Properly check if >> thread is still alive. >> (update_thread_private_data_name, update_thread_private_data): New >> function definition. >> (procfs_find_new_threads): Fetch thread private data. >> (init_procfs_ops): Register to_extra_thread_info. > > Otherwise, looks okay to me. > New patch attached with fixes above. Thanks, -- Aleksandar Ristovski QNX Software Systems * nto-tdep.c (nto_thread_state_str): New array. (nto_extra_thread_info): New function definition. * nto-tdep.h (gdbthread.h): New include. (private_thread_info): New struct. (nto_extra_thread_info): New declaration. * nto-procfs.c (procfs_thread_alive): Properly check if thread is still alive. (update_thread_private_data_name, update_thread_private_data): New function definition. (procfs_find_new_threads): Fetch thread private data. (init_procfs_ops): Register to_extra_thread_info. --------------060503060103000405050101 Content-Type: text/x-patch; name="nto_extra_thread_info-20090727.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="nto_extra_thread_info-20090727.diff" Content-length: 6724 Index: gdb/nto-tdep.c =================================================================== RCS file: /cvs/src/src/gdb/nto-tdep.c,v retrieving revision 1.34 diff -u -p -r1.34 nto-tdep.c --- gdb/nto-tdep.c 12 Jun 2009 02:32:10 -0000 1.34 +++ gdb/nto-tdep.c 27 Jul 2009 20:07:56 -0000 @@ -341,6 +341,40 @@ nto_elf_osabi_sniffer (bfd *abfd) return GDB_OSABI_UNKNOWN; } +static const char *nto_thread_state_str[] = +{ + "DEAD", /* 0 0x00 */ + "RUNNING", /* 1 0x01 */ + "READY", /* 2 0x02 */ + "STOPPED", /* 3 0x03 */ + "SEND", /* 4 0x04 */ + "RECEIVE", /* 5 0x05 */ + "REPLY", /* 6 0x06 */ + "STACK", /* 7 0x07 */ + "WAITTHREAD", /* 8 0x08 */ + "WAITPAGE", /* 9 0x09 */ + "SIGSUSPEND", /* 10 0x0a */ + "SIGWAITINFO", /* 11 0x0b */ + "NANOSLEEP", /* 12 0x0c */ + "MUTEX", /* 13 0x0d */ + "CONDVAR", /* 14 0x0e */ + "JOIN", /* 15 0x0f */ + "INTR", /* 16 0x10 */ + "SEM", /* 17 0x11 */ + "WAITCTX", /* 18 0x12 */ + "NET_SEND", /* 19 0x13 */ + "NET_REPLY" /* 20 0x14 */ +}; + +char * +nto_extra_thread_info (struct thread_info *ti) +{ + if (ti && ti->private + && ti->private->state < ARRAY_SIZE (nto_thread_state_str)) + return (char *)nto_thread_state_str [ti->private->state]; + return ""; +} + void nto_initialize_signals (void) { Index: gdb/nto-tdep.h =================================================================== RCS file: /cvs/src/src/gdb/nto-tdep.h,v retrieving revision 1.16 diff -u -p -r1.16 nto-tdep.h --- gdb/nto-tdep.h 12 Jun 2009 02:32:10 -0000 1.16 +++ gdb/nto-tdep.h 27 Jul 2009 20:07:56 -0000 @@ -25,6 +25,7 @@ #include "solist.h" #include "osabi.h" #include "regset.h" +#include "gdbthread.h" /* Target operations defined for Neutrino targets (-nto-tdep.c). */ @@ -138,6 +139,14 @@ typedef struct _debug_regs qnx_reg64 padding[1024]; } nto_regset_t; +struct private_thread_info +{ + short tid; + unsigned char state; + unsigned char flags; + char name[1]; +}; + /* Generic functions in nto-tdep.c. */ void nto_init_solib_absolute_prefix (void); @@ -162,4 +171,6 @@ void nto_dummy_supply_regset (struct reg int nto_in_dynsym_resolve_code (CORE_ADDR pc); +char *nto_extra_thread_info (struct thread_info *); + #endif Index: gdb/nto-procfs.c =================================================================== RCS file: /cvs/src/src/gdb/nto-procfs.c,v retrieving revision 1.47 diff -u -p -r1.47 nto-procfs.c --- gdb/nto-procfs.c 2 Jul 2009 17:12:25 -0000 1.47 +++ gdb/nto-procfs.c 27 Jul 2009 20:07:57 -0000 @@ -220,11 +220,94 @@ static int procfs_thread_alive (struct target_ops *ops, ptid_t ptid) { pid_t tid; + pid_t pid; + procfs_status status; + int err; tid = ptid_get_tid (ptid); - if (devctl (ctl_fd, DCMD_PROC_CURTHREAD, &tid, sizeof (tid), 0) == EOK) - return 1; - return 0; + pid = ptid_get_pid (ptid); + + if (kill (pid, 0) == -1) + return 0; + + status.tid = tid; + if ((err = devctl (ctl_fd, DCMD_PROC_TIDSTATUS, + &status, sizeof (status), 0)) != EOK) + return 0; + + /* Thread is alive or dead but not yet joined, + or dead and there is an alive (or dead unjoined) thread with + higher tid. + + If the tid is not the same as requested; requested tid is dead. */ + return (status.tid == tid) && (status.state != STATE_DEAD); +} + +static void +update_thread_private_data_name (struct thread_info *new_thread, + const char *newname) +{ + int newnamelen; + struct private_thread_info *pti; + gdb_assert (newname != NULL); + gdb_assert (new_thread != NULL); + + newnamelen = strlen (newname); + + if (!new_thread->private) + { + new_thread->private = xmalloc (offsetof(struct private_thread_info, name) + + newnamelen + 1); + memcpy (new_thread->private->name, newname, newnamelen + 1); + } + else if (strcmp (newname, new_thread->private->name) != 0) + { + /* Reallocate if neccessary. */ + int oldnamelen = strlen (new_thread->private->name); + if (oldnamelen < newnamelen) + new_thread->private = xrealloc (new_thread->private, + offsetof(struct private_thread_info, + name) + + newnamelen + 1); + memcpy (new_thread->private->name, newname, newnamelen + 1); + } +} + +static void +update_thread_private_data (struct thread_info *new_thread, + pthread_t tid, int state, int flags) +{ + struct private_thread_info *pti; + procfs_info pidinfo; + struct _thread_name *tn; + procfs_threadctl tctl; +#if _NTO_VERSION > 630 + gdb_assert (new_thread != NULL); + + if (devctl (ctl_fd, DCMD_PROC_INFO, &pidinfo, + sizeof(pidinfo), 0) != EOK) + return; + + memset (&tctl, 0, sizeof (tctl)); + tctl.cmd = _NTO_TCTL_NAME; + tn = (struct _thread_name *) (&tctl.data); + + /* Fetch name for the given thread. */ + tctl.tid = tid; + tn->name_buf_len = sizeof (tctl.data) - sizeof (*tn); + tn->new_name_len = -1; /* Getting, not setting. */ + if (devctl (ctl_fd, DCMD_PROC_THREADCTL, &tctl, sizeof (tctl), NULL) != EOK) + tn->name_buf[0] = '\0'; + + tn->name_buf[_NTO_THREAD_NAME_MAX] = '\0'; + + update_thread_private_data_name (new_thread, tn->name_buf); + + pti = (struct private_thread_info *) new_thread->private; + pti->tid = tid; + pti->state = state; + pti->flags = flags; +#endif /* _NTO_VERSION */ } void @@ -233,20 +316,33 @@ procfs_find_new_threads (struct target_o procfs_status status; pid_t pid; ptid_t ptid; + pthread_t tid; + struct thread_info *new_thread; if (ctl_fd == -1) return; pid = ptid_get_pid (inferior_ptid); - for (status.tid = 1;; ++status.tid) + status.tid = 1; + + for (tid = 1;; ++tid) { - if (devctl (ctl_fd, DCMD_PROC_TIDSTATUS, &status, sizeof (status), 0) - != EOK && status.tid != 0) + if (status.tid == tid + && (devctl (ctl_fd, DCMD_PROC_TIDSTATUS, &status, sizeof (status), 0) + != EOK)) break; - ptid = ptid_build (pid, 0, status.tid); - if (!in_thread_list (ptid)) - add_thread (ptid); + if (status.tid != tid) + /* The reason why this would not be equal is that devctl might have + returned different tid, meaning the requested tid no longer exists + (e.g. thread exited). */ + continue; + ptid = ptid_build (pid, 0, tid); + new_thread = find_thread_ptid (ptid); + if (!new_thread) + new_thread = add_thread (ptid); + update_thread_private_data (new_thread, tid, status.state, 0); + status.tid++; } return; } @@ -1338,6 +1434,7 @@ init_procfs_ops (void) procfs_ops.to_has_execution = default_child_has_execution; procfs_ops.to_magic = OPS_MAGIC; procfs_ops.to_have_continuable_watchpoint = 1; + procfs_ops.to_extra_thread_info = nto_extra_thread_info; } #define OSTYPE_NTO 1 --------------060503060103000405050101--