Re: [PATCH 3/4] ELF/BFD: Handle both variants of the 64-bit Linux core PRPSINFO note

[Sergio Durigan Junior <sergiodj at redhat dot com>]

On Friday, September 22 2017, Maciej W. Rozycki wrote:

> Fix commit 70a38d42c5b3 ("New entry points for writing Linux NT_PRPSINFO 
> notes."), <https://sourceware.org/ml/binutils/2013-02/msg00023.html>, 
> and handle both variants of the 64-bit Linux core PRPSINFO note across 
> all targets.

Thanks for the patch, Maciej.  Just a comment below.

>
> The 64-bit Linux core PRPSINFO note matches the 64-bit kernel structure, 
> defined as follows:
>
> (gdb) ptype struct elf_prpsinfo
> type = struct elf_prpsinfo {
>     char pr_state;
>     char pr_sname;
>     char pr_zomb;
>     char pr_nice;
>     unsigned long pr_flag;
>     __kernel_uid_t pr_uid;
>     __kernel_gid_t pr_gid;
>     pid_t pr_pid;
>     pid_t pr_ppid;
>     pid_t pr_pgrp;
>     pid_t pr_sid;
>     char pr_fname[16];
>     char pr_psargs[80];
> }
> (gdb)
>
> where the individual data types of most members are the same across all 
> 64-bit Linux ports, with the exception of `__kernel_uid_t' and 
> `__kernel_gid_t'.  These are defined in <asm-generic/posix_types.h> as 
> 32-bit `unsigned int' by default, however overridden as 16-bit `unsigned 
> short' in port-specific <asm/posix_types.h> for the SuperH target.
>
> The default is already handled, as from the commit referred.  Add the 
> other variant then and provide a backend flag to switch between the two 
> cases possible, with the 32-bit one being the default and the 16-bit one 
> explicitly selected.  Set the flag in the SuperH target backend.  This 
> arrangement is analogous to how the 32-bit Linux core PRPSINFO note is
> handled.
>
> 	bfd/
> 	* elf-bfd.h (elf_backend_data): Add `linux_prpsinfo64_ugid16' 
> 	member.
> 	* elf-linux-core.h (elf_external_linux_prpsinfo64): Rename to...
> 	(elf_external_linux_prpsinfo64_ugid32): ... this.
> 	(swap_linux_prpsinfo32_out): Rename to...
> 	(swap_linux_prpsinfo32_ugid32_out): ... this.
> 	(elf_external_linux_prpsinfo64_ugid16): New structure.
> 	(swap_linux_prpsinfo64_ugid16_out): New function.
> 	* elfxx-target.h [!elf_backend_linux_prpsinfo64_ugid16]
> 	(elf_backend_linux_prpsinfo64_ugid16): Define.
> 	(elfNN_bed): Initialize `linux_prpsinfo64_ugid16' member.
> 	* elf.c (elfcore_write_linux_prpsinfo64): Handle both variants 
> 	of the 64-bit Linux core PRPSINFO note.
> 	* elf64-sh64.c (elf_backend_linux_prpsinfo64_ugid16): Define.
> ---
>  bfd/elf-bfd.h        |    4 +++
>  bfd/elf-linux-core.h |   68 +++++++++++++++++++++++++++++++++++++++++++++++----
>  bfd/elf.c            |   21 ++++++++++++---
>  bfd/elf64-sh64.c     |    2 +
>  bfd/elfxx-target.h   |    6 +++-
>  5 files changed, 90 insertions(+), 11 deletions(-)
>
> gdb-elf-linux-core-prpsinfo64-ugid.diff
> Index: binutils/bfd/elf-bfd.h
> ===================================================================
> --- binutils.orig/bfd/elf-bfd.h	2017-09-20 13:03:18.000000000 +0100
> +++ binutils/bfd/elf-bfd.h	2017-09-20 13:06:49.534308157 +0100
> @@ -1556,6 +1556,10 @@ struct elf_backend_data
>    /* True if the 32-bit Linux PRPSINFO structure's `pr_uid' and `pr_gid'
>       members use a 16-bit data type.  */
>    unsigned linux_prpsinfo32_ugid16 : 1;
> +
> +  /* True if the 64-bit Linux PRPSINFO structure's `pr_uid' and `pr_gid'
> +     members use a 16-bit data type.  */
> +  unsigned linux_prpsinfo64_ugid16 : 1;
>  };
>  
>  /* Information about reloc sections associated with a bfd_elf_section_data
> Index: binutils/bfd/elf-linux-core.h
> ===================================================================
> --- binutils.orig/bfd/elf-linux-core.h	2017-09-20 13:03:18.000000000 +0100
> +++ binutils/bfd/elf-linux-core.h	2017-09-20 13:15:40.694234169 +0100
> @@ -129,10 +129,14 @@ swap_linux_prpsinfo32_ugid16_out
>     ABI-defined, thus we choose to use char arrays here in order to
>     avoid dealing with different types in different architectures.
>  
> +   This is the variant for targets which use a 32-bit data type for
> +   UID and GID, as most Linux ports do.  The SH64 port uses a 16-bit
> +   data type instead; see below for the alternative variant.
> +
>     This structure will ultimately be written in the corefile's note
>     section, as the PRPSINFO.  */
>  
> -struct elf_external_linux_prpsinfo64
> +struct elf_external_linux_prpsinfo64_ugid32
>    {
>      char pr_state;			/* Numeric process state.  */
>      char pr_sname;			/* Char for pr_state.  */
> @@ -151,12 +155,13 @@ struct elf_external_linux_prpsinfo64
>    };
>  
>  /* Helper function to copy an elf_internal_linux_prpsinfo in host
> -   endian to an elf_external_linux_prpsinfo64 in target endian.  */
> +   endian to an elf_external_linux_prpsinfo64_ugid32 in target endian.  */
>  
>  static inline void
> -swap_linux_prpsinfo64_out (bfd *obfd,
> -			   const struct elf_internal_linux_prpsinfo *from,
> -			   struct elf_external_linux_prpsinfo64 *to)
> +swap_linux_prpsinfo64_ugid32_out
> +  (bfd *obfd,
> +   const struct elf_internal_linux_prpsinfo *from,
> +   struct elf_external_linux_prpsinfo64_ugid32 *to)
>  {
>    bfd_put_8 (obfd, from->pr_state, &to->pr_state);
>    bfd_put_8 (obfd, from->pr_sname, &to->pr_sname);
> @@ -173,4 +178,57 @@ swap_linux_prpsinfo64_out (bfd *obfd,
>    strncpy (to->pr_psargs, from->pr_psargs, sizeof (to->pr_psargs));
>  }
>  
> +/* External 64-bit structure for PRPSINFO.  This structure is
> +   ABI-defined, thus we choose to use char arrays here in order to
> +   avoid dealing with different types in different architectures.
> +
> +   This is the variant for the SH64 port which uses a 16-bit data
> +   type for UID and GID.  Most Linux ports use a 32-bit data type
> +   instead; see above for the alternative variant.

I noticed that you put the most common definition at the top in the
previous patch (32-bit), but now you're putting the most common at the
bottom.  I'd prefer if you could put the most common at the top and the
specific one at the bottom for both 32- and 64-bit cases.

> +
> +   This structure will ultimately be written in the corefile's note
> +   section, as the PRPSINFO.  */
> +
> +struct elf_external_linux_prpsinfo64_ugid16
> +  {
> +    char pr_state;			/* Numeric process state.  */
> +    char pr_sname;			/* Char for pr_state.  */
> +    char pr_zomb;			/* Zombie.  */
> +    char pr_nice;			/* Nice val.  */
> +    char gap[4];
> +    char pr_flag[8];			/* Flags.  */
> +    char pr_uid[2];
> +    char pr_gid[2];
> +    char pr_pid[4];
> +    char pr_ppid[4];
> +    char pr_pgrp[4];
> +    char pr_sid[4];
> +    char pr_fname[16];			/* Filename of executable.  */
> +    char pr_psargs[80];			/* Initial part of arg list.  */
> +  };
> +
> +/* Helper function to copy an elf_internal_linux_prpsinfo in host
> +   endian to an elf_external_linux_prpsinfo64_ugid16 in target endian.  */
> +
> +static inline void
> +swap_linux_prpsinfo64_ugid16_out
> +  (bfd *obfd,
> +   const struct elf_internal_linux_prpsinfo *from,
> +   struct elf_external_linux_prpsinfo64_ugid16 *to)
> +{
> +  bfd_put_8 (obfd, from->pr_state, &to->pr_state);
> +  bfd_put_8 (obfd, from->pr_sname, &to->pr_sname);
> +  bfd_put_8 (obfd, from->pr_zomb, &to->pr_zomb);
> +  bfd_put_8 (obfd, from->pr_nice, &to->pr_nice);
> +  bfd_put_64 (obfd, from->pr_flag, to->pr_flag);
> +  bfd_put_16 (obfd, from->pr_uid, to->pr_uid);
> +  bfd_put_16 (obfd, from->pr_gid, to->pr_gid);
> +  bfd_put_32 (obfd, from->pr_pid, to->pr_pid);
> +  bfd_put_32 (obfd, from->pr_ppid, to->pr_ppid);
> +  bfd_put_32 (obfd, from->pr_pgrp, to->pr_pgrp);
> +  bfd_put_32 (obfd, from->pr_sid, to->pr_sid);
> +  strncpy (to->pr_fname, from->pr_fname, sizeof (to->pr_fname));
> +  strncpy (to->pr_psargs, from->pr_psargs, sizeof (to->pr_psargs));
> +}
> +
>  #endif
> Index: binutils/bfd/elf.c
> ===================================================================
> --- binutils.orig/bfd/elf.c	2017-09-20 13:03:18.000000000 +0100
> +++ binutils/bfd/elf.c	2017-09-20 13:18:25.396554524 +0100
> @@ -10470,11 +10470,22 @@ elfcore_write_linux_prpsinfo64
>    (bfd *abfd, char *buf, int *bufsiz,
>     const struct elf_internal_linux_prpsinfo *prpsinfo)
>  {
> -  struct elf_external_linux_prpsinfo64 data;
> -
> -  swap_linux_prpsinfo64_out (abfd, prpsinfo, &data);
> -  return elfcore_write_note (abfd, buf, bufsiz,
> -			     "CORE", NT_PRPSINFO, &data, sizeof (data));
> +  if (get_elf_backend_data (abfd)->linux_prpsinfo64_ugid16)
> +    {
> +      struct elf_external_linux_prpsinfo64_ugid16 data;
> +
> +      swap_linux_prpsinfo64_ugid16_out (abfd, prpsinfo, &data);
> +      return elfcore_write_note (abfd, buf, bufsiz,
> +				 "CORE", NT_PRPSINFO, &data, sizeof (data));
> +    }
> +  else
> +    {
> +      struct elf_external_linux_prpsinfo64_ugid32 data;
> +
> +      swap_linux_prpsinfo64_ugid32_out (abfd, prpsinfo, &data);
> +      return elfcore_write_note (abfd, buf, bufsiz,
> +				 "CORE", NT_PRPSINFO, &data, sizeof (data));
> +    }
>  }
>  
>  char *
> Index: binutils/bfd/elf64-sh64.c
> ===================================================================
> --- binutils.orig/bfd/elf64-sh64.c	2017-08-02 13:13:26.000000000 +0100
> +++ binutils/bfd/elf64-sh64.c	2017-09-20 13:19:29.748292419 +0100
> @@ -3944,6 +3944,8 @@ static const struct bfd_elf_special_sect
>  #define elf_backend_got_header_size	24
>  #define elf_backend_dtrel_excludes_plt	1
>  
> +#define elf_backend_linux_prpsinfo64_ugid16	TRUE
> +
>  #include "elf64-target.h"
>  
>  /* NetBSD support.  */
> Index: binutils/bfd/elfxx-target.h
> ===================================================================
> --- binutils.orig/bfd/elfxx-target.h	2017-09-20 13:03:18.000000000 +0100
> +++ binutils/bfd/elfxx-target.h	2017-09-20 13:20:49.220803426 +0100
> @@ -135,6 +135,9 @@
>  #ifndef elf_backend_linux_prpsinfo32_ugid16
>  #define elf_backend_linux_prpsinfo32_ugid16 FALSE
>  #endif
> +#ifndef elf_backend_linux_prpsinfo64_ugid16
> +#define elf_backend_linux_prpsinfo64_ugid16 FALSE
> +#endif
>  #ifndef elf_backend_stack_align
>  #define elf_backend_stack_align 16
>  #endif
> @@ -895,7 +898,8 @@ static struct elf_backend_data elfNN_bed
>    elf_backend_caches_rawsize,
>    elf_backend_extern_protected_data,
>    elf_backend_always_renumber_dynsyms,
> -  elf_backend_linux_prpsinfo32_ugid16
> +  elf_backend_linux_prpsinfo32_ugid16,
> +  elf_backend_linux_prpsinfo64_ugid16
>  };
>  
>  /* Forward declaration for use when initialising alternative_target field.  */

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/