Re: [PING][RFC-v4] Add windows OS Thread Information Block

[Pedro Alves <pedro at codesourcery dot com>]

On Thursday 01 April 2010 10:40:50, Pierre Muller wrote:
>   I am still waiting for a comment from any global
> maintainer concerning the non-(windows specific) parts
> of that patch. Christopher approved the windows part.

It is my intention to look at it.  It's taking me a
bit because it's biggish.  On a quick look (not a formal
review) I saw:

 - what looked like some mixed over returning the TIB
   or the TLB.  E.g.:

> +/* Write Windows OS Thread Information Block address.  */
> +static int
> +win32_get_tib_address (ptid_t ptid, CORE_ADDR *addr)
             ^^^
> +{
> +  win32_thread_info *th;
> +  th = thread_rec (ptid, 0);
> +  if (th == NULL)
> +    return 0;
> +  if (addr)
> +    *addr = (CORE_ADDR) th->thread_local_base;
> +  if (th->thread_local_base)
         ^^^^^^^^^^^^^^^^^^^^^
> +    return 1;

  Does the new packet return the TIB, or the TLB?
  The object is $_tlb now, isn't it?

> > +tlb_value_read (struct value *val)
> > +{
...
> > +  if (!target_get_tib_address (inferior_ptid, &tlb))
> > +    error (_("Unable to read tlb"));

  Either this is quite confused, or I am.



 - Assumptions that GDB thread ids are always the
   same as Win32 threads ids.

> > +/* Display thread information block of a thread specified by ARGS.
> > +   If ARGS is empty, display thread information block of
> > current_thread
> > +   if current_thread is non NULL.
> > +   Otherwise ARGS is parsed and converted to a integer that should
> > +   be the windows ThreadID (not the internal GDB thread ID).  */
> > +static void
> > +display_tib (char * args, int from_tty)
> > +{
> > +  if (args)
> > +    {
> > +      ULONGEST id = (ULONGEST) parse_and_eval_long (args);
> > +      display_one_tib (ptid_build (ptid_get_pid (inferior_ptid), 0,
> > id));


> > +  if (target_get_tib_address (ptid, &thread_local_base) == 0)
> > +    {
> > +      printf_filtered ("Unable to get thread local base for ThreadId
> > %s\n",
> > +     pulongest (ptid_get_tid(ptid)));
> > +      return -1;

There's no garantee the TID field of ptid matches a windows
thread id , particularly when remote debugging (read: that
it will always be that way).  Do you really need to make this
bypass the internal GDB thread id?  It would avoid pain if
this always worked with the GDB thread id instead.

-- 
Pedro Alves