> TBH I do not know what S390_HIGH_GPRS are for. From what I read they are > not present on s390 iron, they are also not present for s390x processes, > they are present only for s390 processes on s390x iron/kernel. But gcc > -m31 still does not use the upper halves of r0-r15 there. I could ask at > IBM but so far I ignore S390_HIGH_GPRS in this and further patches. They > are now just printed by readelf, nothing more.
You should check with some s390 folks. My understanding is that 31-bit processes on 64-bit kernels are free to use the full 64-bit registers. There is an HWCAP bit that says this is available. glibc has IFUNC-selected assembly implementations of mem* functions that use these. So it is important that debuggers and such can see these values. I don't know what the story is on DWARF register number mapping for those. i.e., if there are separate DWARF register numbers assigned for the high halves or what. > I also do not understand much what is there the exception <nitems == 1> in > readelf.c but it crashes without the fix for s390 core file S390_LAST_BREAK. I don't recall enough any more about this. Let's get some clarity before changing it. I did it for some reason. So let's figure out what core file input produces different -n output with and without this code, and then add some comments and go from there on exactly how it should be. > 2012-10-09 Jan Kratochvil <[email protected]> > > * Makefile.am (s390_SRCS): Add s390_corenote.c and s390x_corenote.c. > * s390_corenote.c: new file. > * s390_init.c (s390_init): Install s390x_core_note and s390_core_note. > * s390x_corenote.c: new file. Capitalize "New file". > +static const Ebl_Register_Location prstatus_regs[] = > + { > +#define GR(at, n, dwreg) \ > + { .offset = at * BITS/8, .regno = dwreg, .count = n, .bits = BITS } > + > + GR (0, 1, 64), /* pswm */ > + GR (1, 1, 65), /* pswa */ > + GR (2, 16, 0), /* r0-r15 */ Put a blank line here. > + /* ar0-r15 */ > + { .offset = 18 * BITS/8, .regno = 48, .count = 16, .bits = 32 } > + /* ar15 end is at "offset" (BITS == 32 ? 18 + 16 == 34 : 18 + 16 / 2 == > 26). > + orig_r2 is at "offset" (BITS == 32 ? 34 : 26). */ I'm not sure I understand this comment or why it's here. > +#define PRSTATUS_REGS_SIZE (BITS / 8 * (BITS == 32 ? 37 : 27)) I don't understand this difference. What are the 10 extra words for 32-bit? > +static const Ebl_Register_Location fpregset_regs[] = > + { > +#define FPR(at, n, dwreg) \ > + { .offset = at * 64/8, .regno = dwreg, .count = n, .bits = 64 } > + > + /* fpc is at 0. */ So there is no DWARF number for fpc? Then say so in the comment. And there should be an extra Ebl_Core_Item describing fpc. > +#define PRSTATUS_REGSET_ITEMS \ > + { \ > + .name = "orig_r2", .type = TYPE_LONG, .format = 'd', \ > + .offset = offsetof (struct EBLHOOK(prstatus), \ > + pr_reg[BITS == 32 ? 34 : 26]), .group = "register" \ Bad indentation here. It can be: #define PRSTATUS_REGSET_ITEMS \ { \ .name = "orig_r2", .type = TYPE_LONG, .format = 'd', \ .offset = offsetof (struct EBLHOOK(prstatus), \ pr_reg[BITS == 32 ? 34 : 26]), \ .group = "register" \ } > +static const Ebl_Core_Item no_items[0]; Bletch! You don't need this. Just use EXTRA_REGSET instead of EXTRA_REGSET_ITEMS. > +static const Ebl_Register_Location high_regs[] = > + { > + /* Upper halves of r0-r15 are stored here. > + FIXME: It is currently not combined with the r0-r15 lower halves. */ > + { .offset = 0, .regno = 0, .count = 16, .bits = 32 }, > + }; This can't be right, because those DWARF register numbers are already taken. If they don't have their own register numbers, then there are two possibilities: 1. We define a first-class concept for split registers, so something generically knows how to combine upper and lower halves. 2. We just treat these as non-register values, using Ebl_Core_Item so readelf will print them separately but nothing will know how they correspond to any DWARF-described registers. #2 is easy. #1 would need to be designed. > +static const Ebl_Core_Item last_break_items[] = > + { > + { > + .name = "last_break", .group = "last_break", > + .offset = BITS == 32 ? 4 : 0, > + .count = 0, .type = BITS == 32 ? ELF_T_WORD : ELF_T_XWORD, > + .format = 'x', > + }, > + }; > + > +static const Ebl_Core_Item system_call_items[] = > + { > + { > + .name = "system_call", .group = "system_call", > + .offset = 0, > + .count = 0, .type = ELF_T_WORD, > + .format = 'x', > + }, > + }; The groups are pretty useless if everything has its own group. I'd put these two together in a group called "system" or something. > +static const Ebl_Register_Location no_regs[0]; Bletch again. Just add an EXTRA_ITEMS macro to linux-core-note.c. > + if (eh->class == ELFCLASS64) > + { > + __typeof (s390_core_note) s390x_core_note; > + eh->core_note = s390x_core_note; It would be better to avoid a local declaration like this. But if it's easiest to have it, at least put extern on it. It would look better to do this declaration at top level and have a one-line if body. Thanks, Roland _______________________________________________ elfutils-devel mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/elfutils-devel
