This is the mail archive of the mailing list for the elfutils project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] Implement reglocs for s390/s390x

> It is there for special handling of ./tests/ .
> So I put there 'offset == 0' which is still compatible with VMCOREINFO and
> removed the unclean previous fix.

Sorry if I'm being dense.  But I think we still need some more explanation
about this, and should make it clear in comments.  (Of course it's my fault
for not doing this five years ago, but I don't have time to puzzle through
it now myself.)  The comment shouldn't say it's a special case for a
particular note type.  If anything, say it's a special case for '\n'
format.  But I'd still like first to see some more thorough explanation
about what the special case actually does differently than the general

Incidentally, it looks to me like a '\n' item with descsz==0 will crash
(assert) since Petr's change.  Can you verify that and fix it?  Probably
handle_core_items should just bail out at the top for descsz==0.

I think any readelf changes we settle on should be done in a separate
change before the backend additions.

> I have both fixed it and reworded it, 32-bit s390 has unusual 8 bytes
> alignment.  This fixed displaying the pr_fpvalid field.

I see.  

> --- a/backends/linux-core-note.c
> +++ b/backends/linux-core-note.c
> @@ -83,6 +83,9 @@ struct EBLHOOK(prstatus)
>    struct EBLHOOK(timeval) pr_cstime;
>    FIELD (ULONG, pr_reg[PRSTATUS_REGS_SIZE / sizeof (ULONG)]);
>    FIELD (INT, pr_fpvalid);
> +#ifdef ALIGN_STRUCT
> +  char alignment[] __attribute__ ((aligned (ALIGN_STRUCT)));
> +#endif
>  };

The aligned attribute should be on the pr_reg field, since that's
what actually has that alignment requirement.  What I'd do is:

#ifndef PR_REG_TYPE

and use PR_REG_TYPE rather than ULONG in the pr_reg field declaration.

Then in s390_corenote.c you can do:


and be done.

> +#ifndef BITS
> +# define BITS 		32
> +# define BACKEND	s390_
> +#else
> +# define BITS 		64
> +# define BACKEND	s390x_
> +#endif

#ifdef BITS
# define BITS

is odd.  But I see you were following the model of the existing ones.
So I guess it's fine that way.

> +static const Ebl_Core_Item fpregset_items[] =
> +  {
> +    {
> +      .name = "fpc", .group = "register",
> +      .offset = 0,
> +      .count = 0, .type = ELF_T_WORD,
> +      .format = 'x',
> +    },
> +  };

It's weird to use .count = 0 to mean .count = 1.
I know arm_corenote.c does that, but I don't know why.
It's more normal just to omit .count entirely so it's implicitly zero.

> +      .name = "high_r" #n , .group = "register", .offset = (n) * 4,	\
> +      .count = 0, .type = ELF_T_WORD, .format = 'x',			\

Omit .count here too.

> +    HR (0), HR (1), HR (2), HR (3), HR (4), HR (5), HR (6), HR (7), HR (8),
> +    HR (9), HR (10), HR (11), HR (12), HR (13), HR (14), HR (15)

Split the line symmetrically so there are eight items on each line.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]