Re: PATCH: Add x32_corenote.c

[Mark Wielaard <mjw at redhat dot com>]

On Thu, 2015-03-12 at 15:03 -0700, H.J. Lu wrote:
> This patch adds x32_corenote.c to support x32 coredump.  X32 coredump is
> a hybrid between ia32 coredump and x86-64 coredump.  The exact formats
> are described in bfd/hosts/x86-64linux.h in GNU binutils source tree.

So i386 uses EM_386 and ELFCLASS32. x86_64 uses EM_X86_64 and ELFCLASS32
and x32 also uses EM_X86_64 but with ELFCLASS32? Are there any other
differences in ident or other ehdr identifiers?

Will a x86_64 GNU/Linux setup always support x86_64 and one or both of
i386 and x32?

I don't see a gabi processor supplement for x32 here:
http://refspecs.linuxbase.org/elf/index.html
Do you know where it is kept?

Are there any distros using x32 to run some tests on?

If you want to make sure that x32 is correctly supported (also cross
arch) then you might want to provide a couple of test cases and binaries
for things like tests/run-readelf-mixed-corenote.sh,
tests/run-allregs.sh, run-strip-reloc.sh, run-addrcfi.sh,
tests/run-backtrace-core-x32.sh, etc. The files should have a little
description how to generate the test binaries. Don't feel obliged to add
tests for everything at once (adding one test at a time is preferred).
But it would help making sure the arch is properly supported (even if
the test as is just passes without needing any new backend tweaks).

A ChangeLog entry would make review of patches easier. It is also needed
to get this checked in.

> index e3c0109..08fd09f 100644
> --- a/backends/linux-core-note.c
> +++ b/backends/linux-core-note.c

Please add an appropriate copyright notice for your additions to this
file.

> @@ -42,9 +42,20 @@
>  #define	INT			int32_t
>  #define ALIGN_INT		4
>  #define TYPE_INT		ELF_T_SWORD
> +#ifndef PR_REG
> +# define PR_REG			ULONG
> +#endif
>  #ifndef ALIGN_PR_REG
>  # define ALIGN_PR_REG		ALIGN_ULONG
>  #endif
> +#ifndef PRPSINFO_UID_T
> +# define PRPSINFO_UID_T		UID_T
> +# define ALIGN_PRPSINFO_UID_T	ALIGN_UID_T
> +#endif
> +#ifndef PRPSINFO_GID_T
> +# define PRPSINFO_GID_T		GID_T
> +# define ALIGN_PRPSINFO_GID_T	ALIGN_GID_T
> +#endif
>  
>  #define FIELD(type, name) type name __attribute__ ((aligned (ALIGN_##type)))
>  
> @@ -86,7 +97,7 @@ struct EBLHOOK(prstatus)
>    struct EBLHOOK(timeval) pr_cstime;
>    struct
>    {
> -    FIELD (ULONG, pr_reg[PRSTATUS_REGS_SIZE / sizeof (ULONG)]);
> +    FIELD (PR_REG, pr_reg[PRSTATUS_REGS_SIZE / sizeof (PR_REG)]);
>    }
>  #ifdef ALIGN_PR_REG
>      __attribute__ ((aligned (ALIGN_PR_REG)))
> @@ -105,8 +116,8 @@ struct EBLHOOK(prpsinfo)
>    FIELD (CHAR, pr_zomb);
>    FIELD (CHAR, pr_nice);
>    FIELD (ULONG, pr_flag);
> -  FIELD (UID_T, pr_uid);
> -  FIELD (GID_T, pr_gid);
> +  FIELD (PRPSINFO_UID_T, pr_uid);
> +  FIELD (PRPSINFO_GID_T, pr_gid);
>    FIELD (PID_T, pr_pid);
>    FIELD (PID_T, pr_ppid);
>    FIELD (PID_T, pr_pgrp);
> diff --git a/backends/x32_corenote.c b/backends/x32_corenote.c
> new file mode 100644
> index 0000000..bd6560d
> --- /dev/null
> +++ b/backends/x32_corenote.c
> @@ -0,0 +1,2 @@
> +#define BITS 32
> +#include "x86_64_corenote.c"
> diff --git a/backends/x86_64_corenote.c b/backends/x86_64_corenote.c
> index f9d8db4..3fbb0d9 100644
> --- a/backends/x86_64_corenote.c
> +++ b/backends/x86_64_corenote.c
> @@ -36,7 +36,13 @@
>  #include <stdio.h>
>  #include <sys/time.h>
>  
> -#define BACKEND		x86_64_
> +#ifndef BITS
> +# define BITS 		64
> +# define BACKEND	x86_64_
> +#else
> +# define BITS 		32
> +# define BACKEND	x32_
> +#endif
>
>  #include "libebl_CPU.h"
>  
> 
> @@ -77,11 +83,26 @@ static const Ebl_Register_Location prstatus_regs[] =
>    };
>  #define PRSTATUS_REGS_SIZE	(27 * 8)
>  
> -#define	ULONG			uint64_t
> +#if BITS == 32
> +# define ULONG			uint32_t
> +# define ALIGN_ULONG		4
> +# define PRPSINFO_UID_T		uint16_t
> +# define ALIGN_PRPSINFO_UID_T	2
> +# define PRPSINFO_GID_T		uint16_t
> +# define ALIGN_PRPSINFO_GID_T	2
> +#else
> +# define ULONG			uint64_t
> +# define ALIGN_ULONG		8
> +# define PRPSINFO_UID_T		uint32_t
> +# define ALIGN_PRPSINFO_UID_T	4
> +# define PRPSINFO_GID_T		uint32_t
> +# define ALIGN_PRPSINFO_GID_T	4
> +#endif
> +#define PR_REG			uint64_t
> +#define ALIGN_PR_REG		8
>  #define PID_T			int32_t
>  #define	UID_T			uint32_t
>  #define	GID_T			uint32_t
> -#define ALIGN_ULONG		8
>  #define ALIGN_PID_T		4
>  #define ALIGN_UID_T		4
>  #define ALIGN_GID_T		4

This looks correct. But an explicit test for an x32 core added to
tests/run-readelf-mixed-corenote.sh would be appreciated to show it all
works as expected.

> diff --git a/backends/x86_64_init.c b/backends/x86_64_init.c
> index b885558..1197cc0 100644
> --- a/backends/x86_64_init.c
> +++ b/backends/x86_64_init.c
> @@ -38,6 +38,8 @@
>  /* This defines the common reloc hooks based on x86_64_reloc.def.  */
>  #include "common-reloc.c"
>  
> +extern __typeof (EBLHOOK (core_note)) x32_core_note attribute_hidden;
> +
>  const char *
>  x86_64_init (elf, machine, eh, ehlen)
>       Elf *elf __attribute__ ((unused));
> @@ -53,7 +55,10 @@ x86_64_init (elf, machine, eh, ehlen)
>    eh->name = "AMD x86-64";
>    x86_64_init_reloc (eh);
>    HOOK (eh, reloc_simple_type);
> -  HOOK (eh, core_note);
> +  if (eh->class == ELFCLASS32)
> +    eh->core_note = x32_core_note;
> +  else
> +    HOOK (eh, core_note);
>    HOOK (eh, return_value_location);
>    HOOK (eh, register_info);
>    HOOK (eh, syscall_abi);

Is x32 really completely similar to x86_64 to not need its own
backend/x32_init.c? I am surprised all of the other backend hooks don't
need similar tweaks to the core_note one.

You might want to take a look at some of the backend ppc files to see
how they use symbol aliases to be used by both the ppc and ppc64
backends.

Feel free to introduce backend hooks one by one. That might be easiest.
Just want to make sure we are really covering everything.

Thanks,

Mark