Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA/mips] Fix crash trying to print long double float
@ 2004-08-06 18:16 Joel Brobecker
  2004-08-06 20:24 ` Andrew Cagney
  2004-08-06 20:32 ` Joel Brobecker
  0 siblings, 2 replies; 12+ messages in thread
From: Joel Brobecker @ 2004-08-06 18:16 UTC (permalink / raw)
  To: gdb-patches

Restarting the discussion that started with:

        http://sources.redhat.com/ml/gdb-patches/2004-07/msg00290.html

And then continued in:

        http://sources.redhat.com/ml/gdb-patches/2004-07/msg00310.html

I noticed the following SEGV in our testsuite. Here is below how to
reproduce it using the store.c sources in testsuite/gdb.base:

        % gcc -c -g store.c
        % gcc -o store store.o

The following transcript shows how to cause the SEGV:

        (gdb) b wack_doublest
        Breakpoint 1 at 0x1000256c: file store.c, line 125.
        (gdb) run
        Starting program: /[...]/gdb.base/store

        Breakpoint 1, wack_doublest (u=Unhandled dwarf expression opcode 0x93
        ) at store.c:125
        125       register doublest l = u, r = v;
        (gdb) n
        126       l = add_doublest (l, r);
        (gdb) p l
        zsh: 6356790 segmentation fault (core dumped)  ../../gdb store

The problem is that GDB is currently "configured" via the gdbarch
mechanism to think that "long double" types are 64bits long, when
they are in fact 128 bits long.

When we try to print the value of "l", a 128bit long variable (the
size is deduced from the debugging info), GDB does a floatformat lookup
based on type size. At some point, GDB calls the following function with
a length of 16 bytes:

        static const struct floatformat *
        floatformat_from_length (int len)
        {
          if (len * TARGET_CHAR_BIT == TARGET_FLOAT_BIT)
            return TARGET_FLOAT_FORMAT;
          else if (len * TARGET_CHAR_BIT == TARGET_DOUBLE_BIT)
            return TARGET_DOUBLE_FORMAT;
          else if (len * TARGET_CHAR_BIT == TARGET_LONG_DOUBLE_BIT)
            return TARGET_LONG_DOUBLE_FORMAT;
          /* On i386 the 'long double' type takes 96 bits,
             while the real number of used bits is only 80,
             both in processor and in memory.  
             The code below accepts the real bit size.  */
          else if ((TARGET_LONG_DOUBLE_FORMAT != NULL)
                   && (len * TARGET_CHAR_BIT ==
                       TARGET_LONG_DOUBLE_FORMAT->totalsize))
            return TARGET_LONG_DOUBLE_FORMAT;
        
          return NULL;
        }

Because the gdbarch vector tells GDB that TARGET_LONG_DOUBLE_BIT = 64bit,
we end up returning a NULL floatformat. And unfortunately for us, we
immediatly use that NULL floatformat to feed it to floatformat_is_valid(),
which dereferences it without checking that it's not NULL before hand.
This causes the SEGV.  See values.c:unpack_double():

        if (!floatformat_is_valid (floatformat_from_type (type), valaddr))
          {
            *invp = 1;
            return 0.0;
          }

The first thing that needs to be fixed in the size of long doubles.
The attached patch fixes this, and prevents the crash from happening.
There is a discussion regarding the long double floatformat used, which
happens to work, but only by chance. This will be the subject of another
patch.

2004-08-06  Joel Brobecker  <brobecker@gnat.com>

        * mips-tdep.c (mips_gdbarch_init): Set size of long double
        to 128 bits for N32 and N64 ABIs.

Tested on mips-irix. OK to apply?

Thanks,
-- 
Joel


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

* Re: [RFA/mips] Fix crash trying to print long double float
  2004-08-06 18:16 [RFA/mips] Fix crash trying to print long double float Joel Brobecker
@ 2004-08-06 20:24 ` Andrew Cagney
  2004-08-06 20:38   ` Joel Brobecker
  2004-08-06 20:32 ` Joel Brobecker
  1 sibling, 1 reply; 12+ messages in thread
From: Andrew Cagney @ 2004-08-06 20:24 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

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

> Because the gdbarch vector tells GDB that TARGET_LONG_DOUBLE_BIT = 64bit,
> we end up returning a NULL floatformat. And unfortunately for us, we
> immediatly use that NULL floatformat to feed it to floatformat_is_valid(),
> which dereferences it without checking that it's not NULL before hand.
> This causes the SEGV.  See values.c:unpack_double():
> 
>         if (!floatformat_is_valid (floatformat_from_type (type), valaddr))
>           {
>             *invp = 1;
>             return 0.0;
>           }

Does the attached stop the crash?

Andrew


[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 3165 bytes --]

2004-08-06  Andrew Cagney  <cagney@gnu.org>

	* doublest.c: Update copyright.
	(floatformat_from_length): Call error when floatformat is NULL.
	(extract_floating_by_length): Remove NULL fmt check.
	(store_floating_by_length): Ditto.

Index: doublest.c
===================================================================
RCS file: /cvs/src/src/gdb/doublest.c,v
retrieving revision 1.18
diff -p -u -r1.18 doublest.c
--- doublest.c	29 Jul 2004 19:33:22 -0000	1.18
+++ doublest.c	6 Aug 2004 20:20:14 -0000
@@ -1,8 +1,8 @@
 /* Floating point routines for GDB, the GNU debugger.
 
    Copyright 1986, 1988, 1989, 1990, 1991, 1992, 1993, 1994, 1995,
-   1996, 1997, 1998, 1999, 2000, 2001, 2003 Free Software Foundation,
-   Inc.
+   1996, 1997, 1998, 1999, 2000, 2001, 2003, 2004 Free Software
+   Foundation, Inc.
 
    This file is part of GDB.
 
@@ -618,8 +618,8 @@ floatformat_from_doublest (const struct 
 
 \f
 /* Return a floating-point format for a floating-point variable of
-   length LEN.  Return NULL, if no suitable floating-point format
-   could be found.
+   length LEN.  If no suitable floating-point format is found, an
+   error is thrown.
 
    We need this functionality since information about the
    floating-point format of a type is not always available to GDB; the
@@ -633,12 +633,13 @@ floatformat_from_doublest (const struct 
 static const struct floatformat *
 floatformat_from_length (int len)
 {
+  const struct floatformat *format;
   if (len * TARGET_CHAR_BIT == TARGET_FLOAT_BIT)
-    return TARGET_FLOAT_FORMAT;
+    format = TARGET_FLOAT_FORMAT;
   else if (len * TARGET_CHAR_BIT == TARGET_DOUBLE_BIT)
-    return TARGET_DOUBLE_FORMAT;
+    format = TARGET_DOUBLE_FORMAT;
   else if (len * TARGET_CHAR_BIT == TARGET_LONG_DOUBLE_BIT)
-    return TARGET_LONG_DOUBLE_FORMAT;
+    format = TARGET_LONG_DOUBLE_FORMAT;
   /* On i386 the 'long double' type takes 96 bits,
      while the real number of used bits is only 80,
      both in processor and in memory.  
@@ -646,9 +647,13 @@ floatformat_from_length (int len)
   else if ((TARGET_LONG_DOUBLE_FORMAT != NULL) 
 	   && (len * TARGET_CHAR_BIT ==
                TARGET_LONG_DOUBLE_FORMAT->totalsize))
-    return TARGET_LONG_DOUBLE_FORMAT;
-
-  return NULL;
+    format = TARGET_LONG_DOUBLE_FORMAT;
+  else
+    format = NULL;
+  if (format == NULL)
+    error ("This GDB does not support %d-bit floating-point values.",
+	   len & TARGET_CHAR_BIT);
+  return format;
 }
 
 const struct floatformat *
@@ -675,12 +680,6 @@ extract_floating_by_length (const void *
   const struct floatformat *fmt = floatformat_from_length (len);
   DOUBLEST val;
 
-  if (fmt == NULL)
-    {
-      warning ("Can't extract a floating-point number of %d bytes.", len);
-      return NAN;
-    }
-
   floatformat_to_doublest (fmt, addr, &val);
   return val;
 }
@@ -699,13 +698,6 @@ store_floating_by_length (void *addr, in
 {
   const struct floatformat *fmt = floatformat_from_length (len);
 
-  if (fmt == NULL)
-    {
-      warning ("Can't store a floating-point number of %d bytes.", len);
-      memset (addr, 0, len);
-      return;
-    }
-
   floatformat_from_doublest (fmt, &val, addr);
 }
 

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

* Re: [RFA/mips] Fix crash trying to print long double float
  2004-08-06 18:16 [RFA/mips] Fix crash trying to print long double float Joel Brobecker
  2004-08-06 20:24 ` Andrew Cagney
@ 2004-08-06 20:32 ` Joel Brobecker
  2004-08-07 18:01   ` Andrew Cagney
  1 sibling, 1 reply; 12+ messages in thread
From: Joel Brobecker @ 2004-08-06 20:32 UTC (permalink / raw)
  To: gdb-patches

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

[One more time, with the patch, sigh...]

Restarting the discussion that started with:

        http://sources.redhat.com/ml/gdb-patches/2004-07/msg00290.html

And then continued in:

        http://sources.redhat.com/ml/gdb-patches/2004-07/msg00310.html

I noticed the following SEGV in our testsuite. Here is below how to
reproduce it using the store.c sources in testsuite/gdb.base:

        % gcc -c -g store.c
        % gcc -o store store.o

The following transcript shows how to cause the SEGV:

        (gdb) b wack_doublest
        Breakpoint 1 at 0x1000256c: file store.c, line 125.
        (gdb) run
        Starting program: /[...]/gdb.base/store

        Breakpoint 1, wack_doublest (u=Unhandled dwarf expression opcode 0x93
        ) at store.c:125
        125       register doublest l = u, r = v;
        (gdb) n
        126       l = add_doublest (l, r);
        (gdb) p l
        zsh: 6356790 segmentation fault (core dumped)  ../../gdb store

The problem is that GDB is currently "configured" via the gdbarch
mechanism to think that "long double" types are 64bits long, when
they are in fact 128 bits long.

When we try to print the value of "l", a 128bit long variable (the
size is deduced from the debugging info), GDB does a floatformat lookup
based on type size. At some point, GDB calls the following function with
a length of 16 bytes:

        static const struct floatformat *
        floatformat_from_length (int len)
        {
          if (len * TARGET_CHAR_BIT == TARGET_FLOAT_BIT)
            return TARGET_FLOAT_FORMAT;
          else if (len * TARGET_CHAR_BIT == TARGET_DOUBLE_BIT)
            return TARGET_DOUBLE_FORMAT;
          else if (len * TARGET_CHAR_BIT == TARGET_LONG_DOUBLE_BIT)
            return TARGET_LONG_DOUBLE_FORMAT;
          /* On i386 the 'long double' type takes 96 bits,
             while the real number of used bits is only 80,
             both in processor and in memory.  
             The code below accepts the real bit size.  */
          else if ((TARGET_LONG_DOUBLE_FORMAT != NULL)
                   && (len * TARGET_CHAR_BIT ==
                       TARGET_LONG_DOUBLE_FORMAT->totalsize))
            return TARGET_LONG_DOUBLE_FORMAT;
        
          return NULL;
        }

Because the gdbarch vector tells GDB that TARGET_LONG_DOUBLE_BIT = 64bit,
we end up returning a NULL floatformat. And unfortunately for us, we
immediatly use that NULL floatformat to feed it to floatformat_is_valid(),
which dereferences it without checking that it's not NULL before hand.
This causes the SEGV.  See values.c:unpack_double():

        if (!floatformat_is_valid (floatformat_from_type (type), valaddr))
          {
            *invp = 1;
            return 0.0;
          }

The first thing that needs to be fixed in the size of long doubles.
The attached patch fixes this, and prevents the crash from happening.
There is a discussion regarding the long double floatformat used, which
happens to work, but only by chance. This will be the subject of another
patch.

2004-08-06  Joel Brobecker  <brobecker@gnat.com>

        * mips-tdep.c (mips_gdbarch_init): Set size of long double
        to 128 bits for N32 and N64 ABIs.

Tested on mips-irix. OK to apply?

Thanks,
-- 
Joel

[-- Attachment #2: mips-tdep.c.diff --]
[-- Type: text/plain, Size: 981 bytes --]

Index: mips-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.301
diff -u -p -r1.301 mips-tdep.c
--- mips-tdep.c	10 Jul 2004 01:17:52 -0000	1.301
+++ mips-tdep.c	22 Jul 2004 15:37:12 -0000
@@ -5689,6 +5689,7 @@ mips_gdbarch_init (struct gdbarch_info i
       set_gdbarch_long_bit (gdbarch, 32);
       set_gdbarch_ptr_bit (gdbarch, 32);
       set_gdbarch_long_long_bit (gdbarch, 64);
+      set_gdbarch_long_double_bit (gdbarch, 128);
       break;
     case MIPS_ABI_N64:
       set_gdbarch_push_dummy_call (gdbarch, mips_n32n64_push_dummy_call);
@@ -5699,6 +5700,7 @@ mips_gdbarch_init (struct gdbarch_info i
       set_gdbarch_long_bit (gdbarch, 64);
       set_gdbarch_ptr_bit (gdbarch, 64);
       set_gdbarch_long_long_bit (gdbarch, 64);
+      set_gdbarch_long_double_bit (gdbarch, 128);
       break;
     default:
       internal_error (__FILE__, __LINE__, "unknown ABI in switch");

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

* Re: [RFA/mips] Fix crash trying to print long double float
  2004-08-06 20:24 ` Andrew Cagney
@ 2004-08-06 20:38   ` Joel Brobecker
  2004-08-06 21:03     ` Andrew Cagney
  0 siblings, 1 reply; 12+ messages in thread
From: Joel Brobecker @ 2004-08-06 20:38 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

> Does the attached stop the crash?

I will give it a go sometime this evening, but yes, I think it will
prevent the crash. It's a good general guard.

> 2004-08-06  Andrew Cagney  <cagney@gnu.org>
> 
> 	* doublest.c: Update copyright.
> 	(floatformat_from_length): Call error when floatformat is NULL.
> 	(extract_floating_by_length): Remove NULL fmt check.
> 	(store_floating_by_length): Ditto.

One question:

> @@ -633,12 +633,13 @@ floatformat_from_doublest (const struct 
>  static const struct floatformat *
>  floatformat_from_length (int len)
>  {
> +  const struct floatformat *format;
>    if (len * TARGET_CHAR_BIT == TARGET_FLOAT_BIT)
> -    return TARGET_FLOAT_FORMAT;
> +    format = TARGET_FLOAT_FORMAT;
>    else if (len * TARGET_CHAR_BIT == TARGET_DOUBLE_BIT)
> -    return TARGET_DOUBLE_FORMAT;
> +    format = TARGET_DOUBLE_FORMAT;
>    else if (len * TARGET_CHAR_BIT == TARGET_LONG_DOUBLE_BIT)
> -    return TARGET_LONG_DOUBLE_FORMAT;
> +    format = TARGET_LONG_DOUBLE_FORMAT;
>    /* On i386 the 'long double' type takes 96 bits,
>       while the real number of used bits is only 80,
>       both in processor and in memory.  
> @@ -646,9 +647,13 @@ floatformat_from_length (int len)
>    else if ((TARGET_LONG_DOUBLE_FORMAT != NULL) 
>  	   && (len * TARGET_CHAR_BIT ==
>                 TARGET_LONG_DOUBLE_FORMAT->totalsize))
> -    return TARGET_LONG_DOUBLE_FORMAT;
> -
> -  return NULL;
> +    format = TARGET_LONG_DOUBLE_FORMAT;
> +  else
> +    format = NULL;
> +  if (format == NULL)
> +    error ("This GDB does not support %d-bit floating-point values.",
> +	   len & TARGET_CHAR_BIT);
> +  return format;
>  }

Why do you use a variable? wouldn't have it been simpler to
add one line at the end like this:

        error ("bla bla bla");
        return NULL;  /* Will never be reached.  */

-- 
Joel


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

* Re: [RFA/mips] Fix crash trying to print long double float
  2004-08-06 20:38   ` Joel Brobecker
@ 2004-08-06 21:03     ` Andrew Cagney
  2004-08-07 17:18       ` Andrew Cagney
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Cagney @ 2004-08-06 21:03 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches


> One question:
> 
> 
>>> @@ -633,12 +633,13 @@ floatformat_from_doublest (const struct 
>>>  static const struct floatformat *
>>>  floatformat_from_length (int len)
>>>  {
>>> +  const struct floatformat *format;
>>>    if (len * TARGET_CHAR_BIT == TARGET_FLOAT_BIT)
>>> -    return TARGET_FLOAT_FORMAT;
>>> +    format = TARGET_FLOAT_FORMAT;
>>>    else if (len * TARGET_CHAR_BIT == TARGET_DOUBLE_BIT)
>>> -    return TARGET_DOUBLE_FORMAT;
>>> +    format = TARGET_DOUBLE_FORMAT;
>>>    else if (len * TARGET_CHAR_BIT == TARGET_LONG_DOUBLE_BIT)
>>> -    return TARGET_LONG_DOUBLE_FORMAT;
>>> +    format = TARGET_LONG_DOUBLE_FORMAT;
>>>    /* On i386 the 'long double' type takes 96 bits,
>>>       while the real number of used bits is only 80,
>>>       both in processor and in memory.  
>>> @@ -646,9 +647,13 @@ floatformat_from_length (int len)
>>>    else if ((TARGET_LONG_DOUBLE_FORMAT != NULL) 
>>>  	   && (len * TARGET_CHAR_BIT ==
>>>                 TARGET_LONG_DOUBLE_FORMAT->totalsize))
>>> -    return TARGET_LONG_DOUBLE_FORMAT;
>>> -
>>> -  return NULL;
>>> +    format = TARGET_LONG_DOUBLE_FORMAT;
>>> +  else
>>> +    format = NULL;
>>> +  if (format == NULL)
>>> +    error ("This GDB does not support %d-bit floating-point values.",
>>> +	   len & TARGET_CHAR_BIT);
>>> +  return format;
>>>  }
> 
> 
> Why do you use a variable? wouldn't have it been simpler to
> add one line at the end like this:
> 
>         error ("bla bla bla");
>         return NULL;  /* Will never be reached.  */

And:
	if (TARGET_FLOAT_FORMAT != NULL && len * TARGET_CHAR_BIT == 
TARGET_FLOAT_BIT)
	  return TARGET_FLOAT_FORMAT;
	...
	else
	  error ("blah, blah");
I don't trust *_FORMAT to be non-NULL.

Andrew



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

* Re: [RFA/mips] Fix crash trying to print long double float
  2004-08-06 21:03     ` Andrew Cagney
@ 2004-08-07 17:18       ` Andrew Cagney
  0 siblings, 0 replies; 12+ messages in thread
From: Andrew Cagney @ 2004-08-07 17:18 UTC (permalink / raw)
  To: Andrew Cagney, Joel Brobecker; +Cc: gdb-patches

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

>>>> +    format = NULL;
>>>> +  if (format == NULL)
>>>> +    error ("This GDB does not support %d-bit floating-point values.",
>>>> +       len & TARGET_CHAR_BIT);
>>>> +  return format;
>>>>  }
>>
>>
>>
>> Why do you use a variable? wouldn't have it been simpler to
>> add one line at the end like this:
>>
>>         error ("bla bla bla");
>>         return NULL;  /* Will never be reached.  */
> 
> 
> And:
>     if (TARGET_FLOAT_FORMAT != NULL && len * TARGET_CHAR_BIT == TARGET_FLOAT_BIT)
>       return TARGET_FLOAT_FORMAT;
>     ...
>     else
>       error ("blah, blah");
> I don't trust *_FORMAT to be non-NULL.

By deleting architecture methods of my PPC config I reproduced the crash 
(outch!).  I've committed the attached to both the mainline and branch 
(the error message was tweaked slightly).

Andrew


[-- Attachment #2: diffs --]
[-- Type: text/plain, Size: 3150 bytes --]

2004-08-07  Andrew Cagney  <cagney@gnu.org>

	* doublest.c: Update copyright.
	(floatformat_from_length): Call error when floatformat is NULL.
	(extract_floating_by_length): Remove NULL fmt check.
	(store_floating_by_length): Ditto.

Index: doublest.c
===================================================================
RCS file: /cvs/src/src/gdb/doublest.c,v
retrieving revision 1.18
diff -p -u -r1.18 doublest.c
--- doublest.c	29 Jul 2004 19:33:22 -0000	1.18
+++ doublest.c	7 Aug 2004 17:09:17 -0000
@@ -1,8 +1,8 @@
 /* Floating point routines for GDB, the GNU debugger.
 
    Copyright 1986, 1988, 1989, 1990, 1991, 1992, 1993, 1994, 1995,
-   1996, 1997, 1998, 1999, 2000, 2001, 2003 Free Software Foundation,
-   Inc.
+   1996, 1997, 1998, 1999, 2000, 2001, 2003, 2004 Free Software
+   Foundation, Inc.
 
    This file is part of GDB.
 
@@ -618,8 +618,8 @@ floatformat_from_doublest (const struct 
 
 \f
 /* Return a floating-point format for a floating-point variable of
-   length LEN.  Return NULL, if no suitable floating-point format
-   could be found.
+   length LEN.  If no suitable floating-point format is found, an
+   error is thrown.
 
    We need this functionality since information about the
    floating-point format of a type is not always available to GDB; the
@@ -633,12 +633,13 @@ floatformat_from_doublest (const struct 
 static const struct floatformat *
 floatformat_from_length (int len)
 {
+  const struct floatformat *format;
   if (len * TARGET_CHAR_BIT == TARGET_FLOAT_BIT)
-    return TARGET_FLOAT_FORMAT;
+    format = TARGET_FLOAT_FORMAT;
   else if (len * TARGET_CHAR_BIT == TARGET_DOUBLE_BIT)
-    return TARGET_DOUBLE_FORMAT;
+    format = TARGET_DOUBLE_FORMAT;
   else if (len * TARGET_CHAR_BIT == TARGET_LONG_DOUBLE_BIT)
-    return TARGET_LONG_DOUBLE_FORMAT;
+    format = TARGET_LONG_DOUBLE_FORMAT;
   /* On i386 the 'long double' type takes 96 bits,
      while the real number of used bits is only 80,
      both in processor and in memory.  
@@ -646,9 +647,13 @@ floatformat_from_length (int len)
   else if ((TARGET_LONG_DOUBLE_FORMAT != NULL) 
 	   && (len * TARGET_CHAR_BIT ==
                TARGET_LONG_DOUBLE_FORMAT->totalsize))
-    return TARGET_LONG_DOUBLE_FORMAT;
-
-  return NULL;
+    format = TARGET_LONG_DOUBLE_FORMAT;
+  else
+    format = NULL;
+  if (format == NULL)
+    error ("Unrecognized %d-bit floating-point type.",
+	   len & TARGET_CHAR_BIT);
+  return format;
 }
 
 const struct floatformat *
@@ -675,12 +680,6 @@ extract_floating_by_length (const void *
   const struct floatformat *fmt = floatformat_from_length (len);
   DOUBLEST val;
 
-  if (fmt == NULL)
-    {
-      warning ("Can't extract a floating-point number of %d bytes.", len);
-      return NAN;
-    }
-
   floatformat_to_doublest (fmt, addr, &val);
   return val;
 }
@@ -699,13 +698,6 @@ store_floating_by_length (void *addr, in
 {
   const struct floatformat *fmt = floatformat_from_length (len);
 
-  if (fmt == NULL)
-    {
-      warning ("Can't store a floating-point number of %d bytes.", len);
-      memset (addr, 0, len);
-      return;
-    }
-
   floatformat_from_doublest (fmt, &val, addr);
 }
 

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

* Re: [RFA/mips] Fix crash trying to print long double float
  2004-08-06 20:32 ` Joel Brobecker
@ 2004-08-07 18:01   ` Andrew Cagney
  2004-08-07 18:09     ` Joel Brobecker
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Cagney @ 2004-08-07 18:01 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

> 2004-08-06  Joel Brobecker  <brobecker@gnat.com>
> 
>         * mips-tdep.c (mips_gdbarch_init): Set size of long double
>         to 128 bits for N32 and N64 ABIs.
> 
> Tested on mips-irix. OK to apply?

Can you also clone / tweak the relevant floatformat (I'm wondering if it 
should just go in doublest.[hc] and we should stop using floatformat.[ch]).

Andrew



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

* Re: [RFA/mips] Fix crash trying to print long double float
  2004-08-07 18:01   ` Andrew Cagney
@ 2004-08-07 18:09     ` Joel Brobecker
  2004-08-07 18:22       ` Andrew Cagney
  0 siblings, 1 reply; 12+ messages in thread
From: Joel Brobecker @ 2004-08-07 18:09 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

> >2004-08-06  Joel Brobecker  <brobecker@gnat.com>
> >
> >        * mips-tdep.c (mips_gdbarch_init): Set size of long double
> >        to 128 bits for N32 and N64 ABIs.
> >
> >Tested on mips-irix. OK to apply?
> 
> Can you also clone / tweak the relevant floatformat (I'm wondering if it 
> should just go in doublest.[hc] and we should stop using floatformat.[ch]).

Yes. I am planning on doing this tonight. But could this done separately
from this patch, or do you think it doesn't make sense?

My other question is: Do we need to take care of the big endian
convention only, or do we need to look at the LE case as well (I don't
have a clue as to what is happening in that case :-(, if this ever makes
any sense).

Thanks,
-- 
Joel


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

* Re: [RFA/mips] Fix crash trying to print long double float
  2004-08-07 18:09     ` Joel Brobecker
@ 2004-08-07 18:22       ` Andrew Cagney
  2004-08-08  6:59         ` Joel Brobecker
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Cagney @ 2004-08-07 18:22 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

>>>2004-08-06  Joel Brobecker  <brobecker@gnat.com>
>>>> >
>>>> >        * mips-tdep.c (mips_gdbarch_init): Set size of long double
>>>> >        to 128 bits for N32 and N64 ABIs.
>>>> >
>>>> >Tested on mips-irix. OK to apply?
>>
>>> 
>>> Can you also clone / tweak the relevant floatformat (I'm wondering if it 
>>> should just go in doublest.[hc] and we should stop using floatformat.[ch]).
> 
> 
> Yes. I am planning on doing this tonight. But could this done separately
> from this patch, or do you think it doesn't make sense?

Same patch.

> My other question is: Do we need to take care of the big endian
> convention only, or do we need to look at the LE case as well (I don't
> have a clue as to what is happening in that case :-(, if this ever makes
> any sense).

Just take care of the BE case (set nothing for the LE case).  Now that 
we've stopped the fatal crash this isn't an issue.

Andrew



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

* Re: [RFA/mips] Fix crash trying to print long double float
  2004-08-07 18:22       ` Andrew Cagney
@ 2004-08-08  6:59         ` Joel Brobecker
  2004-08-08 16:57           ` Andrew Cagney
  0 siblings, 1 reply; 12+ messages in thread
From: Joel Brobecker @ 2004-08-08  6:59 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

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

How's this?

2004-08-08  Joel Brobecker  <brobecker@gnat.com>

        * mips-tdep.c (mips_gdbarch_init): Set size of long double
        to 128 bits for N32 and N64 ABIs.
        * mips-irix-tdep.c: Include "floatformat.h".
        (mips_irix_floatformat_always_valid): New function.
        (floatformat_mips_irix_long_double_big): New constant.
        (mips_irix_init_abi): Set the long double floatformat.

Tested on mips-irix. OK to apply?

Thanks,
-- 
Joel

[-- Attachment #2: ld.diff --]
[-- Type: text/plain, Size: 3709 bytes --]

Index: mips-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.309
diff -u -p -r1.309 mips-tdep.c
--- mips-tdep.c	3 Aug 2004 01:12:43 -0000	1.309
+++ mips-tdep.c	8 Aug 2004 06:51:01 -0000
@@ -6323,6 +6323,7 @@ mips_gdbarch_init (struct gdbarch_info i
       set_gdbarch_long_bit (gdbarch, 32);
       set_gdbarch_ptr_bit (gdbarch, 32);
       set_gdbarch_long_long_bit (gdbarch, 64);
+      set_gdbarch_long_double_bit (gdbarch, 128);
       break;
     case MIPS_ABI_N64:
       set_gdbarch_push_dummy_call (gdbarch, mips_n32n64_push_dummy_call);
@@ -6333,6 +6334,7 @@ mips_gdbarch_init (struct gdbarch_info i
       set_gdbarch_long_bit (gdbarch, 64);
       set_gdbarch_ptr_bit (gdbarch, 64);
       set_gdbarch_long_long_bit (gdbarch, 64);
+      set_gdbarch_long_double_bit (gdbarch, 128);
       break;
     default:
       internal_error (__FILE__, __LINE__, "unknown ABI in switch");
Index: mips-irix-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-irix-tdep.c,v
retrieving revision 1.2
diff -u -p -r1.2 mips-irix-tdep.c
--- mips-irix-tdep.c	21 Dec 2002 19:58:07 -0000	1.2
+++ mips-irix-tdep.c	8 Aug 2004 06:50:59 -0000
@@ -22,9 +22,36 @@
 
 #include "defs.h"
 #include "osabi.h"
+#include "floatformat.h"
 
 #include "elf-bfd.h"
 
+static int
+mips_irix_floatformat_always_valid (const struct floatformat *fmt,
+                                    const char *from)
+{
+  return 1;
+}
+
+/* FIXME: brobecker/2004-08-08: Long Double values are 128 bit long.
+   They are implemented as a pair of 64bit doubles where the high
+   part holds the result of the operation rounded to double, and
+   the low double holds the difference between the exact result and
+   the rounded result.  So "high" + "low" contains the result with
+   added precision.  Unfortunately, the floatformat structure used
+   by GDB is not powerful enough to describe this format.  As a temporary
+   measure, we define a 128bit floatformat that only uses the high part.
+   We lose a bit of precision but that's probably the best we can do
+   for now with the current infrastructure.  */
+
+static const struct floatformat floatformat_mips_irix_long_double_big =
+{
+  floatformat_big, 128, 0, 1, 11, 1023, 2047, 12, 52,
+  floatformat_intbit_no,
+  "floatformat_ieee_double_big",
+  mips_irix_floatformat_always_valid
+};
+
 static void
 mips_irix_elf_osabi_sniff_abi_tag_sections (bfd *abfd, asection *sect,
                                             void *obj)
@@ -80,6 +107,8 @@ static void
 mips_irix_init_abi (struct gdbarch_info info,
                     struct gdbarch *gdbarch)
 {
+  set_gdbarch_long_double_format (gdbarch,
+                                  &floatformat_mips_irix_long_double_big);
 }
 
 void
Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.606
diff -u -p -r1.606 Makefile.in
--- Makefile.in	6 Aug 2004 19:14:44 -0000	1.606
+++ Makefile.in	8 Aug 2004 06:50:19 -0000
@@ -2186,7 +2186,8 @@ mem-break.o: mem-break.c $(defs_h) $(sym
 	$(target_h)
 minsyms.o: minsyms.c $(defs_h) $(gdb_string_h) $(symtab_h) $(bfd_h) \
 	$(symfile_h) $(objfiles_h) $(demangle_h) $(value_h) $(cp_abi_h)
-mips-irix-tdep.o: mips-irix-tdep.c $(defs_h) $(osabi_h) $(elf_bfd_h)
+mips-irix-tdep.o: mips-irix-tdep.c $(defs_h) $(osabi_h) $(elf_bfd_h) \
+        $(floatformat_h)
 mips-linux-nat.o: mips-linux-nat.c $(defs_h) $(mips_tdep_h)
 mips-linux-tdep.o: mips-linux-tdep.c $(defs_h) $(gdbcore_h) $(target_h) \
 	$(solib_svr4_h) $(osabi_h) $(mips_tdep_h) $(gdb_string_h) \

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

* Re: [RFA/mips] Fix crash trying to print long double float
  2004-08-08  6:59         ` Joel Brobecker
@ 2004-08-08 16:57           ` Andrew Cagney
  2004-08-09  5:46             ` Joel Brobecker
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Cagney @ 2004-08-08 16:57 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

> How's this?
> 
> 2004-08-08  Joel Brobecker  <brobecker@gnat.com>
> 
>         * mips-tdep.c (mips_gdbarch_init): Set size of long double
>         to 128 bits for N32 and N64 ABIs.
>         * mips-irix-tdep.c: Include "floatformat.h".
>         (mips_irix_floatformat_always_valid): New function.
>         (floatformat_mips_irix_long_double_big): New constant.
>         (mips_irix_init_abi): Set the long double floatformat.
> 
> Tested on mips-irix. OK to apply?

Yes, with these tweaks, commit:

>        set_gdbarch_long_long_bit (gdbarch, 64);
> +      set_gdbarch_long_double_bit (gdbarch, 128);

-- Set the long_double_format here (dito for other place), perhaphs 
conditional on info.byte_order == BFD_ENDIAN_BIG.

The ..._bit and ..._format methods should really be combined into a 
single object.  I think that should wait until GDB is written in an OO 
lanugage.  However, in the mean time we can group them together.

>        break;

-- Move this to mips-tdep and, after some thought, call it n32n64.

> +static int
> +mips_irix_floatformat_always_valid (const struct floatformat *fmt,
> +                                    const char *from)
> +{
> +  return 1;
> +}
> +
> +/* FIXME: brobecker/2004-08-08: Long Double values are 128 bit long.
> +   They are implemented as a pair of 64bit doubles where the high
> +   part holds the result of the operation rounded to double, and
> +   the low double holds the difference between the exact result and
> +   the rounded result.  So "high" + "low" contains the result with
> +   added precision.  Unfortunately, the floatformat structure used
> +   by GDB is not powerful enough to describe this format.  As a temporary
> +   measure, we define a 128bit floatformat that only uses the high part.
> +   We lose a bit of precision but that's probably the best we can do
> +   for now with the current infrastructure.  */
> +
> +static const struct floatformat floatformat_mips_irix_long_double_big =
> +{
> +  floatformat_big, 128, 0, 1, 11, 1023, 2047, 12, 52,
> +  floatformat_intbit_no,
> +  "floatformat_ieee_double_big",
> +  mips_irix_floatformat_always_valid
> +};

IRIX effectively defines the n32/n64 ABIs.  If the other systems differ, 
they can override both the _bit and _format in their local -tdep file.

Andrew



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

* Re: [RFA/mips] Fix crash trying to print long double float
  2004-08-08 16:57           ` Andrew Cagney
@ 2004-08-09  5:46             ` Joel Brobecker
  0 siblings, 0 replies; 12+ messages in thread
From: Joel Brobecker @ 2004-08-09  5:46 UTC (permalink / raw)
  To: Andrew Cagney; +Cc: gdb-patches

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

> >Tested on mips-irix. OK to apply?
> 
> Yes, with these tweaks, commit:

Thanks. Here is what I just checked in:

2004-08-08  Joel Brobecker  <brobecker@gnat.com>

        * mips-tdep.c: Include "floatformat.h".
        (n32n64_floatformat_always_valid): New function.
        (floatformat_n32n64_long_double_big): New constant.
        (mips_gdbarch_init): Set size and format of long double
        for N32 and N64 ABIs.
        * Makefile.in (mips-tdep.o): Update dependencies.

-- 
Joel

[-- Attachment #2: ld.diff --]
[-- Type: text/plain, Size: 3471 bytes --]

Index: mips-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/mips-tdep.c,v
retrieving revision 1.309
diff -u -p -r1.309 mips-tdep.c
--- mips-tdep.c	3 Aug 2004 01:12:43 -0000	1.309
+++ mips-tdep.c	8 Aug 2004 17:26:15 -0000
@@ -54,6 +54,7 @@
 #include "frame-base.h"
 #include "trad-frame.h"
 #include "infcall.h"
+#include "floatformat.h"
 
 static const struct objfile_data *mips_pdr_data;
 
@@ -149,6 +150,32 @@ struct gdbarch_tdep
   const char **mips_processor_reg_names;
 };
 
+static int
+n32n64_floatformat_always_valid (const struct floatformat *fmt,
+                                 const char *from)
+{
+  return 1;
+}
+
+/* FIXME: brobecker/2004-08-08: Long Double values are 128 bit long.
+   They are implemented as a pair of 64bit doubles where the high
+   part holds the result of the operation rounded to double, and
+   the low double holds the difference between the exact result and
+   the rounded result.  So "high" + "low" contains the result with
+   added precision.  Unfortunately, the floatformat structure used
+   by GDB is not powerful enough to describe this format.  As a temporary
+   measure, we define a 128bit floatformat that only uses the high part.
+   We lose a bit of precision but that's probably the best we can do
+   for now with the current infrastructure.  */
+
+static const struct floatformat floatformat_n32n64_long_double_big =
+{
+  floatformat_big, 128, 0, 1, 11, 1023, 2047, 12, 52,
+  floatformat_intbit_no,
+  "floatformat_ieee_double_big",
+  n32n64_floatformat_always_valid
+};
+
 const struct mips_regnum *
 mips_regnum (struct gdbarch *gdbarch)
 {
@@ -6323,6 +6350,9 @@ mips_gdbarch_init (struct gdbarch_info i
       set_gdbarch_long_bit (gdbarch, 32);
       set_gdbarch_ptr_bit (gdbarch, 32);
       set_gdbarch_long_long_bit (gdbarch, 64);
+      set_gdbarch_long_double_bit (gdbarch, 128);
+      set_gdbarch_long_double_format (gdbarch,
+                                      &floatformat_n32n64_long_double_big);
       break;
     case MIPS_ABI_N64:
       set_gdbarch_push_dummy_call (gdbarch, mips_n32n64_push_dummy_call);
@@ -6333,6 +6363,9 @@ mips_gdbarch_init (struct gdbarch_info i
       set_gdbarch_long_bit (gdbarch, 64);
       set_gdbarch_ptr_bit (gdbarch, 64);
       set_gdbarch_long_long_bit (gdbarch, 64);
+      set_gdbarch_long_double_bit (gdbarch, 128);
+      set_gdbarch_long_double_format (gdbarch,
+                                      &floatformat_n32n64_long_double_big);
       break;
     default:
       internal_error (__FILE__, __LINE__, "unknown ABI in switch");
Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.606
diff -u -p -r1.606 Makefile.in
--- Makefile.in	6 Aug 2004 19:14:44 -0000	1.606
+++ Makefile.in	8 Aug 2004 17:26:16 -0000
@@ -2206,7 +2206,8 @@ mips-tdep.o: mips-tdep.c $(defs_h) $(gdb
 	$(target_h) $(arch_utils_h) $(regcache_h) $(osabi_h) $(mips_tdep_h) \
 	$(block_h) $(reggroups_h) $(opcode_mips_h) $(elf_mips_h) \
 	$(elf_bfd_h) $(symcat_h) $(sim_regno_h) $(dis_asm_h) \
-	$(frame_unwind_h) $(frame_base_h) $(trad_frame_h) $(infcall_h)
+	$(frame_unwind_h) $(frame_base_h) $(trad_frame_h) $(infcall_h) \
+	$(floatformat_h)
 mipsv4-nat.o: mipsv4-nat.c $(defs_h) $(inferior_h) $(gdbcore_h) $(target_h) \
 	$(regcache_h) $(gregset_h)
 mn10300-tdep.o: mn10300-tdep.c $(defs_h) $(frame_h) $(inferior_h) \

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

end of thread, other threads:[~2004-08-09  5:46 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-08-06 18:16 [RFA/mips] Fix crash trying to print long double float Joel Brobecker
2004-08-06 20:24 ` Andrew Cagney
2004-08-06 20:38   ` Joel Brobecker
2004-08-06 21:03     ` Andrew Cagney
2004-08-07 17:18       ` Andrew Cagney
2004-08-06 20:32 ` Joel Brobecker
2004-08-07 18:01   ` Andrew Cagney
2004-08-07 18:09     ` Joel Brobecker
2004-08-07 18:22       ` Andrew Cagney
2004-08-08  6:59         ` Joel Brobecker
2004-08-08 16:57           ` Andrew Cagney
2004-08-09  5:46             ` Joel Brobecker

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