Re: PATCH: Add x32_corenote.c

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

On Tue, Mar 24, 2015 at 6:48 AM, Mark Wielaard <mjw@redhat.com> wrote:
> 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

x86_64 uses EM_X86_64 with ELFCLASS64.  x32  uses EM_X86_64
with ELFCLASS32.

> 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?

x86_64 GNU/Linux will always support x86-64.  i386 and x32 support
is optional.

> 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?

http://www.x86-64.org/svn/trunk/x86-64-ABI/

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

Ubuntu 14.02.

> 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).

I will add those.

> 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.

Will do.

>
>> @@ -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.

Will do.

>> 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.

X32 is very similar to x86-64.  The main difference is ELF class.
All other backends similar x32 have

ppc64_corenote.c:#include "ppc_corenote.c"
s390x_corenote.c:#include "s390_corenote.c"
sparc64_corenote.c:#include "sparc_corenote.c"


> 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.

X32 only needs one hook for coredump so far.


-- 
H.J.