Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH] Fix returning floating points values for x86
@ 2001-07-11  1:49 Mark Kettenis
  2001-07-11  9:13 ` Eli Zaretskii
  0 siblings, 1 reply; 4+ messages in thread
From: Mark Kettenis @ 2001-07-11  1:49 UTC (permalink / raw)
  To: gdb-patches; +Cc: grossman

Fixes two FAILS in gdb.base/return2.exp.

Stu, I was about to commit this patch when I saw you FreeBSD patch.
Patch is almost identical (modulo comments), so it must be right :-).
I nevertheless took all the credit ;-).  I will come back to you on
some of the other stuff in your patch.

Checked in.

Mark


Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	* i386-tdep.c (i386_extract_return_value): "Fix" comment.
	(i386_store_return_value): Frob FPU status and tag word to make
	sure the return value is the only value on the FPU stack.

Index: i386-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-tdep.c,v
retrieving revision 1.31
diff -u -p -r1.31 i386-tdep.c
--- i386-tdep.c 2001/05/09 16:16:33 1.31
+++ i386-tdep.c 2001/07/11 08:38:27
@@ -803,7 +803,8 @@ i386_extract_return_value (struct type *
 	  return;
 	}
 
-      /* Floating-point return values can be found in %st(0).  */
+      /* Floating-point return values can be found in %st(0).
+         FIXME: Does %st(0) always correspond to FP0?  */
       if (len == TARGET_LONG_DOUBLE_BIT / TARGET_CHAR_BIT
 	  && TARGET_LONG_DOUBLE_FORMAT == &floatformat_i387_ext)
 	{
@@ -861,6 +862,8 @@ i386_store_return_value (struct type *ty
 
   if (TYPE_CODE (type) == TYPE_CODE_FLT)
     {
+      unsigned int fstat;
+
       if (NUM_FREGS == 0)
 	{
 	  warning ("Cannot set floating-point return value.");
@@ -889,6 +892,16 @@ i386_store_return_value (struct type *ty
 	  write_register_bytes (REGISTER_BYTE (FP0_REGNUM), buf,
 				FPU_REG_RAW_SIZE);
 	}
+
+      /* Set the top of the floating point register stack to 7.  That
+         makes sure that FP0 (which we set above) is indeed %st(0).
+         FIXME: Perhaps we should completely reset the status word?  */
+      fstat = read_register (FSTAT_REGNUM);
+      fstat |= (7 << 11);
+      write_register (FSTAT_REGNUM, fstat);
+
+      /* Mark %st(1) through %st(7) as empty.  */
+      write_register (FTAG_REGNUM, 0x3fff);
     }
   else
     {


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

* Re: [PATCH] Fix returning floating points values for x86
  2001-07-11  1:49 [PATCH] Fix returning floating points values for x86 Mark Kettenis
@ 2001-07-11  9:13 ` Eli Zaretskii
  2001-07-11 13:28   ` Mark Kettenis
  0 siblings, 1 reply; 4+ messages in thread
From: Eli Zaretskii @ 2001-07-11  9:13 UTC (permalink / raw)
  To: kettenis; +Cc: gdb-patches, grossman

> Date: Wed, 11 Jul 2001 10:49:16 +0200
> From: Mark Kettenis <kettenis@wins.uva.nl>
> 
> 	* i386-tdep.c (i386_extract_return_value): "Fix" comment.
> 	(i386_store_return_value): Frob FPU status and tag word to make
> 	sure the return value is the only value on the FPU stack.

Mark, could you please say a few words about the problem that this
solves, and why is this the right solution?  I admit I don't
understand it, and the comment about st(0) scared me a bit: it's
certainly _not_ true, in general, that st(0) is always FP0_REGNUM.

TIA


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

* Re: [PATCH] Fix returning floating points values for x86
  2001-07-11  9:13 ` Eli Zaretskii
@ 2001-07-11 13:28   ` Mark Kettenis
  2001-07-12  0:00     ` Eli Zaretskii
  0 siblings, 1 reply; 4+ messages in thread
From: Mark Kettenis @ 2001-07-11 13:28 UTC (permalink / raw)
  To: eliz; +Cc: gdb-patches

   Date: Wed, 11 Jul 2001 19:12:58 +0300
   From: "Eli Zaretskii" <eliz@is.elta.co.il>

   > Date: Wed, 11 Jul 2001 10:49:16 +0200
   > From: Mark Kettenis <kettenis@wins.uva.nl>
   > 
   > 	* i386-tdep.c (i386_extract_return_value): "Fix" comment.
   > 	(i386_store_return_value): Frob FPU status and tag word to make
   > 	sure the return value is the only value on the FPU stack.

   Mark, could you please say a few words about the problem that this
   solves, and why is this the right solution?  I admit I don't
   understand it, and the comment about st(0) scared me a bit: it's
   certainly _not_ true, in general, that st(0) is always FP0_REGNUM.

With pleasure.  The problem the patch solves is returning from a
function with the GDB `return' command, or more specifically returning
from a function that has a floating point return value.

The System V ABI specifies that floating point return values appear on
the top of the floating point register stack, i.e. in %st(0).
Furthermore is specifies that %st(0) muts be empty before entry to a
function, and that %st(1) through %st(0) (which are called
floating-point scratch registers in the System V ABI) must be empty
before entry and upon exit from a function.

If we suppose that the FPU starts out in a freshly initialized state,
with all registers empty and TOP set to 0, this means that we'll
always end up with storing return values in the hardware register 7,
i.e. with TOP set to 7.  So it makes sense to reset the FPU to that
state in i386_store_return_value.  What value we choose for TOP is in
principle irrelevant since FP0_REGNO always refers to %st(0).  But we
must mark the right register as valid (and all others as empty) in the
tag word, and here the value of TOP does matter.  If we don't mark the
right register as valid, the caller of the function won't be able to
pop the return value from the stack, and if we don't mark the other
registers as empty we might trigger an unwanted stack overflow.

I was obviously a bit confused about %st(0) being FP0_REGNO or not.
The comment that worries you was the result of that.  I will fix the
comments.

Mark


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

* Re: [PATCH] Fix returning floating points values for x86
  2001-07-11 13:28   ` Mark Kettenis
@ 2001-07-12  0:00     ` Eli Zaretskii
  0 siblings, 0 replies; 4+ messages in thread
From: Eli Zaretskii @ 2001-07-12  0:00 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: gdb-patches

On Wed, 11 Jul 2001, Mark Kettenis wrote:

> The problem the patch solves is returning from a
> function with the GDB `return' command, or more specifically returning
> from a function that has a floating point return value.

You mean, the "return EXPRESSION" command, where EXPRESSION is an FP
expression, right?

> If we suppose that the FPU starts out in a freshly initialized state,
> with all registers empty and TOP set to 0, this means that we'll
> always end up with storing return values in the hardware register 7,
> i.e. with TOP set to 7.  So it makes sense to reset the FPU to that
> state in i386_store_return_value.  What value we choose for TOP is in
> principle irrelevant since FP0_REGNO always refers to %st(0).  But we
> must mark the right register as valid (and all others as empty) in the
> tag word, and here the value of TOP does matter.  If we don't mark the
> right register as valid, the caller of the function won't be able to
> pop the return value from the stack, and if we don't mark the other
> registers as empty we might trigger an unwanted stack overflow.

Okay, I think I understand now: you are simulating the state of the
FPU at function return point.  Perhaps this should be added to the
comments.

Thanks.


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

end of thread, other threads:[~2001-07-12  0:00 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2001-07-11  1:49 [PATCH] Fix returning floating points values for x86 Mark Kettenis
2001-07-11  9:13 ` Eli Zaretskii
2001-07-11 13:28   ` Mark Kettenis
2001-07-12  0:00     ` Eli Zaretskii

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