Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* RFC: s390x: correct core file register layout
@ 2003-05-23  6:58 Jim Blandy
  2003-06-03  0:59 ` Michael Snyder
  2003-06-03  5:49 ` Jim Blandy
  0 siblings, 2 replies; 6+ messages in thread
From: Jim Blandy @ 2003-05-23  6:58 UTC (permalink / raw)
  To: gdb-patches


2003-05-23  Jim Blandy  <jimb@redhat.com>

	* s390-nat.c (supply_gregset, fill_gregset): On the s390x, the
	elements of gregset_t are 64 bits each, but access registers
	are still 32 bits, so they're packed two per gregset_t
	element.  Unpack/pack them properly.

Index: gdb/s390-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/s390-nat.c,v
retrieving revision 1.7
diff -c -r1.7 s390-nat.c
*** gdb/s390-nat.c	14 Apr 2003 14:21:07 -0000	1.7
--- gdb/s390-nat.c	23 May 2003 06:52:07 -0000
***************
*** 251,259 ****
    for (regi = 0; regi < S390_NUM_GPRS; regi++)
      supply_register (S390_GP0_REGNUM + regi,
  		     (char *) &gregp[S390_GP0_REGNUM + regi]);
    for (regi = 0; regi < S390_NUM_ACRS; regi++)
      supply_register (S390_FIRST_ACR + regi,
! 		     (char *) &gregp[S390_FIRST_ACR + regi]);
    /* unfortunately this isn't in gregsetp */
    for (regi = 0; regi < S390_NUM_CRS; regi++)
      supply_register (S390_FIRST_CR + regi, NULL);
--- 251,276 ----
    for (regi = 0; regi < S390_NUM_GPRS; regi++)
      supply_register (S390_GP0_REGNUM + regi,
  		     (char *) &gregp[S390_GP0_REGNUM + regi]);
+ 
+ #if defined (CONFIG_ARCH_S390X)
+   /* On the s390x, each element of gregset_t is 8 bytes long, but
+      each access register is still only 32 bits long.  So they're
+      packed two per element.  It's apparently traditional that
+      gregset_t must be an array, so when the registers it provides
+      have different sizes, something has to get strange
+      somewhere.  */
+   {
+     unsigned int *acrs = (unsigned int *) &gregp[S390_FIRST_ACR];
+ 
+     for (regi = 0; regi < S390_NUM_ACRS; regi++)
+       supply_register (S390_FIRST_ACR + regi, (char *) &acrs[regi]);
+   }
+ #else
    for (regi = 0; regi < S390_NUM_ACRS; regi++)
      supply_register (S390_FIRST_ACR + regi,
!                      (char *) &gregp[S390_FIRST_ACR + regi]);
! #endif
! 
    /* unfortunately this isn't in gregsetp */
    for (regi = 0; regi < S390_NUM_CRS; regi++)
      supply_register (S390_FIRST_CR + regi, NULL);
***************
*** 284,295 ****
        for (regi = 0; regi < S390_NUM_GPRS; regi++)
          regcache_collect (S390_GP0_REGNUM + regi,
  			  &gregp[S390_GP0_REGNUM + regi]);
        for (regi = 0; regi < S390_NUM_ACRS; regi++)
          regcache_collect (S390_FIRST_ACR + regi,
  			  &gregp[S390_FIRST_ACR + regi]);
      }
!   else if (regno >= S390_PSWM_REGNUM && regno <= S390_LAST_ACR)
      regcache_collect (regno, &gregp[regno]);
  }
  
  /*  Given a pointer to a floating point register set in /proc format
--- 301,335 ----
        for (regi = 0; regi < S390_NUM_GPRS; regi++)
          regcache_collect (S390_GP0_REGNUM + regi,
  			  &gregp[S390_GP0_REGNUM + regi]);
+ #if defined (CONFIG_ARCH_S390X)
+       /* See the comments about the access registers in
+          supply_gregset, above.  */
+       {
+         unsigned int *acrs = (unsigned int *) &gregp[S390_FIRST_ACR];
+         
+         for (regi = 0; regi < S390_NUM_ACRS; regi++)
+           regcache_collect (S390_FIRST_ACR + regi, &acrs[regi]);
+       }
+ #else
        for (regi = 0; regi < S390_NUM_ACRS; regi++)
          regcache_collect (S390_FIRST_ACR + regi,
  			  &gregp[S390_FIRST_ACR + regi]);
+ #endif
      }
!   else if (regno >= S390_PSWM_REGNUM && regno < S390_FIRST_ACR)
      regcache_collect (regno, &gregp[regno]);
+   else if (regno >= S390_FIRST_ACR && regno <= S390_LAST_ACR)
+     {
+ #if defined (CONFIG_ARCH_S390X)
+       /* See the comments about the access registers in
+          supply_gregset, above.  */
+       unsigned int *acrs = (unsigned int *) &gregp[S390_FIRST_ACR];
+         
+       regcache_collect (regno, &acrs[regno - S390_FIRST_ACR]);
+ #else
+       regcache_collect (regno, &gregp[regno]);
+ #endif
+     }
  }
  
  /*  Given a pointer to a floating point register set in /proc format


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: RFC: s390x: correct core file register layout
  2003-05-23  6:58 RFC: s390x: correct core file register layout Jim Blandy
@ 2003-06-03  0:59 ` Michael Snyder
  2003-06-03  5:48   ` Jim Blandy
  2003-06-03  5:49 ` Jim Blandy
  1 sibling, 1 reply; 6+ messages in thread
From: Michael Snyder @ 2003-06-03  0:59 UTC (permalink / raw)
  To: Jim Blandy; +Cc: gdb-patches

Jim Blandy wrote:
> 
> 2003-05-23  Jim Blandy  <jimb@redhat.com>
> 
>         * s390-nat.c (supply_gregset, fill_gregset): On the s390x, the
>         elements of gregset_t are 64 bits each, but access registers
>         are still 32 bits, so they're packed two per gregset_t
>         element.  Unpack/pack them properly.

What sort of comment are you looking for?

 
> Index: gdb/s390-nat.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/s390-nat.c,v
> retrieving revision 1.7
> diff -c -r1.7 s390-nat.c
> *** gdb/s390-nat.c      14 Apr 2003 14:21:07 -0000      1.7
> --- gdb/s390-nat.c      23 May 2003 06:52:07 -0000
> ***************
> *** 251,259 ****
>     for (regi = 0; regi < S390_NUM_GPRS; regi++)
>       supply_register (S390_GP0_REGNUM + regi,
>                      (char *) &gregp[S390_GP0_REGNUM + regi]);
>     for (regi = 0; regi < S390_NUM_ACRS; regi++)
>       supply_register (S390_FIRST_ACR + regi,
> !                    (char *) &gregp[S390_FIRST_ACR + regi]);
>     /* unfortunately this isn't in gregsetp */
>     for (regi = 0; regi < S390_NUM_CRS; regi++)
>       supply_register (S390_FIRST_CR + regi, NULL);
> --- 251,276 ----
>     for (regi = 0; regi < S390_NUM_GPRS; regi++)
>       supply_register (S390_GP0_REGNUM + regi,
>                      (char *) &gregp[S390_GP0_REGNUM + regi]);
> +
> + #if defined (CONFIG_ARCH_S390X)
> +   /* On the s390x, each element of gregset_t is 8 bytes long, but
> +      each access register is still only 32 bits long.  So they're
> +      packed two per element.  It's apparently traditional that
> +      gregset_t must be an array, so when the registers it provides
> +      have different sizes, something has to get strange
> +      somewhere.  */
> +   {
> +     unsigned int *acrs = (unsigned int *) &gregp[S390_FIRST_ACR];
> +
> +     for (regi = 0; regi < S390_NUM_ACRS; regi++)
> +       supply_register (S390_FIRST_ACR + regi, (char *) &acrs[regi]);
> +   }
> + #else
>     for (regi = 0; regi < S390_NUM_ACRS; regi++)
>       supply_register (S390_FIRST_ACR + regi,
> !                      (char *) &gregp[S390_FIRST_ACR + regi]);
> ! #endif
> !
>     /* unfortunately this isn't in gregsetp */
>     for (regi = 0; regi < S390_NUM_CRS; regi++)
>       supply_register (S390_FIRST_CR + regi, NULL);
> ***************
> *** 284,295 ****
>         for (regi = 0; regi < S390_NUM_GPRS; regi++)
>           regcache_collect (S390_GP0_REGNUM + regi,
>                           &gregp[S390_GP0_REGNUM + regi]);
>         for (regi = 0; regi < S390_NUM_ACRS; regi++)
>           regcache_collect (S390_FIRST_ACR + regi,
>                           &gregp[S390_FIRST_ACR + regi]);
>       }
> !   else if (regno >= S390_PSWM_REGNUM && regno <= S390_LAST_ACR)
>       regcache_collect (regno, &gregp[regno]);
>   }
> 
>   /*  Given a pointer to a floating point register set in /proc format
> --- 301,335 ----
>         for (regi = 0; regi < S390_NUM_GPRS; regi++)
>           regcache_collect (S390_GP0_REGNUM + regi,
>                           &gregp[S390_GP0_REGNUM + regi]);
> + #if defined (CONFIG_ARCH_S390X)
> +       /* See the comments about the access registers in
> +          supply_gregset, above.  */
> +       {
> +         unsigned int *acrs = (unsigned int *) &gregp[S390_FIRST_ACR];
> +
> +         for (regi = 0; regi < S390_NUM_ACRS; regi++)
> +           regcache_collect (S390_FIRST_ACR + regi, &acrs[regi]);
> +       }
> + #else
>         for (regi = 0; regi < S390_NUM_ACRS; regi++)
>           regcache_collect (S390_FIRST_ACR + regi,
>                           &gregp[S390_FIRST_ACR + regi]);
> + #endif
>       }
> !   else if (regno >= S390_PSWM_REGNUM && regno < S390_FIRST_ACR)
>       regcache_collect (regno, &gregp[regno]);
> +   else if (regno >= S390_FIRST_ACR && regno <= S390_LAST_ACR)
> +     {
> + #if defined (CONFIG_ARCH_S390X)
> +       /* See the comments about the access registers in
> +          supply_gregset, above.  */
> +       unsigned int *acrs = (unsigned int *) &gregp[S390_FIRST_ACR];
> +
> +       regcache_collect (regno, &acrs[regno - S390_FIRST_ACR]);
> + #else
> +       regcache_collect (regno, &gregp[regno]);
> + #endif
> +     }
>   }
> 
>   /*  Given a pointer to a floating point register set in /proc format


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: RFC: s390x: correct core file register layout
  2003-06-03  0:59 ` Michael Snyder
@ 2003-06-03  5:48   ` Jim Blandy
  2003-06-03 13:03     ` Daniel Jacobowitz
  0 siblings, 1 reply; 6+ messages in thread
From: Jim Blandy @ 2003-06-03  5:48 UTC (permalink / raw)
  To: Michael Snyder; +Cc: gdb-patches

Michael Snyder <msnyder@redhat.com> writes:

> Jim Blandy wrote:
> > 
> > 2003-05-23  Jim Blandy  <jimb@redhat.com>
> > 
> >         * s390-nat.c (supply_gregset, fill_gregset): On the s390x, the
> >         elements of gregset_t are 64 bits each, but access registers
> >         are still 32 bits, so they're packed two per gregset_t
> >         element.  Unpack/pack them properly.
> 
> What sort of comment are you looking for?

Well, lewd ones, in particular.  But given the nature of the patch I
wasn't expecting much along those lines, and would have settled for
"this isn't the way we deal with native targets, idiot, look at
foo-nat.c" and stuff like that.

I take it it's kosher to use CONFIG_ARCH_foo in -nat.c files, right?
I feel icky writing that in these modern gdbarch'ed times.  But as
long as we're getting types like gregset_t from the system headers,
the decision on how registers are laid out within that type is
inevitably a compile-time thing, so it's legitimate to use #ifdefs to
select the appropriate code.  Right?

Ideally, I was hoping someone from IBM would check it for
correctness.  But they don't seem to follow these lists,
unfortunately.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: RFC: s390x: correct core file register layout
  2003-05-23  6:58 RFC: s390x: correct core file register layout Jim Blandy
  2003-06-03  0:59 ` Michael Snyder
@ 2003-06-03  5:49 ` Jim Blandy
  1 sibling, 0 replies; 6+ messages in thread
From: Jim Blandy @ 2003-06-03  5:49 UTC (permalink / raw)
  To: gdb-patches


I've committed this patch.  (But as always, I'd be happy to revise it
if someone had nice suggestions.)

Jim Blandy <jimb@redhat.com> writes:

> 2003-05-23  Jim Blandy  <jimb@redhat.com>
> 
> 	* s390-nat.c (supply_gregset, fill_gregset): On the s390x, the
> 	elements of gregset_t are 64 bits each, but access registers
> 	are still 32 bits, so they're packed two per gregset_t
> 	element.  Unpack/pack them properly.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: RFC: s390x: correct core file register layout
  2003-06-03  5:48   ` Jim Blandy
@ 2003-06-03 13:03     ` Daniel Jacobowitz
  2003-06-03 16:59       ` Jim Blandy
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Jacobowitz @ 2003-06-03 13:03 UTC (permalink / raw)
  To: Jim Blandy; +Cc: Michael Snyder, gdb-patches

On Tue, Jun 03, 2003 at 12:48:16AM -0500, Jim Blandy wrote:
> Michael Snyder <msnyder@redhat.com> writes:
> 
> > Jim Blandy wrote:
> > > 
> > > 2003-05-23  Jim Blandy  <jimb@redhat.com>
> > > 
> > >         * s390-nat.c (supply_gregset, fill_gregset): On the s390x, the
> > >         elements of gregset_t are 64 bits each, but access registers
> > >         are still 32 bits, so they're packed two per gregset_t
> > >         element.  Unpack/pack them properly.
> > 
> > What sort of comment are you looking for?
> 
> Well, lewd ones, in particular.  But given the nature of the patch I
> wasn't expecting much along those lines, and would have settled for
> "this isn't the way we deal with native targets, idiot, look at
> foo-nat.c" and stuff like that.
> 
> I take it it's kosher to use CONFIG_ARCH_foo in -nat.c files, right?
> I feel icky writing that in these modern gdbarch'ed times.  But as
> long as we're getting types like gregset_t from the system headers,
> the decision on how registers are laid out within that type is
> inevitably a compile-time thing, so it's legitimate to use #ifdefs to
> select the appropriate code.  Right?
> 
> Ideally, I was hoping someone from IBM would check it for
> correctness.  But they don't seem to follow these lists,
> unfortunately.

I missed the salient details because I only skimmed it the first time. 
How about "this isn't the way we want to deal with core files, look at
bfd/elf.c and mips-linux-tdep.c".  Are any two of the gregset types
actually the same size?  If not, in *grok_prstatus, you can autodetect
based on the note size.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: RFC: s390x: correct core file register layout
  2003-06-03 13:03     ` Daniel Jacobowitz
@ 2003-06-03 16:59       ` Jim Blandy
  0 siblings, 0 replies; 6+ messages in thread
From: Jim Blandy @ 2003-06-03 16:59 UTC (permalink / raw)
  To: Daniel Jacobowitz; +Cc: Michael Snyder, gdb-patches

Daniel Jacobowitz <drow@mvista.com> writes:

> On Tue, Jun 03, 2003 at 12:48:16AM -0500, Jim Blandy wrote:
> > Michael Snyder <msnyder@redhat.com> writes:
> > 
> > > Jim Blandy wrote:
> > > > 
> > > > 2003-05-23  Jim Blandy  <jimb@redhat.com>
> > > > 
> > > >         * s390-nat.c (supply_gregset, fill_gregset): On the s390x, the
> > > >         elements of gregset_t are 64 bits each, but access registers
> > > >         are still 32 bits, so they're packed two per gregset_t
> > > >         element.  Unpack/pack them properly.
> > > 
> > > What sort of comment are you looking for?
> > 
> > Well, lewd ones, in particular.  But given the nature of the patch I
> > wasn't expecting much along those lines, and would have settled for
> > "this isn't the way we deal with native targets, idiot, look at
> > foo-nat.c" and stuff like that.
> > 
> > I take it it's kosher to use CONFIG_ARCH_foo in -nat.c files, right?
> > I feel icky writing that in these modern gdbarch'ed times.  But as
> > long as we're getting types like gregset_t from the system headers,
> > the decision on how registers are laid out within that type is
> > inevitably a compile-time thing, so it's legitimate to use #ifdefs to
> > select the appropriate code.  Right?
> > 
> > Ideally, I was hoping someone from IBM would check it for
> > correctness.  But they don't seem to follow these lists,
> > unfortunately.
> 
> I missed the salient details because I only skimmed it the first time. 
> How about "this isn't the way we want to deal with core files, look at
> bfd/elf.c and mips-linux-tdep.c".  Are any two of the gregset types
> actually the same size?  If not, in *grok_prstatus, you can autodetect
> based on the note size.

Ahh, now that's a comment.

I'm stuck in PPC64-land at the moment, but I'll put together a
revision.


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2003-06-03 16:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-05-23  6:58 RFC: s390x: correct core file register layout Jim Blandy
2003-06-03  0:59 ` Michael Snyder
2003-06-03  5:48   ` Jim Blandy
2003-06-03 13:03     ` Daniel Jacobowitz
2003-06-03 16:59       ` Jim Blandy
2003-06-03  5:49 ` Jim Blandy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox