[libiberty and gdb] floatformat_is_valid

[libiberty and gdb] floatformat_is_valid

[Daniel Jacobowitz]

GDB's "struct value" has a union in it.  This union, named "aligner", has a
long double member as well as several others; its goal is to be a cheap way
to ensure that we can load all sorts of values straight from the value
structure, without alignment problems.  Unfortunately, this means that when
debugging GDB we tend to feed random bytes to printf ("%.35Lg").  That's not
kosher, and on i386 I've found a whole bunch of numbers which can crash it -
the gist is that GMP shifts normalized numbers over until it hits a one bit,
and if the explicit integer bit isn't set it walks right out of the number.

This could show up in other places too.  Anywhere you print floating-point
data from the inferior, really.

So here's a patch which checks for the problem.  I added
floatformat_is_valid to libiberty/floatformat.c.  Is this OK?
[Do I need approval?  Floatformat.c is in libiberty, but marked as part of
GDB.]

Oh, this patch misses some copyright year updates.  I'll get them before
checking in.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2003-09-15  Daniel Jacobowitz  <drow@mvista.com>

	* floatformat.h (floatformat_is_valid): Add prototype.

2003-09-15  Daniel Jacobowitz  <drow@mvista.com>

	* floatformat.c (floatformat_is_valid): New function.
	(get_field, put_field): Correct comments.

2003-09-15  Daniel Jacobowitz  <drow@mvista.com>

	* values.c (unpack_double): Call floatformat_is_valid.

Index: gdb/values.c
===================================================================
RCS file: /cvs/src/src/gdb/values.c,v
retrieving revision 1.54
diff -u -p -r1.54 values.c
--- gdb/values.c	14 Sep 2003 16:32:14 -0000	1.54
+++ gdb/values.c	15 Sep 2003 14:30:58 -0000
@@ -739,7 +739,14 @@ unpack_double (struct type *type, const 
 	 also not defined either.  Oops!
 
          Hopefully someone will add both the missing floatformat
-         definitions and floatformat_is_invalid() function.  */
+         definitions and the new cases for floatformat_is_valid ().  */
+
+      if (!floatformat_is_valid (floatformat_from_type (type), valaddr))
+	{
+	  *invp = 1;
+	  return 0.0;
+	}
+
       return extract_typed_floating (valaddr, type);
     }
   else if (nosign)
Index: libiberty/floatformat.c
===================================================================
RCS file: /cvs/src/src/libiberty/floatformat.c,v
retrieving revision 1.7
diff -u -p -r1.7 floatformat.c
--- libiberty/floatformat.c	15 Apr 2003 21:29:34 -0000	1.7
+++ libiberty/floatformat.c	15 Sep 2003 14:30:58 -0000
@@ -149,7 +149,7 @@ static unsigned long get_field PARAMS ((
 					unsigned int,
 					unsigned int));
 
-/* Extract a field which starts at START and is LEN bytes long.  DATA and
+/* Extract a field which starts at START and is LEN bits long.  DATA and
    TOTAL_LEN are the thing we are extracting it from, in byteorder ORDER.  */
 static unsigned long
 get_field (data, order, total_len, start, len)
@@ -273,7 +273,7 @@ static void put_field PARAMS ((unsigned 
 			       unsigned int,
 			       unsigned long));
 
-/* Set a field which starts at START and is LEN bytes long.  DATA and
+/* Set a field which starts at START and is LEN bits long.  DATA and
    TOTAL_LEN are the thing we are extracting it from, in byteorder ORDER.  */
 static void
 put_field (data, order, total_len, start, len, stuff_to_put)
@@ -402,6 +402,39 @@ floatformat_from_double (fmt, from, to)
       mant_off += mant_bits;
       mant_bits_left -= mant_bits;
     }
+}
+
+/* Return non-zero iff the data at FROM is a valid number in format FMT.  */
+
+int
+floatformat_is_valid (fmt, from)
+     const struct floatformat *fmt;
+     char *from;
+{
+  if (fmt == &floatformat_i387_ext)
+    {
+      /* In the i387 double-extended format, if the exponent is all
+	 ones, then the integer bit must be set.  If the exponent
+	 is neither 0 nor ~0, the intbit must also be set.  Only
+	 if the exponent is zero can it be zero, and then it must
+	 be zero.  */
+      unsigned int exponent, int_bit;
+      unsigned char *ufrom = (unsigned char *) from;
+
+      exponent = get_field (ufrom, fmt->byteorder, fmt->totalsize,
+			    fmt->exp_start, fmt->exp_len);
+      int_bit = get_field (ufrom, fmt->byteorder, fmt->totalsize,
+			   fmt->man_start, 1);
+
+      if ((exponent == 0) != (int_bit == 0))
+	return 0;
+      else
+	return 1;
+    }
+
+  /* Other formats with invalid representations should be added
+     here.  */
+  return 1;
 }
 
 
Index: include/floatformat.h
===================================================================
RCS file: /cvs/src/src/include/floatformat.h,v
retrieving revision 1.5
diff -u -p -r1.5 floatformat.h
--- include/floatformat.h	6 Mar 2002 06:29:41 -0000	1.5
+++ include/floatformat.h	15 Sep 2003 14:30:58 -0000
@@ -118,4 +118,9 @@ extern void
 floatformat_from_double PARAMS ((const struct floatformat *,
 				 double *, char *));
 
+/* Return non-zero iff the data at FROM is a valid number in format FMT.  */
+
+extern int
+floatformat_is_valid PARAMS ((const struct floatformat *fmt, char *from));
+
 #endif	/* defined (FLOATFORMAT_H) */

Re: [libiberty and gdb] floatformat_is_valid

[DJ Delorie]

>  static unsigned long

> +      unsigned int exponent, int_bit;

These should match (long != int on some platforms), otherwise OK.

Re: [libiberty and gdb] floatformat_is_valid

[Daniel Jacobowitz]

On Mon, Sep 15, 2003 at 10:39:33AM -0400, Daniel Jacobowitz wrote:
> GDB's "struct value" has a union in it.  This union, named "aligner", has a
> long double member as well as several others; its goal is to be a cheap way
> to ensure that we can load all sorts of values straight from the value
> structure, without alignment problems.  Unfortunately, this means that when
> debugging GDB we tend to feed random bytes to printf ("%.35Lg").  That's not
> kosher, and on i386 I've found a whole bunch of numbers which can crash it -
> the gist is that GMP shifts normalized numbers over until it hits a one bit,
> and if the explicit integer bit isn't set it walks right out of the number.
> 
> This could show up in other places too.  Anywhere you print floating-point
> data from the inferior, really.
> 
> So here's a patch which checks for the problem.  I added
> floatformat_is_valid to libiberty/floatformat.c.  Is this OK?
> [Do I need approval?  Floatformat.c is in libiberty, but marked as part of
> GDB.]
> 
> Oh, this patch misses some copyright year updates.  I'll get them before
> checking in.

Checked in.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [libiberty and gdb] floatformat_is_valid

[Andrew Cagney]

> +/* Return non-zero iff the data at FROM is a valid number in format FMT.  */
> +
> +int
> +floatformat_is_valid (fmt, from)
> +     const struct floatformat *fmt;
> +     char *from;
> +{

Shouldn't this be a new virtual method in floatformat?

> +  if (fmt == &floatformat_i387_ext)
> +    {
> +      /* In the i387 double-extended format, if the exponent is all
> +	 ones, then the integer bit must be set.  If the exponent
> +	 is neither 0 nor ~0, the intbit must also be set.  Only
> +	 if the exponent is zero can it be zero, and then it must
> +	 be zero.  */
> +      unsigned int exponent, int_bit;
> +      unsigned char *ufrom = (unsigned char *) from;
> +
> +      exponent = get_field (ufrom, fmt->byteorder, fmt->totalsize,
> +			    fmt->exp_start, fmt->exp_len);
> +      int_bit = get_field (ufrom, fmt->byteorder, fmt->totalsize,
> +			   fmt->man_start, 1);
> +
> +      if ((exponent == 0) != (int_bit == 0))
> +	return 0;
> +      else
> +	return 1;
> +    }
> +
> +  /* Other formats with invalid representations should be added
> +     here.  */
> +  return 1;
>  }

Andrew

Re: [libiberty and gdb] floatformat_is_valid

[Daniel Jacobowitz]

On Mon, Sep 15, 2003 at 04:54:17PM -0400, Andrew Cagney wrote:
> >+/* Return non-zero iff the data at FROM is a valid number in format FMT.  
> >*/
> >+
> >+int
> >+floatformat_is_valid (fmt, from)
> >+     const struct floatformat *fmt;
> >+     char *from;
> >+{
> 
> Shouldn't this be a new virtual method in floatformat?

Floatformats don't have any virtual methods right now; if you want to
add them, be my guest.  Besides, this is more in line with the
libiberty policy of backwards compatibility, since it does not change
the structure layout.

> 
> >+  if (fmt == &floatformat_i387_ext)
> >+    {
> >+      /* In the i387 double-extended format, if the exponent is all
> >+	 ones, then the integer bit must be set.  If the exponent
> >+	 is neither 0 nor ~0, the intbit must also be set.  Only
> >+	 if the exponent is zero can it be zero, and then it must
> >+	 be zero.  */
> >+      unsigned int exponent, int_bit;
> >+      unsigned char *ufrom = (unsigned char *) from;
> >+
> >+      exponent = get_field (ufrom, fmt->byteorder, fmt->totalsize,
> >+			    fmt->exp_start, fmt->exp_len);
> >+      int_bit = get_field (ufrom, fmt->byteorder, fmt->totalsize,
> >+			   fmt->man_start, 1);
> >+
> >+      if ((exponent == 0) != (int_bit == 0))
> >+	return 0;
> >+      else
> >+	return 1;
> >+    }
> >+
> >+  /* Other formats with invalid representations should be added
> >+     here.  */
> >+  return 1;
> > }
> 
> Andrew
> 
> 
> 

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [libiberty and gdb] floatformat_is_valid

[Andrew Cagney]

> 
> Floatformats don't have any virtual methods right now; if you want to
> add them, be my guest.

I've another floatformat patch pending, so might as well fix both :-/

> Besides, this is more in line with the
> libiberty policy of backwards compatibility, since it does not change
> the structure layout.

That hasn't been an issue before?

Andrew

Re: [libiberty and gdb] floatformat_is_valid

[Andrew Cagney]

> +      if (!floatformat_is_valid (floatformat_from_type (type), valaddr))

I get:

.../values.c:744: warning: passing arg 2 of `floatformat_is_valid' 
discards qualifiers from pointer target type

I'll fix it with the other changes.

Andrew

Re: [libiberty and gdb] floatformat_is_valid

[DJ Delorie]

> > Besides, this is more in line with the
> > libiberty policy of backwards compatibility, since it does not change
> > the structure layout.
> 
> That hasn't been an issue before?

It has always been an issue.

Re: [libiberty and gdb] floatformat_is_valid

[Mark Kettenis]

   Date: Mon, 15 Sep 2003 10:39:33 -0400
   From: Daniel Jacobowitz <drow@mvista.com>

   2003-09-15  Daniel Jacobowitz  <drow@mvista.com>

	   * floatformat.h (floatformat_is_valid): Add prototype.

   2003-09-15  Daniel Jacobowitz  <drow@mvista.com>

	   * floatformat.c (floatformat_is_valid): New function.
	   (get_field, put_field): Correct comments.

   2003-09-15  Daniel Jacobowitz  <drow@mvista.com>

	   * values.c (unpack_double): Call floatformat_is_valid.


There's a problem with cons-correctness :-(.

gcc -c -g -O2    -I. -I../../src/gdb -I../../src/gdb/config -DLOCALEDIR="\"/usr/local/share/locale\"" -DHAVE_CONFIG_H -I../../src/gdb/../include/opcode -I../../src/gdb/../readline/.. -I../bfd -I../../src/gdb/../bfd  -I../../src/gdb/../include -I../intl -I../../src/gdb/../intl  -DMI_OUT=1 -Wimplicit -Wreturn-type -Wcomment -Wtrigraphs -Wformat -Wparentheses -Wpointer-arith -Wuninitialized -Wformat-nonliteral -Werror ../../src/gdb/values.c
cc1: warnings being treated as errors
../../src/gdb/values.c: In function `unpack_double':
../../src/gdb/values.c:744: warning: passing arg 2 of `floatformat_is_valid' discards qualifiers from pointer target type
gmake[1]: *** [values.o] Error 1
gmake[1]: Leaving directory `/usr/home/kettenis/sandbox/gdb/obj/gdb'
gmake: *** [all-gdb] Error 2

Could you fix that please?

Mark

Re: [libiberty and gdb] floatformat_is_valid

[Andrew Cagney]

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

> On Mon, Sep 15, 2003 at 04:54:17PM -0400, Andrew Cagney wrote:
> 
>> >+/* Return non-zero iff the data at FROM is a valid number in format FMT.  
>> >*/
>> >+
>> >+int
>> >+floatformat_is_valid (fmt, from)
>> >+     const struct floatformat *fmt;
>> >+     char *from;
>> >+{
> 
>> 
>> Shouldn't this be a new virtual method in floatformat?

The attached implements this.  Any objections?

> Floatformats don't have any virtual methods right now; if you want to
> add them, be my guest.  Besides, this is more in line with the
> libiberty policy of backwards compatibility, since it does not change
> the structure layout.

There didn't seem to be a problem when "name" was added, or arm_ext was 
deleted.

enjoy,
Andrew


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

Index: include/ChangeLog
2003-09-15  Andrew Cagney  <cagney@redhat.com>

	* floatformat.h (struct floatformat): Add field "is_valid".

Index: libiberty/ChangeLog
2003-09-15  Andrew Cagney  <cagney@redhat.com>

	* floatformat.c (floatformat_i387_ext_is_valid): New function.
	(floatformat_always_valid): New function.
	(floatformat_m68881_ext): Initialize new "is_valid" field to
	"floatformat_i387_ext_is_valid".
	(floatformat_ieee_single_little): Initialize "is_valid" field to
	floatformat_always_valid.
	(floatformat_ieee_double_big): Ditto.
	(floatformat_ieee_double_little): Ditto.
	(floatformat_ieee_double_little): Ditto.
	(floatformat_ieee_double_littlebyte_bigword): Ditto.
	(floatformat_i960_ext): Ditto.
	(floatformat_m88110_ext): Ditto.
	(floatformat_m88110_harris_ext): Ditto.
	(floatformat_arm_ext_big): Ditto.
	(floatformat_arm_ext_littlebyte_bigword): Ditto.
	(floatformat_ia64_spill_big): Ditto.
	(floatformat_ia64_spill_little): Ditto.
	(floatformat_ia64_quad_big): Ditto.
	(floatformat_ia64_quad_little): Ditto.
	(floatformat_ia64_quad_little): Ditto.
	(floatformat_is_valid): Call "is_valid".

Index: include/floatformat.h
===================================================================
RCS file: /cvs/src/src/include/floatformat.h,v
retrieving revision 1.8
diff -u -r1.8 floatformat.h
--- include/floatformat.h	16 Sep 2003 01:47:53 -0000	1.8
+++ include/floatformat.h	16 Sep 2003 02:24:45 -0000
@@ -80,6 +80,9 @@
 
   /* Internal name for debugging. */
   const char *name;
+
+  /* Validator method.  */
+  int (*is_valid) PARAMS ((const struct floatformat *fmt, const char *from));
 };
 
 /* floatformats for IEEE single and double, big and little endian.  */
Index: libiberty/floatformat.c
===================================================================
RCS file: /cvs/src/src/libiberty/floatformat.c,v
retrieving revision 1.9
diff -u -r1.9 floatformat.c
--- libiberty/floatformat.c	16 Sep 2003 01:47:54 -0000	1.9
+++ libiberty/floatformat.c	16 Sep 2003 02:25:42 -0000
@@ -29,6 +29,22 @@
 extern char *memset ();
 #endif
 
+static unsigned long get_field PARAMS ((const unsigned char *,
+					enum floatformat_byteorders,
+					unsigned int,
+					unsigned int,
+					unsigned int));
+static int floatformat_always_valid PARAMS ((const struct floatformat *fmt,
+					     const char *from));
+
+static int
+floatformat_always_valid (fmt, from)
+     const struct floatformat *fmt;
+     const char *from;
+{
+  return 1;
+}
+
 /* The odds that CHAR_BIT will be anything but 8 are low enough that I'm not
    going to bother with trying to muck around with whether it is defined in
    a system header, what we do if not, etc.  */
@@ -39,25 +55,29 @@
 {
   floatformat_big, 32, 0, 1, 8, 127, 255, 9, 23,
   floatformat_intbit_no,
-  "floatformat_ieee_single_big"
+  "floatformat_ieee_single_big",
+  floatformat_always_valid
 };
 const struct floatformat floatformat_ieee_single_little =
 {
   floatformat_little, 32, 0, 1, 8, 127, 255, 9, 23,
   floatformat_intbit_no,
-  "floatformat_ieee_single_little"
+  "floatformat_ieee_single_little",
+  floatformat_always_valid
 };
 const struct floatformat floatformat_ieee_double_big =
 {
   floatformat_big, 64, 0, 1, 11, 1023, 2047, 12, 52,
   floatformat_intbit_no,
-  "floatformat_ieee_double_big"
+  "floatformat_ieee_double_big",
+  floatformat_always_valid
 };
 const struct floatformat floatformat_ieee_double_little =
 {
   floatformat_little, 64, 0, 1, 11, 1023, 2047, 12, 52,
   floatformat_intbit_no,
-  "floatformat_ieee_double_little"
+  "floatformat_ieee_double_little",
+  floatformat_always_valid
 };
 
 /* floatformat for IEEE double, little endian byte order, with big endian word
@@ -67,34 +87,64 @@
 {
   floatformat_littlebyte_bigword, 64, 0, 1, 11, 1023, 2047, 12, 52,
   floatformat_intbit_no,
-  "floatformat_ieee_double_littlebyte_bigword"
+  "floatformat_ieee_double_littlebyte_bigword",
+  floatformat_always_valid
 };
 
+static int floatformat_i387_ext_is_valid PARAMS ((const struct floatformat *fmt, const char *from));
+
+static int
+floatformat_i387_ext_is_valid (fmt, from)
+     const struct floatformat *fmt;
+     const char *from;
+{
+  /* In the i387 double-extended format, if the exponent is all ones,
+     then the integer bit must be set.  If the exponent is neither 0
+     nor ~0, the intbit must also be set.  Only if the exponent is
+     zero can it be zero, and then it must be zero.  */
+  unsigned long exponent, int_bit;
+  const unsigned char *ufrom = (const unsigned char *) from;
+  
+  exponent = get_field (ufrom, fmt->byteorder, fmt->totalsize,
+			fmt->exp_start, fmt->exp_len);
+  int_bit = get_field (ufrom, fmt->byteorder, fmt->totalsize,
+		       fmt->man_start, 1);
+  
+  if ((exponent == 0) != (int_bit == 0))
+    return 0;
+  else
+    return 1;
+}
+
 const struct floatformat floatformat_i387_ext =
 {
   floatformat_little, 80, 0, 1, 15, 0x3fff, 0x7fff, 16, 64,
   floatformat_intbit_yes,
-  "floatformat_i387_ext"
+  "floatformat_i387_ext",
+  floatformat_i387_ext_is_valid
 };
 const struct floatformat floatformat_m68881_ext =
 {
   /* Note that the bits from 16 to 31 are unused.  */
   floatformat_big, 96, 0, 1, 15, 0x3fff, 0x7fff, 32, 64,
   floatformat_intbit_yes,
-  "floatformat_m68881_ext"
+  "floatformat_m68881_ext",
+  floatformat_always_valid
 };
 const struct floatformat floatformat_i960_ext =
 {
   /* Note that the bits from 0 to 15 are unused.  */
   floatformat_little, 96, 16, 17, 15, 0x3fff, 0x7fff, 32, 64,
   floatformat_intbit_yes,
-  "floatformat_i960_ext"
+  "floatformat_i960_ext",
+  floatformat_always_valid
 };
 const struct floatformat floatformat_m88110_ext =
 {
   floatformat_big, 80, 0, 1, 15, 0x3fff, 0x7fff, 16, 64,
   floatformat_intbit_yes,
-  "floatformat_m88110_ext"
+  "floatformat_m88110_ext",
+  floatformat_always_valid
 };
 const struct floatformat floatformat_m88110_harris_ext =
 {
@@ -102,53 +152,54 @@
      double, and the last 64 bits are wasted. */
   floatformat_big,128, 0, 1, 11,  0x3ff,  0x7ff, 12, 52,
   floatformat_intbit_no,
-  "floatformat_m88110_ext_harris"
+  "floatformat_m88110_ext_harris",
+  floatformat_always_valid
 };
 const struct floatformat floatformat_arm_ext_big =
 {
   /* Bits 1 to 16 are unused.  */
   floatformat_big, 96, 0, 17, 15, 0x3fff, 0x7fff, 32, 64,
   floatformat_intbit_yes,
-  "floatformat_arm_ext_big"
+  "floatformat_arm_ext_big",
+  floatformat_always_valid
 };
 const struct floatformat floatformat_arm_ext_littlebyte_bigword =
 {
   /* Bits 1 to 16 are unused.  */
   floatformat_littlebyte_bigword, 96, 0, 17, 15, 0x3fff, 0x7fff, 32, 64,
   floatformat_intbit_yes,
-  "floatformat_arm_ext_littlebyte_bigword"
+  "floatformat_arm_ext_littlebyte_bigword",
+  floatformat_always_valid
 };
 const struct floatformat floatformat_ia64_spill_big =
 {
   floatformat_big, 128, 0, 1, 17, 65535, 0x1ffff, 18, 64,
   floatformat_intbit_yes,
-  "floatformat_ia64_spill_big"
+  "floatformat_ia64_spill_big",
+  floatformat_always_valid
 };
 const struct floatformat floatformat_ia64_spill_little =
 {
   floatformat_little, 128, 0, 1, 17, 65535, 0x1ffff, 18, 64,
   floatformat_intbit_yes,
-  "floatformat_ia64_spill_little"
+  "floatformat_ia64_spill_little",
+  floatformat_always_valid
 };
 const struct floatformat floatformat_ia64_quad_big =
 {
   floatformat_big, 128, 0, 1, 15, 16383, 0x7fff, 16, 112,
   floatformat_intbit_no,
-  "floatformat_ia64_quad_big"
+  "floatformat_ia64_quad_big",
+  floatformat_always_valid
 };
 const struct floatformat floatformat_ia64_quad_little =
 {
   floatformat_little, 128, 0, 1, 15, 16383, 0x7fff, 16, 112,
   floatformat_intbit_no,
-  "floatformat_ia64_quad_little"
+  "floatformat_ia64_quad_little",
+  floatformat_always_valid
 };
 \f
-static unsigned long get_field PARAMS ((const unsigned char *,
-					enum floatformat_byteorders,
-					unsigned int,
-					unsigned int,
-					unsigned int));
-
 /* Extract a field which starts at START and is LEN bits long.  DATA and
    TOTAL_LEN are the thing we are extracting it from, in byteorder ORDER.  */
 static unsigned long
@@ -411,30 +462,7 @@
      const struct floatformat *fmt;
      const char *from;
 {
-  if (fmt == &floatformat_i387_ext)
-    {
-      /* In the i387 double-extended format, if the exponent is all
-	 ones, then the integer bit must be set.  If the exponent
-	 is neither 0 nor ~0, the intbit must also be set.  Only
-	 if the exponent is zero can it be zero, and then it must
-	 be zero.  */
-      unsigned long exponent, int_bit;
-      const unsigned char *ufrom = (const unsigned char *) from;
-
-      exponent = get_field (ufrom, fmt->byteorder, fmt->totalsize,
-			    fmt->exp_start, fmt->exp_len);
-      int_bit = get_field (ufrom, fmt->byteorder, fmt->totalsize,
-			   fmt->man_start, 1);
-
-      if ((exponent == 0) != (int_bit == 0))
-	return 0;
-      else
-	return 1;
-    }
-
-  /* Other formats with invalid representations should be added
-     here.  */
-  return 1;
+  return fmt->is_valid (fmt, from);
 }
 
 

Re: [libiberty and gdb] floatformat_is_valid

[DJ Delorie]

> There didn't seem to be a problem when "name" was added, or arm_ext
> was deleted.

Structures can be extended iff such extensions don't cause
compatibility problems.  Adding a member to a structure allocated by
the application is bad; the sizes won't match.  Adding one to a
structure that's created only by libiberty itself is safer, providing
the compatibility issues are understood.

Re: [libiberty and gdb] floatformat_is_valid

[Daniel Jacobowitz]

On Mon, Sep 15, 2003 at 10:39:33AM -0400, Daniel Jacobowitz wrote:
> GDB's "struct value" has a union in it.  This union, named "aligner", has a
> long double member as well as several others; its goal is to be a cheap way
> to ensure that we can load all sorts of values straight from the value
> structure, without alignment problems.  Unfortunately, this means that when
> debugging GDB we tend to feed random bytes to printf ("%.35Lg").  That's not
> kosher, and on i386 I've found a whole bunch of numbers which can crash it -
> the gist is that GMP shifts normalized numbers over until it hits a one bit,
> and if the explicit integer bit isn't set it walks right out of the number.
> 
> This could show up in other places too.  Anywhere you print floating-point
> data from the inferior, really.
> 
> So here's a patch which checks for the problem.  I added
> floatformat_is_valid to libiberty/floatformat.c.  Is this OK?
> [Do I need approval?  Floatformat.c is in libiberty, but marked as part of
> GDB.]
> 
> Oh, this patch misses some copyright year updates.  I'll get them before
> checking in.

This patch fixes a really annoyin problem when debugging GDB.  It
doesn't generally come up debugging anything else, though.

I'm planning to leave the patch for 6.0.1.  Does anyone believe it
should be in 6.0 instead?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [libiberty and gdb] floatformat_is_valid

[Andrew Cagney]

> On Mon, Sep 15, 2003 at 10:39:33AM -0400, Daniel Jacobowitz wrote:
> 
>> GDB's "struct value" has a union in it.  This union, named "aligner", has a
>> long double member as well as several others; its goal is to be a cheap way
>> to ensure that we can load all sorts of values straight from the value
>> structure, without alignment problems.  Unfortunately, this means that when
>> debugging GDB we tend to feed random bytes to printf ("%.35Lg").  That's not
>> kosher, and on i386 I've found a whole bunch of numbers which can crash it -
>> the gist is that GMP shifts normalized numbers over until it hits a one bit,
>> and if the explicit integer bit isn't set it walks right out of the number.
>> 
>> This could show up in other places too.  Anywhere you print floating-point
>> data from the inferior, really.
>> 
>> So here's a patch which checks for the problem.  I added
>> floatformat_is_valid to libiberty/floatformat.c.  Is this OK?
>> [Do I need approval?  Floatformat.c is in libiberty, but marked as part of
>> GDB.]
>> 
>> Oh, this patch misses some copyright year updates.  I'll get them before
>> checking in.
> 
> 
> This patch fixes a really annoyin problem when debugging GDB.  It
> doesn't generally come up debugging anything else, though.

For the record, I don't care :-)  It appears to work (I guess my 
revision should go in?), and fixes a bug.

> I'm planning to leave the patch for 6.0.1.  Does anyone believe it
> should be in 6.0 instead?

enjoy,
Andrew

Re: [libiberty and gdb] floatformat_is_valid

[Daniel Jacobowitz]

On Thu, Sep 18, 2003 at 06:59:37PM -0400, Andrew Cagney wrote:
> >On Mon, Sep 15, 2003 at 10:39:33AM -0400, Daniel Jacobowitz wrote:
> >
> >>GDB's "struct value" has a union in it.  This union, named "aligner", has 
> >>a
> >>long double member as well as several others; its goal is to be a cheap 
> >>way
> >>to ensure that we can load all sorts of values straight from the value
> >>structure, without alignment problems.  Unfortunately, this means that 
> >>when
> >>debugging GDB we tend to feed random bytes to printf ("%.35Lg").  That's 
> >>not
> >>kosher, and on i386 I've found a whole bunch of numbers which can crash 
> >>it -
> >>the gist is that GMP shifts normalized numbers over until it hits a one 
> >>bit,
> >>and if the explicit integer bit isn't set it walks right out of the 
> >>number.
> >>
> >>This could show up in other places too.  Anywhere you print floating-point
> >>data from the inferior, really.
> >>
> >>So here's a patch which checks for the problem.  I added
> >>floatformat_is_valid to libiberty/floatformat.c.  Is this OK?
> >>[Do I need approval?  Floatformat.c is in libiberty, but marked as part of
> >>GDB.]
> >>
> >>Oh, this patch misses some copyright year updates.  I'll get them before
> >>checking in.
> >
> >
> >This patch fixes a really annoyin problem when debugging GDB.  It
> >doesn't generally come up debugging anything else, though.
> 
> For the record, I don't care :-)  It appears to work (I guess my 
> revision should go in?), and fixes a bug.

I asked myself, "Dan, are you going to add this patch to all your GDB
packages anyway?"  The answer was yes, so I'm checking it in.

+2003-09-19  Daniel Jacobowitz  <drow@mvista.com>
+
+       Merge from mainline:
+       2003-09-15  Daniel Jacobowitz  <drow@mvista.com>
+
+       * values.c (unpack_double): Call floatformat_is_valid.

+2003-09-15  Andrew Cagney  <cagney@redhat.com>
+
+       * floatformat.h (floatformat_to_double): Make input buffer constant.
+       (floatformat_from_double, floatformat_is_valid): Ditto.
+       
+2003-09-15  Daniel Jacobowitz  <drow@mvista.com>
+
+       * floatformat.h (floatformat_is_valid): Add prototype.
+

+2003-09-15  Andrew Cagney  <cagney@redhat.com>
+
+       * floatformat.c (get_field): Make "data" constant.
+       (floatformat_is_valid, floatformat_to_double): Make "from"
+       constant, fix casts.
+       (floatformat_from_double): Make "from" constant.
+
+2003-09-15  Daniel Jacobowitz  <drow@mvista.com>
+
+       * floatformat.c (floatformat_is_valid): New function.
+       (get_field, put_field): Correct comments.
+

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [libiberty and gdb] floatformat_is_valid

[Andrew Cagney]

I've checked this into the mainline.

Andrew

Index: include/ChangeLog
2003-09-15  Andrew Cagney  <cagney@redhat.com>

	* floatformat.h (struct floatformat): Add field "is_valid".

Index: libiberty/ChangeLog
2003-09-15  Andrew Cagney  <cagney@redhat.com>

	* floatformat.c (floatformat_i387_ext_is_valid): New function.
	(floatformat_always_valid): New function.
	(floatformat_m68881_ext): Initialize new "is_valid" field to
	"floatformat_i387_ext_is_valid".
	(floatformat_ieee_single_little): Initialize "is_valid" field to
	floatformat_always_valid.
	(floatformat_ieee_double_big): Ditto.
	(floatformat_ieee_double_little): Ditto.
	(floatformat_ieee_double_little): Ditto.
	(floatformat_ieee_double_littlebyte_bigword): Ditto.
	(floatformat_i960_ext): Ditto.
	(floatformat_m88110_ext): Ditto.
	(floatformat_m88110_harris_ext): Ditto.
	(floatformat_arm_ext_big): Ditto.
	(floatformat_arm_ext_littlebyte_bigword): Ditto.
	(floatformat_ia64_spill_big): Ditto.
	(floatformat_ia64_spill_little): Ditto.
	(floatformat_ia64_quad_big): Ditto.
	(floatformat_ia64_quad_little): Ditto.
	(floatformat_ia64_quad_little): Ditto.
	(floatformat_is_valid): Call "is_valid".