Re: [PATCH V3] amd64-linux: expose system register FS_BASE and GS_BASE for Linux.

[Pedro Alves <palves at redhat dot com>]

On 01/25/2017 10:13 AM, Walfred Tedeschi wrote:

> diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c
> index 4a2036b..b9930b1 100644
> --- a/gdb/amd64-linux-nat.c
> +++ b/gdb/amd64-linux-nat.c
> @@ -40,6 +40,12 @@
>  #include "nat/linux-ptrace.h"
>  #include "nat/amd64-linux-siginfo.h"
>  
> +      /* This definition comes from the prctl.h kernels older than 2.5.64
> +         do not have it.  */

Comment is indented when it should not.

Looks like two sentences mashed in one?  Did you mean:

/* This definition comes from prctl.h.  Kernels older than 2.5.64
   do not have it.  */

> +#ifndef PTRACE_ARCH_PRCTL
> +#define PTRACE_ARCH_PRCTL      30
> +#endif

> diff --git a/gdb/amd64-nat.c b/gdb/amd64-nat.c
> index 680105b..0efdad5 100644
> --- a/gdb/amd64-nat.c
> +++ b/gdb/amd64-nat.c
> @@ -68,6 +68,13 @@ amd64_native_gregset_reg_offset (struct gdbarch *gdbarch, int regnum)
>    if (regnum >= num_regs)
>      return -1;
>  
> +  /* Definition of the offset foresees new kernel support, it has to be
> +     corrected here in case the kernel is older.  */

This missed my comment and suggestion in v1:

> This comment reads to me like we'll need to change the code as soon
> as the kernel is updated/changed for something.  Which is course
> not what you want to say.
>

Change it to:

>  /* Kernels that predate Linux 2.6.25 don't provide access to
>     these segment registers in user_regs_struct.   */

> diff --git a/gdb/testsuite/gdb.arch/amd64-gs_base.c b/gdb/testsuite/gdb.arch/amd64-gs_base.c
> new file mode 100644
> index 0000000..fe81226
> --- /dev/null
> +++ b/gdb/testsuite/gdb.arch/amd64-gs_base.c


> +int
> +main (void)
> +{
> +  return 1;

It's odd to return non-zero (error).  Return 0 unless there's
a reason otherwise.

OK with those changes.

Thanks,
Pedro Alves