Re: [Patch] Fix multithread debugging problems on s390

Re: [Patch] Fix multithread debugging problems on s390

[Gerhard Tonn]

[-- Attachment #1: Type: text/plain, Size: 2454 bytes --]


Attached is the revised patch. I have made both functions more consistent.

The CR registers are not part of gregsetp but included in the register
cache. They are probably not needed at all, I will cleanup that later, when
I will have got a more detailed understanding of gdb.


diff -urN gdb-5.2/gdb/s390-nat.c gdb-5.2-s390/gdb/s390-nat.c
--- gdb-5.2/gdb/s390-nat.c    Tue Nov  6 23:18:13 2001
+++ gdb-5.2-s390/gdb/s390-nat.c     Mon Jul 15 11:48:10 2002
@@ -273,14 +273,22 @@
 void
 fill_gregset (gregset_t * gregsetp, int regno)
 {
+  int regi;
   greg_t *gregp = (greg_t *) gregsetp;

-  if (regno >= S390_FIRST_CR && regno <= S390_LAST_CR)
-    supply_register (regno, NULL);
-  else if (regno != -1)
-    supply_register (regno, (char *) &gregp[regno]);
-  else
-    supply_gregset (gregsetp);
+  if (regno < 0)
+    {
+      regcache_collect (S390_PSWM_REGNUM, &gregp[S390_PSWM_REGNUM]);
+      regcache_collect (S390_PC_REGNUM, &gregp[S390_PC_REGNUM]);
+      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
@@ -291,12 +299,18 @@
 void
 fill_fpregset (fpregset_t * fpregsetp, int regno)
 {
-  if (regno == -1)
-    supply_fpregset (fpregsetp);
-  else
-    supply_register (regno,
-                &((char *) fpregsetp)[REGISTER_BYTE (regno) -
-                                REGISTER_BYTE (S390_FPC_REGNUM)]);
+  int regi;
+
+  if (regno < 0)
+    {
+      regcache_collect (S390_FPC_REGNUM, &fpregsetp->fpc);
+      for (regi = 0; regi < S390_NUM_FPRS; regi++)
+        regcache_collect (S390_FP0_REGNUM + regi, &fpregsetp->fprs[regi]);
+    }
+  else if (regno == S390_FPC_REGNUM)
+    regcache_collect (S390_FPC_REGNUM, &fpregsetp->fpc);
+  else if (regno >= S390_FP0_REGNUM && regno <= S390_FPLAST_REGNUM)
+    regcache_collect (regno, &fpregsetp->fprs[regno - S390_FP0_REGNUM]);
 }




(See attached file: gdb-5.2.1-s390-1.diff)

Regards / Mit freundlichen Gruessen
Gerhard

Gerhard Tonn, Linux for eServer Development, +(49)-7031-16-4716, Lotus
Notes: ton@ibmde,
   Internet: ton@de.ibm.com

[-- Attachment #2: gdb-5.2.1-s390-1.diff --]
[-- Type: application/octet-stream, Size: 1962 bytes --]

diff -urN gdb-5.2/gdb/s390-nat.c gdb-5.2-s390/gdb/s390-nat.c
--- gdb-5.2/gdb/s390-nat.c	Tue Nov  6 23:18:13 2001
+++ gdb-5.2-s390/gdb/s390-nat.c	Mon Jul 15 11:48:10 2002
@@ -273,14 +273,22 @@
 void
 fill_gregset (gregset_t * gregsetp, int regno)
 {
+  int regi;
   greg_t *gregp = (greg_t *) gregsetp;
 
-  if (regno >= S390_FIRST_CR && regno <= S390_LAST_CR)
-    supply_register (regno, NULL);
-  else if (regno != -1)
-    supply_register (regno, (char *) &gregp[regno]);
-  else
-    supply_gregset (gregsetp);
+  if (regno < 0) 
+    {
+      regcache_collect (S390_PSWM_REGNUM, &gregp[S390_PSWM_REGNUM]);
+      regcache_collect (S390_PC_REGNUM, &gregp[S390_PC_REGNUM]);
+      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
@@ -291,12 +299,18 @@
 void
 fill_fpregset (fpregset_t * fpregsetp, int regno)
 {
-  if (regno == -1)
-    supply_fpregset (fpregsetp);
-  else
-    supply_register (regno,
-		     &((char *) fpregsetp)[REGISTER_BYTE (regno) -
-					   REGISTER_BYTE (S390_FPC_REGNUM)]);
+  int regi;
+
+  if (regno < 0) 
+    {
+      regcache_collect (S390_FPC_REGNUM, &fpregsetp->fpc);
+      for (regi = 0; regi < S390_NUM_FPRS; regi++)
+        regcache_collect (S390_FP0_REGNUM + regi, &fpregsetp->fprs[regi]);
+    }
+  else if (regno == S390_FPC_REGNUM)
+    regcache_collect (S390_FPC_REGNUM, &fpregsetp->fpc);
+  else if (regno >= S390_FP0_REGNUM && regno <= S390_FPLAST_REGNUM)
+    regcache_collect (regno, &fpregsetp->fprs[regno - S390_FP0_REGNUM]);
 }
 
 

Re: [Patch] Fix multithread debugging problems on s390

[Andrew Cagney]

Thanks!  I've checked this into mainline and 5.2 branch.

2002-07-15  Andrew Cagney  <ac131313@redhat.com>

         From Gerhard Tonn <TON@de.ibm.com>:
         * s390-nat.c (fill_fpregset, fill_gregset): Use regcache_collect
         instead of supply_register.

Andrew

Re: [Patch] Fix multithread debugging problems on s390

[Andrew Cagney]

> Hi,
> attached is a patch that fixes the multi-thread debugging problems on s390.
> The functions fill_gregset and fill_fpregset were broken, but seem to be
> used only since the gdb thread code has been revised in 5.1. I have
> attached the fix also as file in case there are any line mangling in the
> code below.

> 2002-07-12  Gerhard Tonn  <ton@de.ibm.com>
> 
>             * s390-nat.c (fill_gregset and fill_fpregset):
>               Call regcache_collect functions.
> 
> 
> 
> I would appreciate, if someone could commit it to trunk and 5.2 branch, if
> possible even for 5.2.1

I'll sit on 5.2.1 for slightly longer.  This one is a regression :-(

> Thanks
> 
> --- src/gdb/s390-nat.c  Tue Nov  6 22:18:13 2001
> +++ /home/ton/gdb/src/gdb/s390-nat.c      Wed Jul 10 10:15:26 2002
> @@ -273,14 +273,23 @@
>  void
>  fill_gregset (gregset_t * gregsetp, int regno)
>  {
> +  int regi;
>    greg_t *gregp = (greg_t *) gregsetp;
> 
>    if (regno >= S390_FIRST_CR && regno <= S390_LAST_CR)
> -    supply_register (regno, NULL);
> +    ;

Can you please add a comment indicating that the NO-OP is intentional 
and why.

>    else if (regno != -1)
> -    supply_register (regno, (char *) &gregp[regno]);
> -  else
> -    supply_gregset (gregsetp);
> +    regcache_collect (regno, (char *) &gregp[regno]);
> +  else {

GNU coding standards:

   else
     {
       regcache_collect (...,

> +    regcache_collect (S390_PSWM_REGNUM, (char *)

I don't believe that the cast is needed.  regcache_collect()'s second 
parameter is void.


> &gregp[S390_PSWM_REGNUM]);
> +    regcache_collect (S390_PC_REGNUM, (char *) &gregp[S390_PC_REGNUM]);
> +    for (regi = 0; regi < S390_NUM_GPRS; regi++)
> +      regcache_collect (S390_GP0_REGNUM + regi,
> +           (char *) &gregp[S390_GP0_REGNUM + regi]);

Indentation:

regcache_collect (S390_....,
                   &gregp[...]);

> +  if (regno == -1) {

GNU Coding style.  Also recommend ``< 0'' as safer:

   if (regno < 0)
     {

> +    regcache_collect (S390_FPC_REGNUM, (char *) &fpregsetp->fpc);
> +    for (regi = 0; regi < S390_NUM_FPRS; regi++)
> +      regcache_collect (S390_FP0_REGNUM + regi, (char *) &fpregsetp->fprs[regi]);
> +  }
>    else
> -    supply_register (regno,
> +    regcache_collect (regno,
>                  &((char *) fpregsetp)[REGISTER_BYTE (regno) -
>                                  REGISTER_BYTE (S390_FPC_REGNUM)]);

Can this be written in a way that doesn't use REGISTER_BYTE()? 
REGISTER_BYTE() is going to be removed.

>  }
> 
> (See attached file: gdb-5.2.1-s390.diff)

Andrew

[Patch] Fix multithread debugging problems on s390

[Gerhard Tonn]

[-- Attachment #1: Type: text/plain, Size: 2416 bytes --]

Hi,
attached is a patch that fixes the multi-thread debugging problems on s390.
The functions fill_gregset and fill_fpregset were broken, but seem to be
used only since the gdb thread code has been revised in 5.1. I have
attached the fix also as file in case there are any line mangling in the
code below.

2002-07-12  Gerhard Tonn  <ton@de.ibm.com>

            * s390-nat.c (fill_gregset and fill_fpregset):
              Call regcache_collect functions.



I would appreciate, if someone could commit it to trunk and 5.2 branch, if
possible even for 5.2.1

Thanks

--- src/gdb/s390-nat.c  Tue Nov  6 22:18:13 2001
+++ /home/ton/gdb/src/gdb/s390-nat.c      Wed Jul 10 10:15:26 2002
@@ -273,14 +273,23 @@
 void
 fill_gregset (gregset_t * gregsetp, int regno)
 {
+  int regi;
   greg_t *gregp = (greg_t *) gregsetp;

   if (regno >= S390_FIRST_CR && regno <= S390_LAST_CR)
-    supply_register (regno, NULL);
+    ;
   else if (regno != -1)
-    supply_register (regno, (char *) &gregp[regno]);
-  else
-    supply_gregset (gregsetp);
+    regcache_collect (regno, (char *) &gregp[regno]);
+  else {
+    regcache_collect (S390_PSWM_REGNUM, (char *)
&gregp[S390_PSWM_REGNUM]);
+    regcache_collect (S390_PC_REGNUM, (char *) &gregp[S390_PC_REGNUM]);
+    for (regi = 0; regi < S390_NUM_GPRS; regi++)
+      regcache_collect (S390_GP0_REGNUM + regi,
+           (char *) &gregp[S390_GP0_REGNUM + regi]);
+    for (regi = 0; regi < S390_NUM_ACRS; regi++)
+      regcache_collect (S390_FIRST_ACR + regi,
+           (char *) &gregp[S390_FIRST_ACR + regi]);
+  }
 }

 /*  Given a pointer to a floating point register set in /proc format
@@ -291,10 +300,15 @@
 void
 fill_fpregset (fpregset_t * fpregsetp, int regno)
 {
-  if (regno == -1)
-    supply_fpregset (fpregsetp);
+  int regi;
+
+  if (regno == -1) {
+    regcache_collect (S390_FPC_REGNUM, (char *) &fpregsetp->fpc);
+    for (regi = 0; regi < S390_NUM_FPRS; regi++)
+      regcache_collect (S390_FP0_REGNUM + regi, (char *) &fpregsetp->fprs[regi]);
+  }
   else
-    supply_register (regno,
+    regcache_collect (regno,
                 &((char *) fpregsetp)[REGISTER_BYTE (regno) -
                                 REGISTER_BYTE (S390_FPC_REGNUM)]);
 }

(See attached file: gdb-5.2.1-s390.diff)

Regards / Mit freundlichen Gruessen
Gerhard

Gerhard Tonn, Linux for eServer Development, +(49)-7031-16-4716, Lotus Notes: ton@ibmde,
   Internet: ton@de.ibm.com

[-- Attachment #2: gdb-5.2.1-s390.diff --]
[-- Type: application/octet-stream, Size: 1638 bytes --]

--- src/gdb/s390-nat.c	Tue Nov  6 22:18:13 2001
+++ /home/ton/gdb/src/gdb/s390-nat.c	Wed Jul 10 10:15:26 2002
@@ -273,14 +273,23 @@
 void
 fill_gregset (gregset_t * gregsetp, int regno)
 {
+  int regi;
   greg_t *gregp = (greg_t *) gregsetp;
 
   if (regno >= S390_FIRST_CR && regno <= S390_LAST_CR)
-    supply_register (regno, NULL);
+    ;
   else if (regno != -1)
-    supply_register (regno, (char *) &gregp[regno]);
-  else
-    supply_gregset (gregsetp);
+    regcache_collect (regno, (char *) &gregp[regno]);
+  else {
+    regcache_collect (S390_PSWM_REGNUM, (char *) &gregp[S390_PSWM_REGNUM]);
+    regcache_collect (S390_PC_REGNUM, (char *) &gregp[S390_PC_REGNUM]);
+    for (regi = 0; regi < S390_NUM_GPRS; regi++)
+      regcache_collect (S390_GP0_REGNUM + regi,
+		(char *) &gregp[S390_GP0_REGNUM + regi]);
+    for (regi = 0; regi < S390_NUM_ACRS; regi++)
+      regcache_collect (S390_FIRST_ACR + regi,
+		(char *) &gregp[S390_FIRST_ACR + regi]);
+  }
 }
 
 /*  Given a pointer to a floating point register set in /proc format
@@ -291,10 +300,15 @@
 void
 fill_fpregset (fpregset_t * fpregsetp, int regno)
 {
-  if (regno == -1)
-    supply_fpregset (fpregsetp);
+  int regi;
+
+  if (regno == -1) {
+    regcache_collect (S390_FPC_REGNUM, (char *) &fpregsetp->fpc);
+    for (regi = 0; regi < S390_NUM_FPRS; regi++)
+      regcache_collect (S390_FP0_REGNUM + regi, (char *) &fpregsetp->fprs[regi]);
+  }
   else
-    supply_register (regno,
+    regcache_collect (regno,
 		     &((char *) fpregsetp)[REGISTER_BYTE (regno) -
 					   REGISTER_BYTE (S390_FPC_REGNUM)]);
 }