[PATCH RFA] Fix x86 floating point vs. thread problem

[PATCH RFA] Fix x86 floating point vs. thread problem

[Kevin Buettner]

The patch below fixes the problem reported by David Relson in

    http://sources.redhat.com/ml/gdb/2001-12/msg00001.html

An impressive test matrix regarding this bug has been provided by
Emmanuel Blindauer at

    http://manu.agat.net/bug.html

Anyway, the problem is that GDB is computing the fpxregs version of
the tag value incorrectly.  The fpxregs version of the tag value is
simply a bitmask (of eight bits) which indicate which of the floating
point registers is in use.  i387_fill_fxsave() was incorrectly
shifting by twice the the number of bits that it should have.

	* i387-nat.c (i387_fill_fxsave): Change type of ``val'' from char
	to short so that we don't memcpy() beyond the end of this buffer.
	Also, change shift value used in computing val to account for the
	fact that only eight bits are used.

Index: i387-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/i387-nat.c,v
retrieving revision 1.7
diff -u -p -r1.7 i387-nat.c
--- i387-nat.c	2001/11/12 22:27:35	1.7
+++ i387-nat.c	2001/12/06 23:19:05
@@ -270,7 +270,7 @@ i387_fill_fxsave (char *fxsave, int regn
 	      {
 		/* Converting back is much easier.  */
 
-		unsigned char val = 0;
+		unsigned short val = 0;
 		unsigned short ftag;
 		int fpreg;
 
@@ -281,7 +281,7 @@ i387_fill_fxsave (char *fxsave, int regn
 		    int tag = (ftag >> (fpreg * 2)) & 3;
 
 		    if (tag != 3)
-		      val |= (1 << (fpreg * 2));
+		      val |= (1 << fpreg);
 		  }
 
 		memcpy (FXSAVE_ADDR (fxsave, i), &val, 2);

Re: [PATCH RFA] Fix x86 floating point vs. thread problem

[Daniel Jacobowitz]

On Thu, Dec 06, 2001 at 04:48:07PM -0700, Kevin Buettner wrote:
> The patch below fixes the problem reported by David Relson in
> 
>     http://sources.redhat.com/ml/gdb/2001-12/msg00001.html
> 
> An impressive test matrix regarding this bug has been provided by
> Emmanuel Blindauer at
> 
>     http://manu.agat.net/bug.html
> 
> Anyway, the problem is that GDB is computing the fpxregs version of
> the tag value incorrectly.  The fpxregs version of the tag value is
> simply a bitmask (of eight bits) which indicate which of the floating
> point registers is in use.  i387_fill_fxsave() was incorrectly
> shifting by twice the the number of bits that it should have.
> 
> 	* i387-nat.c (i387_fill_fxsave): Change type of ``val'' from char
> 	to short so that we don't memcpy() beyond the end of this buffer.
> 	Also, change shift value used in computing val to account for the
> 	fact that only eight bits are used.

Out of curiousity, can you explain what I saw when looking at this?  I
found that we never set any fp-related register, and yet when the value
of d was written to memory it was incorrect.  Was it not actually yet
written to memory, or was I just mistaken?

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [PATCH RFA] Fix x86 floating point vs. thread problem

[Kevin Buettner]

On Dec 6,  7:01pm, Daniel Jacobowitz wrote:

> On Thu, Dec 06, 2001 at 04:48:07PM -0700, Kevin Buettner wrote:
> > The patch below fixes the problem reported by David Relson in
> > 
> >     http://sources.redhat.com/ml/gdb/2001-12/msg00001.html
> > 
> > An impressive test matrix regarding this bug has been provided by
> > Emmanuel Blindauer at
> > 
> >     http://manu.agat.net/bug.html
> > 
> > Anyway, the problem is that GDB is computing the fpxregs version of
> > the tag value incorrectly.  The fpxregs version of the tag value is
> > simply a bitmask (of eight bits) which indicate which of the floating
> > point registers is in use.  i387_fill_fxsave() was incorrectly
> > shifting by twice the the number of bits that it should have.
> > 
> > 	* i387-nat.c (i387_fill_fxsave): Change type of ``val'' from char
> > 	to short so that we don't memcpy() beyond the end of this buffer.
> > 	Also, change shift value used in computing val to account for the
> > 	fact that only eight bits are used.
> 
> Out of curiousity, can you explain what I saw when looking at this?  I
> found that we never set any fp-related register, and yet when the value
> of d was written to memory it was incorrect.  Was it not actually yet
> written to memory, or was I just mistaken?

I was puzzled by that too for a while.  I used the version of the
test program at Emmanuel Blindauer's page:

1       #include <stdlib.h>
2
3       int main() {
4         char *t="1.0";
5         double d=0;
6         d=strtod(t,(char **)NULL);
7         return(0);
8       }

Line 6 is comprised of the following instructions:

0x8048493 <main+35>:    call   0x804835c <strtod>
0x8048498 <main+40>:    add    $0x10,%esp
0x804849b <main+43>:    fstpl  0xfffffff0(%ebp)

Umm, I guess I'm missing the argument setup, but that's good enough. 
If you put a break on *main+40, you'll see that a floating point
register is in use when you've hit this breakpoint.

Anyway... the bug as reported was to put a breakpoint on line 6,
and then do a ``next''.  IIRC, the ``next'' operation actually
singlesteps into the call, places a breakpoint on the call exit,
and then singlesteps the instructions at main+40 and main+43.  In
the course of doing this, GDB fetches and stores the registers
many, many times.  (Too many times, IMO.)  As far as I can tell,
it's when GDB hits the step_resume breakpoint at main+40 that the
corruption occurs.

Kevin

Re: [PATCH RFA] Fix x86 floating point vs. thread problem

[Mark Kettenis]

Kevin Buettner <kevinb@redhat.com> writes:

> 	* i387-nat.c (i387_fill_fxsave): Change type of ``val'' from char
> 	to short so that we don't memcpy() beyond the end of this buffer.
> 	Also, change shift value used in computing val to account for the
> 	fact that only eight bits are used.

Looks good.  I'm not surprised there was a bug lurking here.  Please
check this in.  Thanks!

Mark

Re: [PATCH RFA] Fix x86 floating point vs. thread problem

[Kevin Buettner]

On Dec 12,  9:22pm, Mark Kettenis wrote:

> Kevin Buettner <kevinb@redhat.com> writes:
> 
> > 	* i387-nat.c (i387_fill_fxsave): Change type of ``val'' from char
> > 	to short so that we don't memcpy() beyond the end of this buffer.
> > 	Also, change shift value used in computing val to account for the
> > 	fact that only eight bits are used.
> 
> Looks good.  I'm not surprised there was a bug lurking here.  Please
> check this in.  Thanks!

Committed.

I think this one should go on the 5.1 branch too.  Any objections?

Kevin

Re: [PATCH RFA] Fix x86 floating point vs. thread problem

[Mark Kettenis]

   Date: Thu, 13 Dec 2001 10:37:04 -0700
   From: Kevin Buettner <kevinb@redhat.com>

   I think this one should go on the 5.1 branch too.  Any objections?

Not from me :-).

Mark

Re: [PATCH RFA] Fix x86 floating point vs. thread problem

[Kevin Buettner]

On Dec 14, 12:44am, Mark Kettenis wrote:

>    Date: Thu, 13 Dec 2001 10:37:04 -0700
>    From: Kevin Buettner <kevinb@redhat.com>
> 
>    I think this one should go on the 5.1 branch too.  Any objections?
> 
> Not from me :-).

Since there were no objections, I've committed this patch to the
5.1 branch also.

Kevin