Re: [RFC-v2] Add windows Thread Information Block

Christopher Faylor
2009-07-01T18:10:31Z

On Wed, Jul 01, 2009 at 04:41:44PM +0200, Pierre Muller wrote:
>2009-07-01 Pierre Muller <muller@ics.u-strasbg.fr>
>
> * 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_xfer_partial): Handle TARGET_OBJECT_OSDATA type.
> (_initialize_windows_nat): Replace info_w32_cmdlist
> initialization by a call to init_w32_command_list.
> * windows-nat.c (info_w32_command): Moved from here...
> * windows-tdep.c (info_w32_command): to here.
> * windows-nat.c (info_w32_cmdlist): Deleted, move to...
> * windows-tdep.c (info_w32_cmdlist): to here, made global.
> * windows-tdep.h (info_w32_cmdlist): Declare.
> * windows-tdep.c (thread_information_32): New struct.
> (thread_information_64): New struct.
> (TIB_NAME): New char array.
> (MAX_TIB32, MAX_TIB64): New constants.
> (display_one_tib): New function.
> (display_tib): New function.
> (init_w32_command_list): New function.
> (_initialize_windows_tdep): New function.
> * windows-tdep.h (init_w32_command_list): New external function
>declaration.

Does the indentation and duplication in the above ChangeLog really adhere
to GNU standards?

>Index: windows-nat.c
>===================================================================
>RCS file: /cvs/src/src/gdb/windows-nat.c,v
>retrieving revision 1.195
>diff -u -p -r1.195 windows-nat.c
>--- windows-nat.c 17 Jun 2009 18:44:23 -0000 1.195
>+++ windows-nat.c 1 Jul 2009 14:10:39 -0000
>@@ -57,6 +57,7 @@
> #include "solist.h"
> #include "solib.h"
> #include "xml-support.h"
>+#include "target.h"
>
> #include "i386-tdep.h"
> #include "i387-tdep.h"
>@@ -155,6 +156,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;
>@@ -284,7 +286,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;
>@@ -299,6 +301,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);
>@@ -984,15 +987,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 (\
>@@ -1181,9 +1175,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);

This is apparently just some whitespace artifact but I can't actually
apply the patch because of it so I can't tell for sure if the
indentation is whacked here too. I guess I'll just trust that you
haven't added extra gratuitous blank lines and have adhered to the
indentation rules.

>Index: windows-tdep.c
>===================================================================
>RCS file: /cvs/src/src/gdb/windows-tdep.c,v
>retrieving revision 1.3
>diff -u -p -r1.3 windows-tdep.c
>--- windows-tdep.c 13 Jan 2009 04:14:07 -0000 1.3
>+++ windows-tdep.c 1 Jul 2009 14:10:39 -0000
>@@ -19,6 +19,160 @@
> #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"
>+
>+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 thread_local_storage; /* %fs:0x0028 */
>+ uint32_t active_rpc_handle; /* %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 */
>+ }
>+thread_information_64;

Do we really have to reinvent this structure here? Can't we use
a Windows header definition rather than inventing our own?

cgf