Re: [patch] Implement reglocs for s390/s390x

[Roland McGrath <roland at hack dot frob dot com>]

> Formats 'b', 'B' and '\n' are really very special as they process everything
> till the end of the note.  So it does not make sense to combine them with
> printing some other data as there are no other data.

That's always true of '\n'.  For 'b' and 'B' it's only true when it's the
only item in the note (i.e. true for ioperm, false for sigpend/sighold).

I don't think the code should special-case 'b' or 'B' happening to be the
first item.  The special case is really only for nitems==1, which is the
only time the repeated_size behavior really makes sense.  (Theoretically it
could also apply sensically to the last item even if there were multiple,
but since we don't have any such situations today I don't think we want to
try to account for them.)

Incidentally, I would be entirely fine with thoroughly revamping the
details of the Ebl_Core_Item scheme with the format letters.  As is clear
from all these issues, I just came up with it ad hoc as the simplest
quasi-generic table-driven thing I could think of to get the output I
wanted without resorting to backends actually supplying printing code
(which is not a good fit for any other use of this interface than readelf).
We would like to handle NT_X86_XSAVE, which has a more hairy format and
probably won't fit into the current scheme very well.  I haven't thought
about any of it in more detail than that and I don't expect you to worry
about the subject any time soon, but I thought I'd mention it as a future
project for somebody or other.

> The bug was that format 'x' was processed by this special code.

Agreed.

> > 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?
> 
> It should be fixed now as part of the change.

I don't see how that's fixed.

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

Note your log entries are now out of date, referring to the old version of
the readelf.c changes.

> OK, thanks (I was not aware 'aligned' aligns even end - not just start - of
> fields).
>
> But I had to create there also PR_FPVALID_TYPE as both pr_reg and whole
> pr_status have extra alignment to 8 bytes on s390.

Sorry, I was mistaken.  I was thinking that it behaved like it does for
types, where it has to increase the size so that an array of that type
would have the right alignment for each member.

So what I think is really the clean way to do this is to define a separate
type for pr_reg rather than making it an array itself.  That matches the
way the prstatus struct is actually defined in the kernel.  So I think it
can just be:

struct EBLHOOK(elf_gregset)
{
  FIELD (ULONG, pr_reg[PRSTATUS_REGS_SIZE / sizeof (ULONG)]);
} 
#ifdef ALIGN_PR_REG
  __attribute__ ((aligned (ALIGN_PR_REG)))
#endif
  ;

and then:

  struct EBLHOOK(elf_gregset) pr_reg;

in EBLHOOK(prstatus).

> +static const Ebl_Core_Item system_call_items[] =
> +  {
> +    {
> +      .name = "system_call", .group = "system", .offset = 0, .type = ELF_T_WORD,
> +      .format = 'x',
> +    },
> +  };

System call numbers are usually used in decimal.


Thanks,
Roland