patch: signal trampoline frame cache corruption (repost?)

patch: signal trampoline frame cache corruption (repost?)

[PAUL GILLIAM]

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

This might be a repost.  

Attached is a patch to fix some signal trampoline frame cache corruption
that occurs on machines that have no FPU registers.  The corruption was
reported when running gdb 6.4 on a PowerPC 440.  If a backtrace command
was run after hitting a signal trampoline frame, the frame cache would
become corrupted and GDB would SIGSEGV.

I have checked the cvs head sources, and the code has moved, but has not
been fixed.  So I updated the patch to apply to cvs head.

Ok to commit?  (This is almost an obvious fix, isn't it?)



[-- Attachment #2: fix-sigtramp-cache.patch --]
[-- Type: text/x-patch, Size: 1049 bytes --]

---
 gdb/ppc-linux-tdep.c |   18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

--- gdb-6.4.orig/gdb/ppc-linux-tdep.c
+++ gdb-6.4/gdb/ppc-linux-tdep.c
@@ -967,14 +967,16 @@ ppc_linux_sigtramp_cache (struct frame_i
   trad_frame_set_reg_addr (this_cache, tdep->ppc_cr_regnum,
 			   gpregs + 38 * tdep->wordsize);
 
-  /* Floating point registers.  */
-  for (i = 0; i < 32; i++)
-    {
-      int regnum = i + FP0_REGNUM;
-      trad_frame_set_reg_addr (this_cache, regnum, fpregs + i * tdep->wordsize);
-    }
-  trad_frame_set_reg_addr (this_cache, tdep->ppc_fpscr_regnum,
-			   fpregs + 32 * tdep->wordsize);
+  if (ppc_floating_point_unit_p(gdbarch)) {
+	  /* Floating point registers.  */
+	  for (i = 0; i < 32; i++)
+	  {
+		  int regnum = i + FP0_REGNUM;
+		  trad_frame_set_reg_addr (this_cache, regnum, fpregs + i * tdep->wordsize);
+	  }
+	  trad_frame_set_reg_addr (this_cache, tdep->ppc_fpscr_regnum,
+			  fpregs + 32 * tdep->wordsize);
+  }
   trad_frame_set_id (this_cache, frame_id_build (base, func));
 }
 

Re: patch: signal trampoline frame cache corruption (repost?)

[Mark Kettenis]

> From: PAUL GILLIAM <pgilliam@us.ibm.com>
> Date: Tue, 31 Oct 2006 11:33:47 -0800
> 
> This might be a repost.  
> 
> Attached is a patch to fix some signal trampoline frame cache corruption
> that occurs on machines that have no FPU registers.  The corruption was
> reported when running gdb 6.4 on a PowerPC 440.  If a backtrace command
> was run after hitting a signal trampoline frame, the frame cache would
> become corrupted and GDB would SIGSEGV.
> 
> I have checked the cvs head sources, and the code has moved, but has not
> been fixed.  So I updated the patch to apply to cvs head.
> 
> Ok to commit?  (This is almost an obvious fix, isn't it?)

Not as you've posted it, since it doesn't adhere to the GNU coding
style.  But the idea seems alright to me.

Mark

Re: patch: signal trampoline frame cache corruption (repost?)

[PAUL GILLIAM]

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

On Wed, 2006-11-01 at 00:28 +0100, Mark Kettenis wrote:
> > . . .
> > Ok to commit?  (This is almost an obvious fix, isn't it?)
> 
> Not as you've posted it, since it doesn't adhere to the GNU coding
> style.  But the idea seems alright to me.
> 
> Mark

Ufda!

Sorry about the formatting.  I fixed that, and included a ChangeLog
entry which I also forgot the first time.

Now is it OK to commit?

-=# Paul #=-

[-- Attachment #2: fix-sigtramp-cache.patch --]
[-- Type: text/x-patch, Size: 1381 bytes --]


2006-11-03  Paul Gilliam <pgilliam@us.ibm.com>
	
	* ppc-linux-tdep.c (ppc_linux_sigtramp_cache):  Only
	deal with the floating point registers if the processor
	has a floating point unit.

Index: ppc-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/ppc-linux-tdep.c,v
retrieving revision 1.78
diff -a -u -r1.78 ppc-linux-tdep.c
--- ppc-linux-tdep.c	18 Apr 2006 19:20:06 -0000	1.78
+++ ppc-linux-tdep.c	3 Nov 2006 17:45:28 -0000
@@ -916,14 +916,18 @@
   trad_frame_set_reg_addr (this_cache, tdep->ppc_cr_regnum,
 			   gpregs + 38 * tdep->wordsize);
 
-  /* Floating point registers.  */
-  for (i = 0; i < 32; i++)
+  if (ppc_floating_point_unit_p(gdbarch))
     {
-      int regnum = i + FP0_REGNUM;
-      trad_frame_set_reg_addr (this_cache, regnum, fpregs + i * tdep->wordsize);
+      /* Floating point registers.  */
+      for (i = 0; i < 32; i++)
+        {
+          int regnum = i + FP0_REGNUM;
+          trad_frame_set_reg_addr (this_cache, regnum, 
+				   fpregs + i * tdep->wordsize);
+        }
+      trad_frame_set_reg_addr (this_cache, tdep->ppc_fpscr_regnum,
+                               fpregs + 32 * tdep->wordsize);
     }
-  trad_frame_set_reg_addr (this_cache, tdep->ppc_fpscr_regnum,
-			   fpregs + 32 * tdep->wordsize);
   trad_frame_set_id (this_cache, frame_id_build (base, func));
 }
 

Re: patch: signal trampoline frame cache corruption (repost?)

[Daniel Jacobowitz]

On Fri, Nov 03, 2006 at 09:50:36AM -0800, PAUL GILLIAM wrote:
> Sorry about the formatting.  I fixed that, and included a ChangeLog
> entry which I also forgot the first time.
> 
> Now is it OK to commit?

More formatting fixes:

> 2006-11-03  Paul Gilliam <pgilliam@us.ibm.com>

Two spaces before "<"...

> 	* ppc-linux-tdep.c (ppc_linux_sigtramp_cache):  Only

... but not after ":".

> +  if (ppc_floating_point_unit_p(gdbarch))

Always a space before open parentheses.

With those three fixed, feel free to repost and commit this.

-- 
Daniel Jacobowitz
CodeSourcery

[commit] signal trampoline frame cache corruption

[PAUL GILLIAM]

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

Thanks Mark and Daniel.

I have committed the attached patch

-=# Paul Gilliam #=-

On Fri, 2006-11-10 at 16:22 -0500, Daniel Jacobowitz wrote:
> On Fri, Nov 03, 2006 at 09:50:36AM -0800, PAUL GILLIAM wrote:
> > Sorry about the formatting.  I fixed that, and included a ChangeLog
> > entry which I also forgot the first time.
> > 
> > Now is it OK to commit?
> 
> More formatting fixes:
> 
> > 2006-11-03  Paul Gilliam <pgilliam@us.ibm.com>
> 
> Two spaces before "<"...
> 
> > 	* ppc-linux-tdep.c (ppc_linux_sigtramp_cache):  Only
> 
> ... but not after ":".
> 
> > +  if (ppc_floating_point_unit_p(gdbarch))
> 
> Always a space before open parentheses.
> 
> With those three fixed, feel free to repost and commit this.
> 

[-- Attachment #2: fix-sigtramp-cache.patch --]
[-- Type: text/x-patch, Size: 1388 bytes --]

2006-10-31  Paul Gilliam  <pgilliam@us.ibm.com>
        * ppc-linux-tdep.c (ppc_linux_sigtramp_cache): Don't futz with
        the floating point registers if there aren't any.

Index: ppc-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/ppc-linux-tdep.c,v
retrieving revision 1.78
diff -a -u -r1.78 ppc-linux-tdep.c
--- ppc-linux-tdep.c	18 Apr 2006 19:20:06 -0000	1.78
+++ ppc-linux-tdep.c	31 Oct 2006 19:30:12 -0000
@@ -916,14 +916,16 @@
   trad_frame_set_reg_addr (this_cache, tdep->ppc_cr_regnum,
 			   gpregs + 38 * tdep->wordsize);
 
-  /* Floating point registers.  */
-  for (i = 0; i < 32; i++)
-    {
-      int regnum = i + FP0_REGNUM;
-      trad_frame_set_reg_addr (this_cache, regnum, fpregs + i * tdep->wordsize);
-    }
-  trad_frame_set_reg_addr (this_cache, tdep->ppc_fpscr_regnum,
-			   fpregs + 32 * tdep->wordsize);
+  if (ppc_floating_point_unit_p (gdbarch)) {
+         /* Floating point registers.  */
+         for (i = 0; i < 32; i++)
+         {
+                 int regnum = i + FP0_REGNUM;
+                 trad_frame_set_reg_addr (this_cache, regnum, fpregs + i * tdep->wordsize);
+         }
+         trad_frame_set_reg_addr (this_cache, tdep->ppc_fpscr_regnum,
+                         fpregs + 32 * tdep->wordsize);
+  }
   trad_frame_set_id (this_cache, frame_id_build (base, func));
 }
 

Re: [commit] signal trampoline frame cache corruption

[Daniel Jacobowitz]

On Mon, Nov 13, 2006 at 09:35:27AM -0800, PAUL GILLIAM wrote:
> Thanks Mark and Daniel.
> 
> I have committed the attached patch

> +  if (ppc_floating_point_unit_p (gdbarch)) {
> +         /* Floating point registers.  */
> +         for (i = 0; i < 32; i++)
> +         {
> +                 int regnum = i + FP0_REGNUM;
> +                 trad_frame_set_reg_addr (this_cache, regnum, fpregs + i * tdep->wordsize);
> +         }
> +         trad_frame_set_reg_addr (this_cache, tdep->ppc_fpscr_regnum,
> +                         fpregs + 32 * tdep->wordsize);
> +  }

You had your indentation and braces right the last time you posted
this; what's happened to them?  The version you've committed looks
like it's been reindented mostly to the Linux kernel style.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [commit] signal trampoline frame cache corruption

[PAUL GILLIAM]

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

I don't know what happened.  I fixed the indentation problems introduced
by the patch.  
I  have attached the original patch it's self, with correct indentation.

-=# Paul #=-



[-- Attachment #2: fix-sigtramp-cache.patch --]
[-- Type: text/x-patch, Size: 1377 bytes --]

2006-10-31  Paul Gilliam  <pgilliam@us.ibm.com>
        * ppc-linux-tdep.c (ppc_linux_sigtramp_cache): Don't futz with
        the floating point registers if there aren't any.

Index: ppc-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/ppc-linux-tdep.c,v
retrieving revision 1.78
diff -a -u -r1.78 ppc-linux-tdep.c
--- ppc-linux-tdep.c	18 Apr 2006 19:20:06 -0000	1.78
+++ ppc-linux-tdep.c	31 Oct 2006 19:30:12 -0000
@@ -916,14 +916,16 @@
   trad_frame_set_reg_addr (this_cache, tdep->ppc_cr_regnum,
 			   gpregs + 38 * tdep->wordsize);
 
-  /* Floating point registers.  */
-  for (i = 0; i < 32; i++)
-    {
-      int regnum = i + FP0_REGNUM;
-      trad_frame_set_reg_addr (this_cache, regnum, fpregs + i * tdep->wordsize);
-    }
-  trad_frame_set_reg_addr (this_cache, tdep->ppc_fpscr_regnum,
-			   fpregs + 32 * tdep->wordsize);
+  if (ppc_floating_point_unit_p (gdbarch))
+    {
+      /* Floating point registers.  */
+      for (i = 0; i < 32; i++)
+	{
+	  int regnum = i + FP0_REGNUM;
+	  trad_frame_set_reg_addr (this_cache, regnum,
+	                           fpregs + i * tdep->wordsize);
+	}
+      trad_frame_set_reg_addr (this_cache, tdep->ppc_fpscr_regnum,
+                               fpregs + 32 * tdep->wordsize);
+    }
   trad_frame_set_id (this_cache, frame_id_build (base, func));
 }
 

Re: [commit] signal trampoline frame cache corruption

[Daniel Jacobowitz]

On Mon, Nov 13, 2006 at 10:50:34AM -0800, PAUL GILLIAM wrote:
> I don't know what happened.  I fixed the indentation problems introduced
> by the patch.  
> I  have attached the original patch it's self, with correct indentation.

Thanks!  Looks good this time.  I fixed the formatting and date of the
ChangeLog entry for you.


-- 
Daniel Jacobowitz
CodeSourcery

Re: [commit] signal trampoline frame cache corruption

[PAUL GILLIAM]

Thanks Daniel

On Mon, 2006-11-13 at 14:05 -0500, Daniel Jacobowitz wrote:
> On Mon, Nov 13, 2006 at 10:50:34AM -0800, PAUL GILLIAM wrote:
> > I don't know what happened.  I fixed the indentation problems introduced
> > by the patch.  
> > I  have attached the original patch it's self, with correct indentation.
> 
> Thanks!  Looks good this time.  I fixed the formatting and date of the
> ChangeLog entry for you.
> 
>