Re: [rfc] bi-arch for x86-64 corefile.

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Sun, Oct 21, 2012 at 9:50 PM, Yao Qi <yao@codesourcery.com> wrote:
> Hi,
> This patch fixes the issue of generating x86-64 corefile on
> i686-pc-linux-gnu gdb.  GDB/BFD starts to use backend function
> 'write_core_note' to generate corefile.  Backend function
> 'write_core_note' is defined on x86-64, but only used when CORE_HEADER
> is defined.
>
> When we configure native gdb 'i686-pc-linux-gnu' with
> '--enable-64-bit-bfd', elf64-x86-64.c is compiled but CORE_HEADER is
> *not* defined for it, so that BFD will fall back to common code,
> instead of backend function, to generate corefile.  Part of corefile
> is generated as 'i686-pc-linux-gnu', which is wrong.
>
> This patch defines CORE_HEADER even in 'i[3-7]86-*-linux-*' if
> '--enable-64-bit-bfd' is true, so that the right backend function is
> hooked to 'write_core_note'.  After this change, it exposes some
> problems in elf64-x86-64.c that 'struct elf_prstatus' and 'struct
> elf_prpsinfo' are different on different arch (i386 vs. x86_64), so
> this patch continues to define something new.
>
> Is it OK?
>
> bfd/
>         * configure.in: Set CORE_HEADER for 'i[3-7]86-*-linux-*'
>         if 'want64' is true.
>         * configure: Regenerated.
>
>         * elf64-x86-64.c (elf_x86_64_write_core_note): Replace type
>         'prpsinfo_t' and 'prstatus_t' with 'prpsinfo64_t' and
>         'prstatus64_t' respectively.
>         * hosts/x86-64linux.h (struct prstatus64_timeval): New.
>         (struct elf_prstatus64): New.
>         (struct elf_prpsinfo64): New.
>         (prstatus64_t, prpsinfo64_t): New typedef.
> ---
>  bfd/configure           |    3 ++
>  bfd/configure.in        |    3 ++
>  bfd/elf64-x86-64.c      |    4 +-
>  bfd/hosts/x86-64linux.h |   49 +++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 57 insertions(+), 2 deletions(-)
>
> diff --git a/bfd/configure b/bfd/configure
> index fdf5356..e462f0d 100755
> --- a/bfd/configure
> +++ b/bfd/configure
> @@ -13927,6 +13927,9 @@ if test "${target}" = "${host}"; then
>    i[3-7]86-*-linux-*)
>         COREFILE=trad-core.lo
>         TRAD_HEADER='"hosts/i386linux.h"'
> +       if test x${want64} = xtrue ; then
> +       CORE_HEADER='"hosts/x86-64linux.h"'
> +       fi
>         ;;
>    i[3-7]86-*-isc*)     COREFILE=trad-core.lo ;;
>    i[3-7]86-*-aix*)     COREFILE=aix386-core.lo ;;
> diff --git a/bfd/configure.in b/bfd/configure.in
> index 530bbcb..5ed3ac2 100644
> --- a/bfd/configure.in
> +++ b/bfd/configure.in
> @@ -331,6 +331,9 @@ changequote(,)dnl
>  changequote([,])dnl
>         COREFILE=trad-core.lo
>         TRAD_HEADER='"hosts/i386linux.h"'
> +       if test x${want64} = xtrue ; then
> +       CORE_HEADER='"hosts/x86-64linux.h"'
> +       fi
>         ;;

Please use "else" for CORE_HEADER=.

>  changequote(,)dnl
>    i[3-7]86-*-isc*)     COREFILE=trad-core.lo ;;
> diff --git a/bfd/elf64-x86-64.c b/bfd/elf64-x86-64.c
> index a440647..ab11658 100644
> --- a/bfd/elf64-x86-64.c
> +++ b/bfd/elf64-x86-64.c
> @@ -449,7 +449,7 @@ elf_x86_64_write_core_note (bfd *abfd, char *buf, int *bufsiz,
>         }
>        else
>         {
> -         prpsinfo_t data;
> +         prpsinfo64_t data;
>           memset (&data, 0, sizeof (data));
>           strncpy (data.pr_fname, fname, sizeof (data.pr_fname));
>           strncpy (data.pr_psargs, psargs, sizeof (data.pr_psargs));
> @@ -490,7 +490,7 @@ elf_x86_64_write_core_note (bfd *abfd, char *buf, int *bufsiz,
>         }
>        else
>         {
> -         prstatus_t prstat;
> +         prstatus64_t prstat;
>           memset (&prstat, 0, sizeof (prstat));
>           prstat.pr_pid = pid;
>           prstat.pr_cursig = cursig;
> diff --git a/bfd/hosts/x86-64linux.h b/bfd/hosts/x86-64linux.h
> index 4ffc3f2..94ed81d 100644
> --- a/bfd/hosts/x86-64linux.h
> +++ b/bfd/hosts/x86-64linux.h
> @@ -128,6 +128,12 @@ struct prstatus32_timeval
>      int tv_usec;
>    };
>
> +struct prstatus64_timeval
> +  {
> +    uint64_t tv_sec;
> +    uint64_t tv_usec;
> +  };
> +
>  struct elf_prstatus32
>    {
>      struct elf_siginfo pr_info;                /* Info associated with signal.  */
> @@ -164,6 +170,29 @@ struct elf_prstatusx32
>      int pr_fpvalid;                    /* True if math copro being used.  */
>    };
>
> +struct elf_prstatus64
> +  {
> +    struct elf_siginfo pr_info;        /* Info associated with signal.  */
> +    short int pr_cursig;               /* Current signal.  */
> +    uint64_t pr_sigpend;               /* Set of pending signals.  */
> +    uint64_t pr_sighold;               /* Set of held signals.  */
> +    pid_t pr_pid;
> +    pid_t pr_ppid;
> +    pid_t pr_pgrp;
> +    pid_t pr_sid;
> +
> +    struct prstatus64_timeval pr_utime;                /* User time.  */
> +    struct prstatus64_timeval pr_stime;                /* System time.  */
> +    struct prstatus64_timeval pr_cutime;       /* Cumulative user time.  */
> +    struct prstatus64_timeval pr_cstime;       /* Cumulative system time.  */
> +
> +    elf_gregsetx32_t pr_reg;           /* GP registers.  */

Please rename elf_gregsetx32_t to elf_gregset64_t.

> +    int pr_fpvalid;                    /* True if math copro being used.  */
> +#if __WORDSIZE == 32
> +    char pad[4];
> +#endif
> +  };

This is wrong. There should be no pad.  Please properly copy the
64-bit elf_prstatus.

>  struct elf_prpsinfo32
>    {
>      char pr_state;                     /* Numeric process state.  */
> @@ -179,6 +208,24 @@ struct elf_prpsinfo32
>      char pr_psargs[ELF_PRARGSZ];       /* Initial part of arg list.  */
>    };
>
> +struct elf_prpsinfo64
> +  {
> +    char pr_state;                     /* Numeric process state.  */
> +    char pr_sname;                     /* Char for pr_state.  */
> +    char pr_zomb;                      /* Zombie.  */
> +    char pr_nice;                      /* Nice val.  */
> +#if __WORDSIZE == 32
> +    char pad[4];
> +#endif
> +    uint64_t pr_flag;          /* Flags.  */
> +    unsigned int pr_uid;
> +    unsigned int pr_gid;
> +    int pr_pid, pr_ppid, pr_pgrp, pr_sid;
> +    /* Lots missing */
> +    char pr_fname[16];                 /* Filename of executable.  */
> +    char pr_psargs[ELF_PRARGSZ];       /* Initial part of arg list.  */
> +  };
> +

This is wrong. There should be no pad.  Please properly copy the
64-bit elf_prpsinfo64.

>  /* The rest of this file provides the types for emulation of the
>     Solaris <proc_service.h> interfaces that should be implemented by
> @@ -187,4 +234,6 @@ struct elf_prpsinfo32
>  /* Process status and info.  In the end we do provide typedefs for them.  */
>  typedef struct elf_prstatus32 prstatus32_t;
>  typedef struct elf_prstatusx32 prstatusx32_t;
> +typedef struct elf_prstatus64 prstatus64_t;
>  typedef struct elf_prpsinfo32 prpsinfo32_t;
> +typedef struct elf_prpsinfo64 prpsinfo64_t;
> --
> 1.7.7.6
>



-- 
H.J.