Re: [RFA] [2/4] Mingw64 gdbserver support

Pedro Alves
2010-04-19T23:22:41Z

On Monday 19 April 2010 23:57:42, Pierre Muller wrote:
> 2010-04-19 Pierre Muller <muller@ics.u-strasbg.fr>
>
> * win32-i386-low.c: Add 64-bit support.
> (COMPILE_WIN64): New macro set if
> compilation target to debug Windows 64-bit executables.
> (CONTEXT_EXTENDED_REGISTERS): Set macro to zero if not exisiting.
> (init_registers_amd64): Declare.
> (mappings): Add 64-bit version of array.
> (init_windows_x86): New function.
> (the_low_target): Change init_arch field to init_windows_x86.
>
> Index: gdbserver/win32-i386-low.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbserver/win32-i386-low.c,v
> retrieving revision 1.18
> diff -u -p -r1.18 win32-i386-low.c
> --- gdbserver/win32-i386-low.c 20 Jan 2010 22:55:38 -0000 1.18
> +++ gdbserver/win32-i386-low.c 19 Apr 2010 22:35:38 -0000
> @@ -19,13 +19,25 @@
> #include "win32-low.h"
> #include "i386-low.h"
>
> +#ifdef __x86_64
> +#define COMPILE_WIN64

Get rid of `COMPILE_WIN64' entirely, and just use __x86_64
throughout. Does __x86_64__ not work instead? It is more
common to use the latter.

> +#ifndef CONTEXT_EXTENDED_REGISTERS
> +#define CONTEXT_EXTENDED_REGISTERS 0
> +#endif
> +#endif
> +

Why is this all wrapped in __x86_64 ? Can't you
just do:

+#ifndef CONTEXT_EXTENDED_REGISTERS
+# define CONTEXT_EXTENDED_REGISTERS 0
+#endif

and that's it?

> #define FCS_REGNUM 27
> #define FOP_REGNUM 31
>
> #define FLAG_TRACE_BIT 0x100
>
> +#ifdef COMPILE_WIN64
> +/* Defined in auto-generated file reg-i386.c. */

Presumably, something like reg-x86-64.c or reg-amd64.c, I think?

> +void init_registers_amd64 (void);
> +#else
> /* Defined in auto-generated file reg-i386.c. */
> void init_registers_i386 (void);
> +#endif
>
> static struct i386_debug_reg_state debug_reg_state;
>
> @@ -214,6 +226,8 @@ i386_single_step (win32_thread_info *th)
> th->context.EFlags |= FLAG_TRACE_BIT;
> }
>
> +#ifndef COMPILE_WIN64
> +
> /* An array of offset mappings into a Win32 Context structure.
> This is a one-to-one mapping which is indexed by gdb's register
> numbers. It retrieves an offset into the context structure where
> @@ -269,6 +283,75 @@ static const int mappings[] = {
> };
> #undef context_offset
>
> +#else /* COMPILE_WIN64 */
> +
> +#define context_offset(x) (offsetof (CONTEXT, x))
> +static const int mappings[] =
> +{
> + context_offset (Rax),
> + context_offset (Rbx),
> + context_offset (Rcx),
> + context_offset (Rdx),
> + context_offset (Rsi),
> + context_offset (Rdi),
> + context_offset (Rbp),
> + context_offset (Rsp),
> + context_offset (R8),
> + context_offset (R9),
> + context_offset (R10),
> + context_offset (R11),
> + context_offset (R12),
> + context_offset (R13),
> + context_offset (R14),
> + context_offset (R15),
> + context_offset (Rip),
> + context_offset (EFlags),
> + context_offset (SegCs),
> + context_offset (SegSs),
> + context_offset (SegDs),
> + context_offset (SegEs),
> + context_offset (SegFs),
> + context_offset (SegGs),
> + context_offset (FloatSave.FloatRegisters[0]),
> + context_offset (FloatSave.FloatRegisters[1]),
> + context_offset (FloatSave.FloatRegisters[2]),
> + context_offset (FloatSave.FloatRegisters[3]),
> + context_offset (FloatSave.FloatRegisters[4]),
> + context_offset (FloatSave.FloatRegisters[5]),
> + context_offset (FloatSave.FloatRegisters[6]),
> + context_offset (FloatSave.FloatRegisters[7]),
> + context_offset (FloatSave.ControlWord),
> + context_offset (FloatSave.StatusWord),
> + context_offset (FloatSave.TagWord),
> + context_offset (FloatSave.ErrorSelector),
> + context_offset (FloatSave.ErrorOffset),
> + context_offset (FloatSave.DataSelector),
> + context_offset (FloatSave.DataOffset),
> + context_offset (FloatSave.ErrorSelector)
> + /* XMM0-7 */ ,
> + context_offset (Xmm0),
> + context_offset (Xmm1),
> + context_offset (Xmm2),
> + context_offset (Xmm3),
> + context_offset (Xmm4),
> + context_offset (Xmm5),
> + context_offset (Xmm6),
> + context_offset (Xmm7),
> + context_offset (Xmm8),
> + context_offset (Xmm9),
> + context_offset (Xmm10),
> + context_offset (Xmm11),
> + context_offset (Xmm12),
> + context_offset (Xmm13),
> + context_offset (Xmm14),
> + context_offset (Xmm15),
> + /* MXCSR */
> + context_offset (FloatSave.MxCsr)
> +};
> +#undef context_offset
> +
> +#endif /* COMPILE_WIN64 */
> +
> /* Fetch register from gdbserver regcache data. */
> static void
> i386_fetch_inferior_register (struct regcache *regcache,
> @@ -303,8 +386,18 @@ i386_store_inferior_register (struct reg
> static const unsigned char i386_win32_breakpoint = 0xcc;
> #define i386_win32_breakpoint_len 1
>
> +static void
> +init_windows_x86 ()

^ write (void) instead.

In C, an unprototyped function is not the same as a function
declared as taking `void' as argument.

> +{
> +#ifdef COMPILE_WIN64
> + init_registers_amd64 ();
> +#else
> + init_registers_i386 ();
> +#endif
> +}
> +
> struct win32_target_ops the_low_target = {
> - init_registers_i386,
> + init_windows_x86,
> sizeof (mappings) / sizeof (mappings[0]),
> i386_initial_stuff,
> i386_get_thread_context,
>

This is okay with those changes. Thanks.

--
Pedro Alves