Re: [PATCH] Port GDB to Hurd x86_64.

[John Baldwin <jhb@FreeBSD.org>]

On 2/12/24 8:31 PM, Flavio Cruz wrote:
> This port extends the existing i686 port to support x86_64 by trying to
> reuse existing code whenever it makes sense.
> 
> * gdb/amd64-gnu-tdep.c: Adds logic for handling signal frames and
>    position of amd64 registers in the different Hurd structs, including
>    i386_thread_state. The signal code is very similar to i686, except the
>    trampoline code is adapted.
> * gdb/amd64-gnu-tdep.h: export register offsets for x86-gnu-nat.c.
> * gdb/config/i386/nm-i386gnu.h: renamed to gdb/config/i386/nm-x86-gnu.h
>    and adapt it for x86_64.
> * gdb/config/i386/i386gnu.mn: renamed to gdb/config/i386/nm-x86-gnu.mn
>    and reuse it for x86_64.
> * gdb/configure.host: recognize gnu64 as a host.
> * gdb/configure.nat: recognize gnu64 host and update existing i386gnu to
>    reuse the new shared files.
> * gdb/configure.tgt: recognize x86_64-*-gnu* triplet and use
>    amd64-gnu-tdep.c.
> * gdb/i386-gnu-tdep.c: added i386_gnu_thread_state_reg_offset that is
>    copied from i386-gnu-nat.c. This makes it similar to amd64.
> * gdb/i386-gnu-tdep.h: export register offsets and number of registers.
> * gdb/i386-gnu-nat.c: rename it to x86-gnu-nat.c since we reuse this for
>    i386 and amd64. Updated REG_ADDR to use one of the structures. Added
>    VALID_REGISTER to make sure it's a register we can provide at this time
>    (not all of them are available in amd64). FLAGS_REGISTER is either rfl
>    or efl depending on the arch. Renamed functions and class from i386 to x86
>    whenever they can be reused.
> 
> Tested on Hurd x86_64 and i686.
> ---
> 
> For Hurd x86_64 to work, "[PATCH] Hurd port: update interface to match
> upstream and fix warnings" needs to be applied too.
> 
>   gdb/amd64-gnu-tdep.c                          | 256 ++++++++++++++++++
>   gdb/amd64-gnu-tdep.h                          |  29 ++
>   .../i386/{nm-i386gnu.h => nm-x86-gnu.h}       |   7 +
>   gdb/config/i386/{i386gnu.mn => x86-gnu.mn}    |   0
>   gdb/configure.host                            |   1 +
>   gdb/configure.nat                             |  27 +-
>   gdb/configure.tgt                             |   4 +
>   gdb/i386-gnu-tdep.c                           |  37 ++-
>   gdb/i386-gnu-tdep.h                           |  29 ++
>   gdb/{i386-gnu-nat.c => x86-gnu-nat.c}         | 128 +++++----
>   10 files changed, 457 insertions(+), 61 deletions(-)
>   create mode 100644 gdb/amd64-gnu-tdep.c
>   create mode 100644 gdb/amd64-gnu-tdep.h
>   rename gdb/config/i386/{nm-i386gnu.h => nm-x86-gnu.h} (83%)
>   rename gdb/config/i386/{i386gnu.mn => x86-gnu.mn} (100%)
>   create mode 100644 gdb/i386-gnu-tdep.h
>   rename gdb/{i386-gnu-nat.c => x86-gnu-nat.c} (75%)
> 
> diff --git a/gdb/amd64-gnu-tdep.c b/gdb/amd64-gnu-tdep.c
> new file mode 100644
> index 00000000000..57aeccea8b9
> --- /dev/null
> +++ b/gdb/amd64-gnu-tdep.c
> @@ -0,0 +1,256 @@
> +/* Mapping between the general-purpose registers in `struct
> +   sigcontext' format (starting at sc_i386_thread_state)
> +   and GDB's register cache layout.  */
> +
> +/* From <bits/sigcontext.h>.  */
> +static int amd64_gnu_sc_reg_offset[] =
> +{
> +  15 * 8,			/* %rax */
> +  12 * 8,			/* %rbx */
> +  14 * 8,			/* %rcx */
> +  13 * 8,			/* %rdx */
> +  10 * 8,			/* %rsi */
> +  9 * 8,			/* %rdi */
> +  10 * 8,			/* %rbp */
> +  11 * 8,			/* %rsp */
> +  0 * 8,			/* %r8 ...  */
> +  8 * 8,
> +  7 * 8,
> +  6 * 8,
> +  3 * 8,
> +  2 * 8,
> +  1 * 8,
> +  0 * 8,			/* ... %r15 */
> +  16 * 8,			/* %rip */
> +  18 * 8,			/* %eflags */
> +  17 * 8,			/* %cs */
> +};
> +
> +/* From <sys/ucontext.h>.  */
> +static int amd64_gnu_gregset_reg_offset[] =
> +{
> +  10 * 8,			/* %rax */
> +  5 * 8,			/* %rbx */
> +  11 * 8,			/* %rcx */
> +  12 * 8,			/* %rdx */
> +  13 * 8,			/* %rsi */
> +  14 * 8,			/* %rdi */
> +  4 * 8,			/* %rbp */
> +  19 * 8,			/* %rsp */
> +  9 * 8,			/* %r8 ...  */
> +  8 * 8,
> +  7 * 8,
> +  6 * 8,
> +  3 * 8,
> +  2 * 8,
> +  1 * 8,
> +  0 * 8,			/* ... %r15 */
> +  16 * 8,			/* %rip */
> +  18 * 8,			/* %eflags */
> +  17 * 8,			/* %cs */
> +  -1,				  /* %ss */
> +  -1,				  /* %ds */
> +  -1,				  /* %es */
> +  -1,				  /* %fs */
> +  -1,				  /* %gs */
> +};
> +
> +/* Offset to the thread_state_t location where REG is stored.  */
> +#define REG_OFFSET(reg) offsetof (struct i386_thread_state, reg)

You can't use a reference to this OS-specific type in a tdep.c file,
only in a nat.c file.  tdep.c should be buildable on other platforms
to permit cross debugging of core dumps, remote targets, etc.

> +/* At REG_OFFSET[N] is the offset to the thread_state_t location where
> +   the GDB register N is stored.  */
> +int amd64_gnu_thread_state_reg_offset[] =
> +{
> +  REG_OFFSET (rax),		/* %rax */
> +  REG_OFFSET (rbx),		/* %rbx */
> +  REG_OFFSET (rcx),		/* %rcx */
> +  REG_OFFSET (rdx),		/* %rdx */
> +  REG_OFFSET (rsi),		/* %rsi */
> +  REG_OFFSET (rdi),		/* %rdi */
> +  REG_OFFSET (rbp),		/* %rbp */
> +  REG_OFFSET (ursp),	/* %rsp */
> +  REG_OFFSET (r8),		/* %r8 ...  */
> +  REG_OFFSET (r9),
> +  REG_OFFSET (r10),
> +  REG_OFFSET (r11),
> +  REG_OFFSET (r12),
> +  REG_OFFSET (r13),
> +  REG_OFFSET (r14),
> +  REG_OFFSET (r15),		/* ... %r15 */
> +  REG_OFFSET (rip),		/* %rip */
> +  REG_OFFSET (rfl),		/* %rflags */
> +  REG_OFFSET (cs)	    /* %cs */
> +};
> +
> +const int amd64_gnu_thread_state_num_regs =
> +  ARRAY_SIZE (amd64_gnu_thread_state_reg_offset);

That said, I also don't see any references to amd64_gnu_thread_state_*
in this file, and it looks to only be used in x86-gnu-nat.c, so I think
you should instead move this array to x86-gnu-nat.c instead (and
similarly for i386_gnu_thread_state_* you added in i386-gnu-tdep.c).

-- 
John Baldwin