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

[Pedro Alves <pedro at codesourcery dot com>]

On Thursday 01 April 2010 13:56:50, Pierre Muller wrote:

> 2010-03-15  Pierre Muller  <muller@ics.u-strasbg.fr>
> 
> 	* remote.c (PACKET_qGetTIBAddr): New enum element.
> 	(remote_get_tib_address): New function.
> 	(init_remote_ops): Set TO_GET_TIB_ADDRESS field
> 	to remote_get_tib_address.
> 	(_initialize_remote): Add add_packet_config_cmd
> 	for PACKET_qGetTIBAddr.
> 	* target.c (update_current_target): Set default value for
> 	new TO_GET_TIB_ADDRESS field.
> 	* target.h (target_ops): New field TO_GET_TIB_ADDRESS.
> 	(target_get_tib_address): New macro.
> 	
> 	* windows-nat.c (thread_info): Add THREAD_LOCAL_BASE field.
> 	(windows_add_thread): Add TLB argument of type 'void *'.
> 	(fake_create_process): Adapt windows_add_thread call.
> 	(get_windows_debug_event): Idem.
> 	(windows_get_tib_address): New function.
> 	(init_windows_ops): Set TO_GET_TIB_ADDRESS field
> 	to remote_get_tib_address.
> 	(_initialize_windows_nat): Replace info_w32_cmdlist
> 	initialization by a call to init_w32_command_list.
> 	(info_w32_command, info_w32_cmdlist): Removed from here...
> 	to windows-tdep.c file.
> 	*  windows-tdep.h (info_w32_cmdlist): Declare.
> 	(init_w32_command_list): New external function 
> 	declaration.
> 	* windows-tdep.c: Add several headers.
> 	(info_w32_cmdlist): to here, made global.
> 	(thread_information_32): New struct.
> 	(thread_information_64): New struct.
> 	(TIB_NAME): New char array.
> 	(MAX_TIB32, MAX_TIB64, FULL_TIB_SIZE): New constants.
> 	(maint_display_all_tib): New static variable.
> 	(windows_get_tlb_type): New function.
> 	(tlb_value_read, tlb_value_write): New functions.
> 	(tlb_value_funcs): New static struct.
> 	(tlb_make_value): New function.
> 	(display_one_tib): New function.
> 	(display_tib): New function.
> 	(info_w32_command): Moved from windows-nat.c.
> 	(init_w32_command_list): New function.
> 	(_initialize_windows_tdep): New function.
> 	New "maint set/show show-all-tib" command
> 	New "$_tlb" internal variable.
> 
> gdbserver/ChangeLog entry:
> 
> 	* server.c (handle_query): Acknowledge support
> 	for 'qGetTIBAddr' if GET_TIB_ADDR field of THE_TARGET
> 	is set.
> 	Handle 'qGetTIBAddr' query.
> 	* target.h (target_ops): New GET_TIB_ADDRESS field.
> 	* win32-low.h (win32_thread_info): Add THREAD_LOCAL_BASE field.
> 	* win32-low.c (child_add_thread): Add TLB argument.
> 	Set THREAD_LOCAL_BASE field to TLB argument.
> 	(get_child_debug_event): Adapt to child_add_thread change.
> 	(win32_get_tib_address): New function.
> 	(win32_target_ops): Set GET_TIB_ADDRESS field to
> 	win32_get_tib_address

Uppercasing variables names is for talking about a
variable's _value_, not name:

http://www.gnu.org/prep/standards/standards.html#Comments:

 "The variable name itself should be lower case, but write it in upper case
 when you are speaking about the value rather than the variable itself. Thus,
 âthe inode number NODE_NUMâ rather than âan inodeâ."


> 	
> 
> doc/ChangeLog entry:
> 
> 	gdb.texinfo ($__tlb): Document new automatic convience variable.
> 	(info w32 thread-information-block): Docmuent new command.
> 	(qGetTIBAddress): Document new gdbserver query.
> 	(maint set/show show-all-tib): Document new command.

Typos: convience, Docmuent

> 
> Index: remote.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/remote.c,v
> retrieving revision 1.397
> diff -u -p -r1.397 remote.c
> --- remote.c	31 Mar 2010 14:36:41 -0000	1.397
> +++ remote.c	1 Apr 2010 12:46:43 -0000
> @@ -1139,6 +1139,7 @@ enum {
>    PACKET_qXfer_spu_write,
>    PACKET_qXfer_osdata,
>    PACKET_qXfer_threads,
> +  PACKET_qGetTIBAddr,
>    PACKET_qGetTLSAddr,
>    PACKET_qSupported,
>    PACKET_QPassSignals,
> @@ -8441,6 +8442,48 @@ remote_get_thread_local_address (struct 
>    return 0;
>  }
>  
> +/* Provide thread local base, i.e. Thread Information Block address.  */
> +/* Returns 1 if ptid is found and thread_local_base is non zero.  */
> +int

Merge both comments in a single /**/ block.  Please add an empty
line between comment and function (everywhere).

> +remote_get_tib_address (ptid_t ptid, CORE_ADDR *addr)
> +{
> +  if (remote_protocol_packets[PACKET_qGetTIBAddr].support !=
> PACKET_DISABLE)
> +    {
> +      struct remote_state *rs = get_remote_state ();
> +      char *p = rs->buf;
> +      char *endp = rs->buf + get_remote_packet_size ();
> +      enum packet_result result;
> +
> +      strcpy (p, "qGetTIBAddr:");
> +      p += strlen (p);
> +      p = write_ptid (p, endp, ptid);
> +      *p++ = '\0';
> +
> +      putpkt (rs->buf);
> +      getpkt (&rs->buf, &rs->buf_size, 0);
> +      result = packet_ok (rs->buf,
> +			  &remote_protocol_packets[PACKET_qGetTIBAddr]);
> +      if (result == PACKET_OK)
> +	{
> +	  ULONGEST result;
> +
> +	  unpack_varlen_hex (rs->buf, &result);
> +	  if (addr)
> +	    *addr = (CORE_ADDR) result;
> +	  return 1;
> +	}
> +      else if (result == PACKET_UNKNOWN)
> +	error (_("Remote target doesn't support qGetTIBAddr packet"));
> +      else
> +	error (_("Remote target failed to process qGetTIBAddr request"));
> +    }
> +  else
> +    error (_("qGetTIBAddr not supported or disabled on this target"));
> +  /* Not reached.  */
> +  return 0;
> +}
> +
> +
>  /* Support for inferring a target description based on the current
>     architecture and the size of a 'g' packet.  While the 'g' packet
>     can have any size (since optional registers can be left off the
> @@ -9885,6 +9928,8 @@ Specify the serial device it is connecte
>    remote_ops.to_set_circular_trace_buffer =
> remote_set_circular_trace_buffer;
>    remote_ops.to_core_of_thread = remote_core_of_thread;
>    remote_ops.to_verify_memory = remote_verify_memory;
> +  remote_ops.to_get_tib_address = remote_get_tib_address;
> + 
>  }
>  
>  /* Set up the extended remote vector by making a copy of the standard
> @@ -10304,6 +10349,10 @@ Show the maximum size of the address (in
>  			 "qGetTLSAddr", "get-thread-local-storage-address",
>  			 0);
>  
> +  add_packet_config_cmd (&remote_protocol_packets[PACKET_qGetTIBAddr],
> +			 "qGetTIBAddr",
> "get-thread-information-block-address",
> +			 0);
> +

It looks like the patch got line-wrap mangled here, and
in several other places.  Could you make sure line-wrapping
is disabled when you paste patches in the email?  Thanks.


>    add_packet_config_cmd (&remote_protocol_packets[PACKET_bc],
>  			 "bc", "reverse-continue", 0);
>  
> Index: target.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/target.c,v
> retrieving revision 1.245
> diff -u -p -r1.245 target.c
> --- target.c	24 Mar 2010 01:12:13 -0000	1.245
> +++ target.c	1 Apr 2010 12:46:44 -0000
> @@ -660,6 +660,7 @@ update_current_target (void)
>        INHERIT (to_get_raw_trace_data, t);
>        INHERIT (to_set_disconnected_tracing, t);
>        INHERIT (to_set_circular_trace_buffer, t);
> +      INHERIT (to_get_tib_address, t);
>        INHERIT (to_magic, t);
>        /* Do not inherit to_memory_map.  */
>        /* Do not inherit to_flash_erase.  */
> @@ -853,6 +854,9 @@ update_current_target (void)
>    de_fault (to_set_circular_trace_buffer,
>  	    (void (*) (int))
>  	    target_ignore);
> +  de_fault (to_get_tib_address,
> +	    (int (*) (ptid_t, CORE_ADDR *))
> +	    tcomplain);
>  #undef de_fault
>  
>    /* Finally, position the target-stack beneath the squashed
> Index: target.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/target.h,v
> retrieving revision 1.177
> diff -u -p -r1.177 target.h
> --- target.h	24 Mar 2010 01:12:13 -0000	1.177
> +++ target.h	1 Apr 2010 12:46:45 -0000
> @@ -682,6 +682,10 @@ struct target_ops
>      int (*to_verify_memory) (struct target_ops *, const gdb_byte *data,
>  			     CORE_ADDR memaddr, ULONGEST size);
>  
> +    /* Return the address of the start of the Thread Information Block
> +       a windows OS specific feature.  */

Windows.

> +    int (*to_get_tib_address) (ptid_t ptid, CORE_ADDR *addr);
> +
>      int to_magic;
>      /* Need sub-structure for target machine related rather than comm
> related?
>       */
> @@ -1371,6 +1375,9 @@ extern int target_search_memory (CORE_AD
>  #define	target_set_circular_trace_buffer(val)	\
>    (*current_target.to_set_circular_trace_buffer) (val)
>  
> +#define target_get_tib_address(ptid, addr) \
> +  (*current_target.to_get_tib_address) ((ptid), (addr))
> +
>  /* Command logging facility.  */
>  
>  #define target_log_command(p)						\
> Index: windows-nat.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/windows-nat.c,v
> retrieving revision 1.207
> diff -u -p -r1.207 windows-nat.c
> --- windows-nat.c	10 Mar 2010 15:57:07 -0000	1.207
> +++ windows-nat.c	1 Apr 2010 12:46:45 -0000
> @@ -191,6 +191,7 @@ typedef struct thread_info_struct
>      struct thread_info_struct *next;
>      DWORD id;
>      HANDLE h;
> +    CORE_ADDR thread_local_base;
>      char *name;
>      int suspended;
>      int reload_context;
> @@ -320,7 +321,7 @@ thread_rec (DWORD id, int get_context)
>  
>  /* Add a thread to the thread list.  */
>  static thread_info *
> -windows_add_thread (ptid_t ptid, HANDLE h)
> +windows_add_thread (ptid_t ptid, HANDLE h, void *tlb)
>  {
>    thread_info *th;
>    DWORD id;
> @@ -335,6 +336,7 @@ windows_add_thread (ptid_t ptid, HANDLE 
>    th = XZALLOC (thread_info);
>    th->id = id;
>    th->h = h;
> +  th->thread_local_base = (CORE_ADDR) (uintptr_t) tlb;
>    th->next = thread_head.next;
>    thread_head.next = th;
>    add_thread (ptid);
> @@ -1074,15 +1076,6 @@ display_selectors (char * args, int from
>      }
>  }
>  
> -static struct cmd_list_element *info_w32_cmdlist = NULL;
> -
> -static void
> -info_w32_command (char *args, int from_tty)
> -{
> -  help_list (info_w32_cmdlist, "info w32 ", class_info, gdb_stdout);
> -}
> -
> -
>  #define DEBUG_EXCEPTION_SIMPLE(x)       if (debug_exceptions) \
>    printf_unfiltered ("gdb: Target exception %s at %s\n", x, \
>      host_address_to_string (\
> @@ -1271,9 +1264,11 @@ fake_create_process (void)
>        /*  We can not debug anything in that case.  */
>      }
>    main_thread_id = current_event.dwThreadId;
> -  current_thread = windows_add_thread (ptid_build
> (current_event.dwProcessId, 0,
> -
> current_event.dwThreadId),
> -
> current_event.u.CreateThread.hThread);
> +  current_thread = windows_add_thread (
> +		     ptid_build (current_event.dwProcessId, 0,
> +				 current_event.dwThreadId),
> +		     current_event.u.CreateThread.hThread,
> +		     current_event.u.CreateThread.lpThreadLocalBase);
>    return main_thread_id;
>  }
>  
> @@ -1447,7 +1442,9 @@ get_windows_debug_event (struct target_o
>        retval = current_event.dwThreadId;
>        th = windows_add_thread (ptid_build (current_event.dwProcessId, 0,
>  					 current_event.dwThreadId),
> -			     current_event.u.CreateThread.hThread);
> +			     current_event.u.CreateThread.hThread,
> +
> current_event.u.CreateThread.lpThreadLocalBase);
> +
>        break;
>  
>      case EXIT_THREAD_DEBUG_EVENT:
> @@ -1481,7 +1478,8 @@ get_windows_debug_event (struct target_o
>        /* Add the main thread */
>        th = windows_add_thread (ptid_build (current_event.dwProcessId, 0,
>  					   current_event.dwThreadId),
> -			       current_event.u.CreateProcessInfo.hThread);
> +	     current_event.u.CreateProcessInfo.hThread,
> +	     current_event.u.CreateProcessInfo.lpThreadLocalBase);
>        retval = current_event.dwThreadId;
>        break;
>  
> @@ -2266,6 +2264,26 @@ windows_xfer_partial (struct target_ops 
>      }
>  }
>  
> +/* Provide thread local base, i.e. Thread Information Block address.  */
> +/* Returns 1 if ptid is found and thread_local_base is non zero.  */
> +int

Same here regarding the /**/ merging, and empty line missing.

> +windows_get_tib_address (ptid_t ptid, CORE_ADDR *addr)
> +{
> +  thread_info *th;
> +
> +  th = thread_rec (ptid_get_tid (ptid), 0);
> +  if (th == NULL)
> +    return 0;
> +
> +  if (addr)
> +    {
> +      *addr = (CORE_ADDR) th->thread_local_base;
> +    }

One line statements don't get a wrapped in {}, just
like you have correctly done above.  Drop the
cast, as the new thread_local_base field is already
a CORE_ADDR.  In one case, you've checked a pointer
against NULL, and in the other you haven't.  Please
make this all consistent:

  if (addr != NULL)
    *addr = th->thread_local_base;

> +  if (th->thread_local_base)
> +    return 1;

Style wise, it looks strange to fill *addr even when
th->thread_local_base would be 0, and hence you'd return
0 below.

> +  return 0;
> +}

Here's what I'd suggest:

  th = thread_rec (ptid_get_tid (ptid), 0);
  if (th == NULL || th->thread_local_base == 0)
    return 0;

  if (addr != NULL)
    *addr = th->thread_local_base;

  return 1;



> +
>  static ptid_t
>  windows_get_ada_task_ptid (long lwp, long thread)
>  {
> @@ -2314,6 +2332,7 @@ init_windows_ops (void)
>    windows_ops.to_has_execution = default_child_has_execution;
>    windows_ops.to_pid_to_exec_file = windows_pid_to_exec_file;
>    windows_ops.to_get_ada_task_ptid = windows_get_ada_task_ptid;
> +  windows_ops.to_get_tib_address = windows_get_tib_address;
>  
>    i386_use_watchpoints (&windows_ops);
>  
> @@ -2415,9 +2434,7 @@ Show whether to display kernel exception
>  			   NULL, /* FIXME: i18n: */
>  			   &setlist, &showlist);
>  
> -  add_prefix_cmd ("w32", class_info, info_w32_command,
> -		  _("Print information specific to Win32 debugging."),
> -		  &info_w32_cmdlist, "info w32 ", 0, &infolist);
> +  init_w32_command_list ();
>  
>    add_cmd ("selector", class_info, display_selectors,
>  	   _("Display selectors infos."),
> Index: windows-tdep.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/windows-tdep.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 windows-tdep.c
> --- windows-tdep.c	1 Jan 2010 07:31:46 -0000	1.5
> +++ windows-tdep.c	1 Apr 2010 12:46:45 -0000
> @@ -19,6 +19,372 @@
>  #include "windows-tdep.h"
>  #include "gdb_obstack.h"
>  #include "xml-support.h"
> +#include "gdbarch.h"
> +#include "target.h"
> +#include "value.h"
> +#include "inferior.h"
> +#include "command.h"
> +#include "gdbcmd.h"
> +#include "gdbthread.h"
> +
> +struct cmd_list_element *info_w32_cmdlist;
> +
> +typedef struct thread_information_block_32
> +  {
> +    uint32_t current_seh;			/* %fs:0x0000 */
> +    uint32_t current_top_of_stack; 		/* %fs:0x0004 */
> +    uint32_t current_bottom_of_stack;		/* %fs:0x0008 */
> +    uint32_t sub_system_tib;			/* %fs:0x000c */
> +    uint32_t fiber_data;			/* %fs:0x0010 */
> +    uint32_t arbitrary_data_slot;		/* %fs:0x0014 */
> +    uint32_t linear_address_tib;		/* %fs:0x0018 */
> +    uint32_t environment_pointer;		/* %fs:0x001c */
> +    uint32_t process_id;			/* %fs:0x0020 */
> +    uint32_t current_thread_id;			/* %fs:0x0024 */
> +    uint32_t active_rpc_handle;			/* %fs:0x0028 */
> +    uint32_t thread_local_storage;		/* %fs:0x002c */
> +    uint32_t process_environment_block;		/* %fs:0x0030 */
> +    uint32_t last_error_number;			/* %fs:0x0034 */
> +  }
> +thread_information_32;
> +
> +typedef struct thread_information_block_64
> +  {
> +    uint64_t current_seh;			/* %gs:0x0000 */
> +    uint64_t current_top_of_stack; 		/* %gs:0x0008 */
> +    uint64_t current_bottom_of_stack;		/* %gs:0x0010 */
> +    uint64_t sub_system_tib;			/* %gs:0x0018 */
> +    uint64_t fiber_data;			/* %gs:0x0020 */
> +    uint64_t arbitrary_data_slot;		/* %gs:0x0028 */
> +    uint64_t linear_address_tib;		/* %gs:0x0030 */
> +    uint64_t environment_pointer;		/* %gs:0x0038 */
> +    uint64_t process_id;			/* %gs:0x0040 */
> +    uint64_t current_thread_id;			/* %gs:0x0048 */
> +    uint64_t active_rpc_handle;			/* %gs:0x0050 */
> +    uint64_t thread_local_storage;		/* %gs:0x0058 */
> +    uint64_t process_environment_block;		/* %gs:0x0060 */
> +    uint64_t last_error_number;			/* %gs:0x0068 */
> +  }
> +thread_information_64;
> +
> +
> +static const
> +char* TIB_NAME[] =

static const char* TIB_NAME[] =

> +  {
> +    " current_seh                 ",	/* %fs:0x0000 */
> +    " current_top_of_stack        ", 	/* %fs:0x0004 */
> +    " current_bottom_of_stack     ",	/* %fs:0x0008 */
> +    " sub_system_tib              ",	/* %fs:0x000c */
> +    " fiber_data                  ",	/* %fs:0x0010 */
> +    " arbitrary_data_slot         ",	/* %fs:0x0014 */
> +    " linear_address_tib          ",	/* %fs:0x0018 */
> +    " environment_pointer         ",	/* %fs:0x001c */
> +    " process_id                  ",	/* %fs:0x0020 */
> +    " current_thread_id           ",	/* %fs:0x0024 */
> +    " active_rpc_handle           ",	/* %fs:0x0028 */
> +    " thread_local_storage        ",	/* %fs:0x002c */
> +    " process_environment_block   ",	/* %fs:0x0030 */
> +    " last_error_number           "	/* %fs:0x0034 */
> +  };
> +
> +static const int
> +MAX_TIB32 = sizeof (thread_information_32) / sizeof (uint32_t);
> +static const int
> +MAX_TIB64 = sizeof (thread_information_64) / sizeof (uint64_t);
> +static const int
> +FULL_TIB_SIZE = 0x1000;

Only functions need a line break before the function name.

> +
> +static int maint_display_all_tib = 0;
> +
> +/* Define ThreadLocalBase pointer type */

Missing period.  Two spaces after period.  Empty line between comment
and function.

> +struct type *
> +windows_get_tlb_type (struct gdbarch *gdbarch)
> +{
> +  struct type *dword_ptr_type, *dword32_type, *void_ptr_type;
> +  struct type *peb_ldr_type, *peb_ldr_ptr_type;
> +  struct type *peb_type, *peb_ptr_type, *list_type, *list_ptr_type;
> +  struct type *module_list_ptr_type;
> +  struct type *tib_type, *seh_type, *tib_ptr_type, *seh_ptr_type;
> +
> +  dword_ptr_type = arch_integer_type (gdbarch, gdbarch_ptr_bit (gdbarch),
> +				 1, "DWORD_PTR");
> +  dword32_type = arch_integer_type (gdbarch, 32,
> +				 1, "DWORD32");
> +  void_ptr_type = lookup_pointer_type (builtin_type
> (gdbarch)->builtin_void);
> +
> +  /* list entry */
> +
> +  list_type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
> +  TYPE_NAME (list_type) = xstrdup ("list");
> +
> +  list_ptr_type = arch_type (gdbarch, TYPE_CODE_PTR,
> +			    TYPE_LENGTH (void_ptr_type), NULL);
> +
> +  module_list_ptr_type = void_ptr_type;
> +
> +  append_composite_type_field (list_type, "forward_list",
> module_list_ptr_type);
> +  append_composite_type_field (list_type, "backward_list",
> +			       module_list_ptr_type);
> +
> +  /* Structured Exception Handler */
> +
> +  seh_type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
> +  TYPE_NAME (seh_type) = xstrdup ("seh");
> +
> +  seh_ptr_type = arch_type (gdbarch, TYPE_CODE_PTR,
> +			    TYPE_LENGTH (void_ptr_type), NULL);
> +  TYPE_TARGET_TYPE (seh_ptr_type) = seh_type;
> +
> +  append_composite_type_field (seh_type, "next_seh", seh_ptr_type);
> +  append_composite_type_field (seh_type, "handler", void_ptr_type);
> +
> +  /* struct _PEB_LDR_DATA */
> +  /* FIXME: 64bit layout is unknown.  */

Is this still unknown?

> +  peb_ldr_type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
> +  TYPE_NAME (peb_ldr_type) = xstrdup ("peb_ldr_data");
> +
> +  append_composite_type_field (peb_ldr_type, "length", dword32_type);
> +  append_composite_type_field (peb_ldr_type, "initialized", dword32_type);
> +  append_composite_type_field (peb_ldr_type, "ss_handle", void_ptr_type);
> +  append_composite_type_field (peb_ldr_type, "in_load_order", list_type);
> +  append_composite_type_field (peb_ldr_type, "in_memory_order", list_type);
> +  append_composite_type_field (peb_ldr_type, "in_init_order", list_type);
> +  append_composite_type_field (peb_ldr_type, "entry_in_progress",
> +			       void_ptr_type);
> +  peb_ldr_ptr_type = arch_type (gdbarch, TYPE_CODE_PTR,
> +			    TYPE_LENGTH (void_ptr_type), NULL);
> +  TYPE_TARGET_TYPE (peb_ldr_ptr_type) = peb_ldr_type;
> +
> +
> +  /* struct process environment block */
> +  peb_type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
> +  TYPE_NAME (peb_type) = xstrdup ("peb");
> +
> +  /* 4 first byte contain several flags.  */
> +  /* FIXME: 64bit layout is unknown.  */
> +  append_composite_type_field (peb_type, "flags", dword_ptr_type);
> +  append_composite_type_field (peb_type, "mutant", void_ptr_type);
> +  append_composite_type_field (peb_type, "image_base_address",
> void_ptr_type);
> +  append_composite_type_field (peb_type, "ldr", peb_ldr_ptr_type);
> +  append_composite_type_field (peb_type, "process_parameters",
> void_ptr_type);
> +  append_composite_type_field (peb_type, "sub_system_data", void_ptr_type);
> +  append_composite_type_field (peb_type, "process_heap", void_ptr_type);
> +  append_composite_type_field (peb_type, "fast_peb_lock", void_ptr_type);
> +  peb_ptr_type = arch_type (gdbarch, TYPE_CODE_PTR,
> +			    TYPE_LENGTH (void_ptr_type), NULL);
> +  TYPE_TARGET_TYPE (peb_ptr_type) = peb_type;
> +
> +
> +  /* struct thread information block */
> +  tib_type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
> +  TYPE_NAME (tib_type) = xstrdup ("tib");
> +
> +  /* uint32_t current_seh;			%fs:0x0000 */
> +  append_composite_type_field (tib_type, "current_seh", seh_ptr_type);
> +  /* uint32_t current_top_of_stack; 		%fs:0x0004 */
> +  append_composite_type_field (tib_type, "current_top_of_stack",
> void_ptr_type);
> +  /* uint32_t current_bottom_of_stack;		%fs:0x0008 */
> +  append_composite_type_field (tib_type, "current_bottom_of_stack",
> +			       void_ptr_type);
> +  /* uint32_t sub_system_tib;			%fs:0x000c */
> +  append_composite_type_field (tib_type, "sub_system_tib", void_ptr_type);
> +
> +  /* uint32_t fiber_data;			%fs:0x0010 */
> +  append_composite_type_field (tib_type, "fiber_data", void_ptr_type);
> +  /* uint32_t arbitrary_data_slot;		%fs:0x0014 */
> +  append_composite_type_field (tib_type, "arbitrary_data_slot",
> void_ptr_type);
> +  /* uint32_t linear_address_tib;		%fs:0x0018 */
> +  append_composite_type_field (tib_type, "linear_address_tib",
> void_ptr_type);
> +  /* uint32_t environment_pointer;		%fs:0x001c */
> +  append_composite_type_field (tib_type, "environment_pointer",
> void_ptr_type);
> +  /* uint32_t process_id;			%fs:0x0020 */
> +  append_composite_type_field (tib_type, "process_id", dword_ptr_type);
> +  /* uint32_t current_thread_id;		%fs:0x0024 */
> +  append_composite_type_field (tib_type, "thread_id", dword_ptr_type);
> +  /* uint32_t active_rpc_handle;		%fs:0x0028 */
> +  append_composite_type_field (tib_type, "active_rpc_handle",
> dword_ptr_type);
> +  /* uint32_t thread_local_storage;		%fs:0x002c */
> +  append_composite_type_field (tib_type, "thread_local_storage",
> void_ptr_type);
> +  /* uint32_t process_environment_block;	%fs:0x0030 */
> +  append_composite_type_field (tib_type, "process_environment_block",
> +			       peb_ptr_type);
> +  /* uint32_t last_error_number;		%fs:0x0034 */
> +  append_composite_type_field (tib_type, "last_error_number",
> dword_ptr_type);
> +
> +  tib_ptr_type = arch_type (gdbarch, TYPE_CODE_PTR,
> +			    TYPE_LENGTH (void_ptr_type), NULL);
> +  TYPE_TARGET_TYPE (tib_ptr_type) = tib_type;
> +
> +  return tib_ptr_type;
> +}
> +/* The $_tlb convenience variable is a bit special.  We don't know
> +   for sure the type of the value until we actually have a chance to
> +   fetch the data.  The type can change depending on gdbarch, so it it
> +   also dependent on which thread you have selected.
> +
> +     1. making $_tlb be an internalvar that creates a new value on
> +     access.
> +
> +     2. making the value of $_tlbi be an lval_computed value.  */
> +
> +/* This function implements the lval_computed support for reading a
> +   $_tlb value.  */
> +
> +static void
> +tlb_value_read (struct value *val)
> +{
> +  CORE_ADDR tlb;
> +  ULONGEST num;
> +  struct type *type = check_typedef (value_type (val));
> +  enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
> +  struct value *parent = value_parent (val);
> +  LONGEST offset = value_offset (val);

What are these two for?

> +  int length = TYPE_LENGTH (type);
> +
> +  /* This needs to be changed if multi-process support is added.  */
> +
> +  if (!target_get_tib_address (inferior_ptid, &tlb))
> +    error (_("Unable to read tlb"));
> +  num = (ULONGEST) tlb;
> +  store_unsigned_integer (value_contents_raw (val), length, byte_order,
> num);

This assumes `tlb' is an address, and VAL is a pointer, so, take a look
at value_from_pointer, and try doing something similar instead:
 
  store_typed_address (value_contents_raw (val), type, addr);

> +}
> +
> +/* This function implements the lval_computed support for writing a
> +   $_siginfo value.  */

Copy/paste error:  s/_siginfo/_tlb/

> +
> +static void
> +tlb_value_write (struct value *v, struct value *fromval)
> +{
> +  error (_("Impossible to change tlb"));
> +}
> +
> +static struct lval_funcs tlb_value_funcs =
> +  {
> +    tlb_value_read,
> +    tlb_value_write
> +  };
> +
> +
> +/* Return a new value with the correct type for the tlb object of
> +   the current thread using architecture GDBARCH.  Return a void value
> +   if there's no object available.  */
> +
> +static struct value *
> +tlb_make_value (struct gdbarch *gdbarch, struct internalvar *var)
> +{
> +  if (target_has_stack
> +      && !ptid_equal (inferior_ptid, null_ptid))
> +    {
> +      struct type *type = windows_get_tlb_type (gdbarch);
> +      return allocate_computed_value (type, &tlb_value_funcs, NULL);
> +    }
> +
> +  return allocate_value (builtin_type (gdbarch)->builtin_void);
> +}
> +
> +
> +/* Display thread information block of a given thread.  */
> +static int
> +display_one_tib (ptid_t ptid)
> +{
> +#define PTID_STRING_SIZE 40
> +  char annex[PTID_STRING_SIZE];
> +  char *annex_end = annex + PTID_STRING_SIZE;
> +  gdb_byte *tib = NULL;
> +  gdb_byte *index;
> +  CORE_ADDR thread_local_base;
> +  ULONGEST i, val, max, max_name, size, tib_size;
> +  ULONGEST sizeof_ptr = gdbarch_ptr_bit (target_gdbarch);
> +  enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch);
> +
> +  if (sizeof_ptr == 64)
> +    {
> +      size = sizeof (uint64_t);
> +      tib_size = sizeof (thread_information_64);
> +      max = MAX_TIB64;
> +    }
> +  else
> +    {
> +      size = sizeof (uint32_t);
> +      tib_size = sizeof (thread_information_32);
> +      max = MAX_TIB32;
> +    }
> +
> +  max_name = max;
> +
> +  if (maint_display_all_tib)
> +    {
> +      tib_size = FULL_TIB_SIZE;
> +      max = tib_size / size;
> +    }
> +  
> +  tib = alloca (tib_size);
> +
> +  /* This needs to be changed if multi-process support is added.  */

What needs changing?

> +
> +  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)));

Incomplete transition: please avoid the ptid_get_tid in common code.

> +      return -1;
> +    }
> +
> +  if (target_read (&current_target, TARGET_OBJECT_MEMORY,
> +		   annex, tib, thread_local_base, tib_size) != tib_size)
> +    {
> +      printf_filtered ("Unable to read thread information block for
> ThreadId %s at address %s\n",
> +	pulongest (ptid_get_tid (ptid)), 
> +	paddress (target_gdbarch, thread_local_base));

Same.  Missing i18n.

> +      return -1;
> +    }
> +
> +  printf_filtered ("Thread Information Block %s at %s\n",
> +		   pulongest (ptid_get_tid (ptid)),
> +		   paddress (target_gdbarch, thread_local_base));

Same.

> +
> +  index = (gdb_byte *) tib;
> +
> +  /* All fields have the size of a pointer, this allows to iterate 
> +     using the same for loop for both layouts.  */
> +  for (i = 0; i < max; i++)
> +    {
> +      val = extract_unsigned_integer (index, size, byte_order);
> +      if (i < max_name)
> +	printf_filtered ("%s is 0x%s\n", TIB_NAME [i], phex (val, size));

No space in array subscripting:

 TIB_NAME[i]

> +      else if (val != 0)
> +	printf_filtered ("TIB[0x%s] is 0x%s\n", phex (i*size, 2),
> +			 phex (val, size));

i18n.  Spaces missing around '*'.

> +      index += size;
> +    } 
> +  return 1;  
> +}
> +
> +/* 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)
> +    {
> +      struct thread_info *tp;
> +      int gdb_id = value_as_long (parse_and_eval (args));
> +
> +      tp = find_thread_id (gdb_id);
> +
> +      if (!tp)
> +	error (_("Thread ID %d not known."), gdb_id);
> +
> +      if (!target_thread_alive (tp->ptid))
> +	error (_("Thread ID %d has terminated."), gdb_id);
> +
> +      display_one_tib (tp->ptid);
> +    }
> +  else if (!ptid_equal (inferior_ptid, null_ptid))
> +    display_one_tib (inferior_ptid);
> +}
>  
>  void
>  windows_xfer_shared_library (const char* so_name, CORE_ADDR load_addr,
> @@ -36,3 +402,51 @@ windows_xfer_shared_library (const char*
>    obstack_grow_str (obstack, paddress (gdbarch, load_addr + 0x1000));
>    obstack_grow_str (obstack, "\"/></library>");
>  }
> +
> +static void
> +info_w32_command (char *args, int from_tty)
> +{
> +  help_list (info_w32_cmdlist, "info w32 ", class_info, gdb_stdout);
> +}
> +
> +static int w32_prefix_command_valid = 0;
> +void
> +init_w32_command_list (void)
> +{
> +  if (!w32_prefix_command_valid)
> +    {
> +      add_prefix_cmd ("w32", class_info, info_w32_command,
> +		      _("Print information specific to Win32 debugging."),
> +		      &info_w32_cmdlist, "info w32 ", 0, &infolist);
> +      w32_prefix_command_valid = 1;
> +    }
> +}
> +
> +void
> +_initialize_windows_tdep (void)
> +{
> +  init_w32_command_list ();
> +  add_cmd ("thread-information-block", class_info, display_tib,
> +	   _("Display thread information block."),
> +	   &info_w32_cmdlist);
> +  add_alias_cmd ("tib", "thread-information-block", class_info, 1,
> +		 &info_w32_cmdlist);
> +
> +  add_setshow_boolean_cmd ("show-all-tib", class_maintenance,
> +			   &maint_display_all_tib, _("\
> +Set whether to display all non-zero fields of thread information block."),
> _("\
> +Show whether to display all non-zero fields of thread information block."),
> _("\
> +Use \"on\" to enable, \"off\" to disable.\n\
> +If enabled, all non-zero fields of thread information block are
> displayed,\n\
> +even if its meaning is unknown."),
> +			   NULL,
> +			   NULL,
> +			   &maintenance_set_cmdlist,
> +			   &maintenance_show_cmdlist);
> +
> +  /* Explicitly create without lookup, since that tries to create a
> +     value with a void typed value, and when we get here, gdbarch
> +     isn't initialized yet.  At this point, we're quite sure there
> +     isn't another convenience variable of the same name.  */
> +  create_internalvar_type_lazy ("_tlb", tlb_make_value);
> +}
> Index: windows-tdep.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/windows-tdep.h,v
> retrieving revision 1.4
> diff -u -p -r1.4 windows-tdep.h
> --- windows-tdep.h	1 Jan 2010 07:31:46 -0000	1.4
> +++ windows-tdep.h	1 Apr 2010 12:46:45 -0000
> @@ -21,6 +21,10 @@
>  struct obstack;
>  struct gdbarch;
>  
> +extern struct cmd_list_element *info_w32_cmdlist;
> +
> +extern void init_w32_command_list (void);
> +
>  extern void windows_xfer_shared_library (const char* so_name,
>  					 CORE_ADDR load_addr,
>  					 struct gdbarch *gdbarch,
> Index: doc/gdb.texinfo
> ===================================================================
> RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
> retrieving revision 1.692
> diff -u -p -r1.692 gdb.texinfo
> --- doc/gdb.texinfo	31 Mar 2010 12:19:52 -0000	1.692
> +++ doc/gdb.texinfo	1 Apr 2010 12:46:53 -0000
> @@ -8054,6 +8054,14 @@ The variable @code{$_siginfo} contains e
>  (@pxref{extra signal information}).  Note that @code{$_siginfo}
>  could be empty, if the application has not yet received any signals.
>  For example, it will be empty before you execute the @code{run} command.
> +
> +@item $_tlb
> +@vindex $_tlb@r{, convenience variable}
> +The variable @code{$_tlb} is automatically set for Windows OS running
> +applications in native mode or connected to a gdbserver that supports
> +@code{qGetTIBAddr} requests. This variable contains the address of the
> thread
> +information block.

I don't think users would (or should) know what a qGetTIBAddr request
is, or how to check if their gdbserver supports it.

> +
>  @end table
>  
>  On HP-UX systems, if you refer to a function or variable name that
> @@ -15729,6 +15737,10 @@ are:
>  @tab @code{qGetTLSAddr}
>  @tab Displaying @code{__thread} variables
>  
> +@item @code{w32 thread-information-block}
> +@tab @code{qGetTIBAddr}
> +@tab Display Windows OS Thread Information Block.
> +
>  @item @code{search-memory}
>  @tab @code{qSearch:memory}
>  @tab @code{find}
> @@ -16508,6 +16520,11 @@ a long value to give the information abo
>  Without argument, this command displays information
>  about the six segment registers.
>  
> +@item info w32 thread-information-block
> +This command displays thread specific information stored in the
> +Thread Information Block (readable using @code{$fs} selector for 32-bit
> +programs and @code{$gs} for 64-bit programs).

This implies x86; ARM Windows CE programs are also 32-bit, but that
comment would apply.

> +
>  @kindex info dll
>  @item info dll
>  This is a Cygwin-specific alias of @code{info shared}.
> @@ -29173,6 +29190,14 @@ enabled, the debug registers values are 
>  removes a hardware breakpoint or watchpoint, and when the inferior
>  triggers a hardware-assisted breakpoint or watchpoint.
>  
> +@kindex maint set show-all-tib
> +@kindex maint show show-all-tib
> +@item maint set show-all-tib
> +@itemx maint show show-all-tib
> +Control whether to show all non zero areas within a 1k block starting
> +at thread local base, when using @samp{info w32 thread-information-block}
> +command.
> +
>  @kindex maint space
>  @cindex memory used by commands
>  @item maint space
> @@ -30386,6 +30411,28 @@ An error occurred.  @var{nn} are hex dig
>  An empty reply indicates that @samp{qGetTLSAddr} is not supported by the
> stub.
>  @end table
>  
> +@item qGetTIBAddr:@var{thread-id}:
> +@cindex get thread information block address
> +@cindex @samp{qGetTIBAddr} packet
> +Fetch address of the Windows OS specific Thread Information Block.
> +
> +@var{thread-id} is the thread ID associated with the thread.
> +
> +Reply:
> +@table @samp
> +@item @var{XX}@dots{}
> +Hex encoded (big endian) bytes representing the linear address of the
> +thread information block.
> +
> +@item E @var{nn}
> +An error occured. This means that either the thread was not found, or the
> +address could not be retrieved.
> +
> +@item
> +An empty reply indicates that @samp{qGetTIBAddr} is not supported by the
> stub.
> +@end table
> +
> +
>  @item qL @var{startflag} @var{threadcount} @var{nextthread}
>  Obtain thread information from RTOS.  Where: @var{startflag} (one hex
>  digit) is one to indicate the first query and zero to indicate a
> Index: gdbserver/server.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbserver/server.c,v
> retrieving revision 1.110
> diff -u -p -r1.110 server.c
> --- gdbserver/server.c	24 Mar 2010 00:14:54 -0000	1.110
> +++ gdbserver/server.c	1 Apr 2010 12:46:54 -0000
> @@ -1321,6 +1321,9 @@ handle_query (char *own_buf, int packet_
>        if (the_target->qxfer_osdata != NULL)
>  	strcat (own_buf, ";qXfer:osdata:read+");
>  
> +      if (the_target->get_tib_address != NULL)
> +	strcat (own_buf, ";qGetTIBAddr+");

Was this documented?  Is this needed at all?

> +
>        if (target_supports_multi_process ())
>  	strcat (own_buf, ";multiprocess+");
>  
> @@ -1397,6 +1400,31 @@ handle_query (char *own_buf, int packet_
>        /* Otherwise, pretend we do not understand this packet.  */
>      }
>  
> +  /* Windows OS Thread Information Block address support.  */
> +  if (the_target->get_tib_address != NULL
> +      && strncmp ("qGetTIBAddr:", own_buf, 12) == 0)
> +    {
> +      char *annex;
> +      int n;
> +      CORE_ADDR tlb;
> +      ptid_t ptid = read_ptid (own_buf + 12, &annex);
> +
> +      n = (*the_target->get_tib_address) (ptid, &tlb);
> +      if (n == 1)
> +	{
> +	  sprintf (own_buf, "%llx", tlb);
> +	  return;
> +	}
> +      else if (n == 0)
> +	{
> +	  write_enn (own_buf);
> +	  return;
> +	}
> +      return;
> +    }
> +
> +
> +
>    /* Handle "monitor" commands.  */
>    if (strncmp ("qRcmd,", own_buf, 6) == 0)
>      {
> Index: gdbserver/target.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbserver/target.h,v
> retrieving revision 1.43
> diff -u -p -r1.43 target.h
> --- gdbserver/target.h	20 Jan 2010 22:55:38 -0000	1.43
> +++ gdbserver/target.h	1 Apr 2010 12:46:54 -0000
> @@ -267,6 +267,9 @@ struct target_ops
>  			unsigned const char *writebuf,
>  			CORE_ADDR offset, int len);
>  
> +  /* Read Thread Information Block address.  */
> +  int (*get_tib_address) (ptid_t ptid, CORE_ADDR *address);
> +
>    int (*supports_non_stop) (void);
>  
>    /* Enables async target events.  Returns the previous enable
> Index: gdbserver/win32-low.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbserver/win32-low.c,v
> retrieving revision 1.43
> diff -u -p -r1.43 win32-low.c
> --- gdbserver/win32-low.c	20 Jan 2010 22:55:38 -0000	1.43
> +++ gdbserver/win32-low.c	1 Apr 2010 12:46:54 -0000
> @@ -178,7 +178,7 @@ thread_rec (ptid_t ptid, int get_context
>  
>  /* Add a thread to the thread list.  */
>  static win32_thread_info *
> -child_add_thread (DWORD pid, DWORD tid, HANDLE h)
> +child_add_thread (DWORD pid, DWORD tid, HANDLE h, void *tlb)
>  {
>    win32_thread_info *th;
>    ptid_t ptid = ptid_build (pid, tid, 0);
> @@ -189,6 +189,7 @@ child_add_thread (DWORD pid, DWORD tid, 
>    th = xcalloc (1, sizeof (*th));
>    th->tid = tid;
>    th->h = h;
> +  th->thread_local_base = (CORE_ADDR) (uintptr_t) tlb;
>  
>    add_thread (ptid, th);
>    set_inferior_regcache_data ((struct thread_info *)
> @@ -1449,7 +1450,8 @@ get_child_debug_event (struct target_wai
>        /* Record the existence of this thread.  */
>        child_add_thread (current_event.dwProcessId,
>  			current_event.dwThreadId,
> -			current_event.u.CreateThread.hThread);
> +			current_event.u.CreateThread.hThread,
> +			current_event.u.CreateThread.lpThreadLocalBase);
>        break;
>  
>      case EXIT_THREAD_DEBUG_EVENT:
> @@ -1479,7 +1481,8 @@ get_child_debug_event (struct target_wai
>        /* Add the main thread.  */
>        child_add_thread (current_event.dwProcessId,
>  			main_thread_id,
> -			current_event.u.CreateProcessInfo.hThread);
> +			current_event.u.CreateProcessInfo.hThread,
> +
> current_event.u.CreateProcessInfo.lpThreadLocalBase);
>  
>        ourstatus->value.related_pid = debug_event_ptid (&current_event);
>  #ifdef _WIN32_WCE
> @@ -1750,6 +1753,22 @@ wince_hostio_last_error (char *buf)
>  }
>  #endif
>  
> +/* 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;
> +  return 0;

Same comments as for the same windows-nat.c changes.

> +}
> +
> +
>  static struct target_ops win32_target_ops = {
>    win32_create_inferior,
>    win32_attach,
> @@ -1778,6 +1797,9 @@ static struct target_ops win32_target_op
>  #else
>    hostio_last_error_from_errno,
>  #endif
> +  NULL,
> +  NULL,
> +  win32_get_tib_address,
>  };
>  
>  /* Initialize the Win32 backend.  */
> Index: gdbserver/win32-low.h
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbserver/win32-low.h,v
> retrieving revision 1.12
> diff -u -p -r1.12 win32-low.h
> --- gdbserver/win32-low.h	20 Jan 2010 22:55:38 -0000	1.12
> +++ gdbserver/win32-low.h	1 Apr 2010 12:46:54 -0000
> @@ -28,6 +28,9 @@ typedef struct win32_thread_info
>    /* The handle to the thread.  */
>    HANDLE h;
>  
> +  /* Thread Information Block address.  */
> +  CORE_ADDR thread_local_base;
> +
>    /* Non zero if SuspendThread was called on this thread.  */
>    int suspended;
>  

Otherwise, it's looking good.

-- 
Pedro Alves