Re: [RFC] Stabs parsing regression from GDB 6.6 to GDB 6.6.90

[Pedro Alves <pedro_alves@portugalmail.pt>]

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

Daniel Jacobowitz wrote:
> If no one objects, I'll approve such a reversion for the branch and
> your patch for trunk.  Give it a day or two in case someone else has
> a better idea.

Just finished testing the reversion on 6.7 branch.  No results changes,
and I've confirmed that the 'unsigned long long' problem is cured by it.

I've checked this in to the branch ...

2007-10-08  Pedro Alves  <pedro_alves@portugalmail.pt>

         Revert:
         2007-08-10  Michael Snyder  <msnyder@access-company.com>

         * stabsread.c (read_huge_number): Attempt to compute value before
         values that it depends on.

... and these to head:

2007-10-09  Pedro Alves  <pedro_alves@portugalmail.pt>

         * stabsread.c (read_huge_number): Fix handling of octal
         representation when the bit width is known.
         (read_range_type): Record unsigned integral types with their size,
         when the type size is known.

2007-10-09  Pedro Alves  <pedro_alves@portugalmail.pt>

	* gdb.base/whatis.c (v_long_long, v_signed_long_long)
	(v_unsigned_long_long, v_long_long_array)
	(v_signed_long_long_array, v_unsigned_long_long_array)
	(slong_long_addr, a_slong_long_addr, v_long_long_pointer)
	(v_signed_long_long_pointer, v_unsigned_long_long_pointer)
	[!NO_LONG_LONG]: New.
	(t_struct, v_struct2, t_union, v_union2) [!NO_LONG_LONG]: Add
	v_long_long_member.

	(v_long_long_func, v_signed_long_long_func)
	(v_unsigned_long_long_func) [!NO_LONG_LONG]: New.
	(main) [!NO_LONG_LONG]: Initialize long long variants.

	* gdb.base/whatis.exp: If board file requests no_long_long, build
	test with NO_LONG_LONG defined.  Test long long, signed long long,
	and unsigned long long variants but only if board file doesn't
	disable it.

Full patches attached.

---

Testing on cygwin looks much better on the 6.7 branch compared to the
version currently shipped with Cygwin.  Here are .sum results
with 'set_board_info gdb,nosignals 1' - cygwin signals aren't
supported yet.

==> gdb-20060706-2/testresults/gdb.sum <==
# of expected passes            8855
# of unexpected failures        343
# of unexpected successes       1
# of expected failures          45
# of known failures             52
# of unresolved testcases       17
# of untested testcases         17
# of unsupported tests          30
/cygdrive/d/gdb/gdb-20060706-2/build/gdb/testsuite/../../gdb/gdb version 
6.5.50.20060706-cvs -nx

==> gdb-6.7/testresults/reverted/gdb.sum <==
# of expected passes            9471
# of unexpected failures        170
# of unexpected successes       1
# of expected failures          47
# of known failures             30
# of unresolved testcases       1
# of untested testcases         13
# of unsupported tests          32
/cygdrive/d/gdb/gdb-6.7/build-cygwin/gdb/testsuite/../../gdb/gdb version 
6.6.90.20071007-cvs -nx

-- 

Cheers,
Pedro Alves

[-- Attachment #2: revert_read_huge_number.diff --]
[-- Type: text/x-diff, Size: 1026 bytes --]

2007-10-08  Pedro Alves  <pedro_alves@portugalmail.pt>

	Revert:
	2007-08-10  Michael Snyder  <msnyder@access-company.com>

	* stabsread.c (read_huge_number): Attempt to compute value before
	values that it depends on.

---
 gdb/stabsread.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Index: src/gdb/stabsread.c
===================================================================
--- src.orig/gdb/stabsread.c	2007-09-05 01:51:48.000000000 +0100
+++ src/gdb/stabsread.c	2007-10-07 23:08:02.000000000 +0100
@@ -3711,7 +3711,7 @@ read_huge_number (char **pp, int end, in
   int nbits = 0;
   int c;
   long upper_limit;
-  int twos_complement_representation;
+  int twos_complement_representation = radix == 8 && twos_complement_bits > 0;
 
   if (*p == '-')
     {
@@ -3727,7 +3727,6 @@ read_huge_number (char **pp, int end, in
       p++;
     }
 
-  twos_complement_representation = radix == 8 && twos_complement_bits > 0;
   upper_limit = LONG_MAX / radix;
 
   while ((c = *p++) >= '0' && c < ('0' + radix))

[-- Attachment #3: read_huge_number2.diff --]
[-- Type: text/x-diff, Size: 5161 bytes --]

2007-10-09  Pedro Alves  <pedro_alves@portugalmail.pt>

	* stabsread.c (read_huge_number): Fix handling of octal
	representation when the bit width is known.
	(read_range_type): Record unsigned integral types with their size,
	when the type size is known.

---
 gdb/stabsread.c |   94 ++++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 65 insertions(+), 29 deletions(-)

Index: src/gdb/stabsread.c
===================================================================
--- src.orig/gdb/stabsread.c	2007-09-24 01:38:14.000000000 +0100
+++ src/gdb/stabsread.c	2007-09-25 21:21:52.000000000 +0100
@@ -3705,13 +3705,12 @@ read_huge_number (char **pp, int end, in
   int sign = 1;
   int sign_bit;
   long n = 0;
-  long sn = 0;
   int radix = 10;
   char overflow = 0;
   int nbits = 0;
   int c;
   long upper_limit;
-  int twos_complement_representation;
+  int twos_complement_representation = 0;
 
   if (*p == '-')
     {
@@ -3727,7 +3726,37 @@ read_huge_number (char **pp, int end, in
       p++;
     }
 
-  twos_complement_representation = radix == 8 && twos_complement_bits > 0;
+  /* Skip extra zeros.  */
+  while (*p == '0')
+    p++;
+
+  if (sign > 0 && radix == 8 && twos_complement_bits > 0)
+    {
+      /* Octal, possibly signed.  Check if we have enough chars for a
+	 negative number.  */
+
+      size_t len;
+      char *p1 = p;
+      while ((c = *p1) >= '0' && c < '8')
+	p1++;
+
+      len = p1 - p;
+      if (len > twos_complement_bits / 3
+	  || (twos_complement_bits % 3 == 0 && len == twos_complement_bits / 3))
+	{
+	  /* Ok, we have enough characters for a signed value, check
+	     for signness by testing if the sign bit is set.  */
+	  sign_bit = (twos_complement_bits % 3 + 2) % 3;
+	  c = *p - '0';
+	  if (c & (1 << sign_bit))
+	    {
+	      /* Definitely signed.  */
+	      twos_complement_representation = 1;
+	      sign = -1;
+	    }
+	}
+    }
+
   upper_limit = LONG_MAX / radix;
 
   while ((c = *p++) >= '0' && c < ('0' + radix))
@@ -3736,23 +3765,18 @@ read_huge_number (char **pp, int end, in
         {
           if (twos_complement_representation)
             {
-              /* Octal, signed, twos complement representation. In this case,
-                 sn is the signed value, n is the corresponding absolute
-                 value. signed_bit is the position of the sign bit in the
-                 first three bits.  */
-              if (sn == 0)
-                {
-                  sign_bit = (twos_complement_bits % 3 + 2) % 3;
-                  sn = c - '0' - ((2 * (c - '0')) | (2 << sign_bit));
-                }
+	      /* Octal, signed, twos complement representation.  In
+		 this case, n is the corresponding absolute value.  */
+	      if (n == 0)
+		{
+		  long sn = c - '0' - ((2 * (c - '0')) | (2 << sign_bit));
+		  n = -sn;
+		}
               else
                 {
-                  sn *= radix;
-                  sn += c - '0';
+                  n *= radix;
+                  n -= c - '0';
                 }
-
-              if (sn < 0)
-                n = -sn;
             }
           else
             {
@@ -3796,6 +3820,15 @@ read_huge_number (char **pp, int end, in
   else
     --p;
 
+  if (radix == 8 && twos_complement_bits > 0 && nbits > twos_complement_bits)
+    {
+      /* We were supposed to parse a number with maximum
+	 TWOS_COMPLEMENT_BITS bits, but something went wrong.  */
+      if (bits != NULL)
+	*bits = -1;
+      return 0;
+    }
+
   *pp = p;
   if (overflow)
     {
@@ -3809,8 +3842,9 @@ read_huge_number (char **pp, int end, in
 	}
 
       /* -0x7f is the same as 0x80.  So deal with it by adding one to
-         the number of bits.  */
-      if (sign == -1)
+         the number of bits.  Two's complement represention octals
+         can't have a '-' in front.  */
+      if (sign == -1 && !twos_complement_representation)
 	++nbits;
       if (bits)
 	*bits = nbits;
@@ -3819,10 +3853,7 @@ read_huge_number (char **pp, int end, in
     {
       if (bits)
 	*bits = 0;
-      if (twos_complement_representation)
-        return sn;
-      else
-        return n * sign;
+      return n * sign;
     }
   /* It's *BITS which has the interesting information.  */
   return 0;
@@ -3947,15 +3978,20 @@ read_range_type (char **pp, int typenums
 	return float_type;
     }
 
-  /* If the upper bound is -1, it must really be an unsigned int.  */
+  /* If the upper bound is -1, it must really be an unsigned integral.  */
 
   else if (n2 == 0 && n3 == -1)
     {
-      /* It is unsigned int or unsigned long.  */
-      /* GCC 2.3.3 uses this for long long too, but that is just a GDB 3.5
-         compatibility hack.  */
-      return init_type (TYPE_CODE_INT, 
-			gdbarch_int_bit (current_gdbarch) / TARGET_CHAR_BIT,
+      int bits = type_size;
+      if (bits <= 0)
+	{
+	  /* We don't know its size.  It is unsigned int or unsigned
+	     long.  GCC 2.3.3 uses this for long long too, but that is
+	     just a GDB 3.5 compatibility hack.  */
+	  bits = gdbarch_int_bit (current_gdbarch);
+	}
+
+      return init_type (TYPE_CODE_INT, bits / TARGET_CHAR_BIT,
 			TYPE_FLAG_UNSIGNED, NULL, objfile);
     }
 

[-- Attachment #4: whatis_long_long.diff --]
[-- Type: text/x-diff, Size: 8184 bytes --]

2007-10-09  Pedro Alves  <pedro_alves@portugalmail.pt>

	* gdb.base/whatis.c (v_long_long, v_signed_long_long)
	(v_unsigned_long_long, v_long_long_array)
	(v_signed_long_long_array, v_unsigned_long_long_array)
	(slong_long_addr, a_slong_long_addr, v_long_long_pointer)
	(v_signed_long_long_pointer, v_unsigned_long_long_pointer)
	[!NO_LONG_LONG]: New.
	(t_struct, v_struct2, t_union, v_union2) [!NO_LONG_LONG]: Add
	v_long_long_member.

	(v_long_long_func, v_signed_long_long_func)
	(v_unsigned_long_long_func) [!NO_LONG_LONG]: New.
	(main) [!NO_LONG_LONG]: Initialize long long variants.

	* gdb.base/whatis.exp: If board file requests no_long_long, build
	test with NO_LONG_LONG defined.  Test long long, signed long long,
	and unsigned long long variants but only if board file doesn't
	disable it.

---
 gdb/testsuite/gdb.base/whatis.c   |   60 +++++++++++++++++++++++++++++++++++++-
 gdb/testsuite/gdb.base/whatis.exp |   49 ++++++++++++++++++++++++++++++-
 2 files changed, 107 insertions(+), 2 deletions(-)

Index: src/gdb/testsuite/gdb.base/whatis.c
===================================================================
--- src.orig/gdb/testsuite/gdb.base/whatis.c	2007-10-03 01:28:46.000000000 +0100
+++ src/gdb/testsuite/gdb.base/whatis.c	2007-10-08 23:49:04.000000000 +0100
@@ -42,6 +42,12 @@ long		v_long;
 signed long	v_signed_long;
 unsigned long	v_unsigned_long;
 
+#ifndef NO_LONG_LONG
+long long		v_long_long;
+signed long long	v_signed_long_long;
+unsigned long long	v_unsigned_long_long;
+#endif
+
 float		v_float;
 double		v_double;
 
@@ -68,6 +74,12 @@ long		v_long_array[2];
 signed long	v_signed_long_array[2];
 unsigned long	v_unsigned_long_array[2];
 
+#ifndef NO_LONG_LONG
+long long		v_long_long_array[2];
+signed long long	v_signed_long_long_array[2];
+unsigned long long	v_unsigned_long_long_array[2];
+#endif
+
 float		v_float_array[2];
 double		v_double_array[2];
 
@@ -83,6 +95,10 @@ typedef unsigned short *ushort_addr;
 static ushort_addr a_ushort_addr;
 typedef signed long *slong_addr;
 static slong_addr a_slong_addr;
+#ifndef NO_LONG_LONG
+typedef signed long long *slong_long_addr;
+static slong_long_addr a_slong_long_addr;
+#endif
 
 char		*v_char_pointer;
 signed char	*v_signed_char_pointer;
@@ -100,6 +116,12 @@ long		*v_long_pointer;
 signed long	*v_signed_long_pointer;
 unsigned long	*v_unsigned_long_pointer;
 
+#ifndef NO_LONG_LONG
+long long		*v_long_long_pointer;
+signed long long	*v_signed_long_long_pointer;
+unsigned long long	*v_unsigned_long_long_pointer;
+#endif
+
 float		*v_float_pointer;
 double		*v_double_pointer;
 
@@ -110,6 +132,9 @@ struct t_struct {
     short	v_short_member;
     int		v_int_member;
     long	v_long_member;
+#ifndef NO_LONG_LONG
+    long long	v_long_long_member;
+#endif
     float	v_float_member;
     double	v_double_member;
 } v_struct1;
@@ -119,6 +144,9 @@ struct {
     short	v_short_member;
     int		v_int_member;
     long	v_long_member;
+#ifndef NO_LONG_LONG
+    long long	v_long_long_member;
+#endif
     float	v_float_member;
     double	v_double_member;
 } v_struct2;
@@ -130,6 +158,9 @@ union t_union {
     short	v_short_member;
     int		v_int_member;
     long	v_long_member;
+#ifndef NO_LONG_LONG
+    long long	v_long_long_member;
+#endif
     float	v_float_member;
     double	v_double_member;
 } v_union;
@@ -139,6 +170,9 @@ union {
     short	v_short_member;
     int		v_int_member;
     long	v_long_member;
+#ifndef NO_LONG_LONG
+    long long	v_long_long_member;
+#endif
     float	v_float_member;
     double	v_double_member;
 } v_union2;
@@ -161,6 +195,12 @@ long		v_long_func () { return (0); }
 signed long	v_signed_long_func () { return (0); }
 unsigned long	v_unsigned_long_func () { return (0); }
 
+#ifndef NO_LONG_LONG
+long long		v_long_long_func () { return (0); }
+signed long long	v_signed_long_long_func () { return (0); }
+unsigned long long	v_unsigned_long_long_func () { return (0); }
+#endif
+
 float		v_float_func () { return (0.0); }
 double		v_double_func () { return (0.0); }
 
@@ -229,7 +269,13 @@ int main ()
   v_long = 9;
   v_signed_long = 10;
   v_unsigned_long = 11;    
-  
+
+#ifndef NO_LONG_LONG
+  v_long_long = 12;
+  v_signed_long_long = 13;
+  v_unsigned_long_long = 14;
+#endif
+
   v_float = 100.0;
   v_double = 200.0;
 
@@ -250,6 +296,12 @@ int main ()
   v_signed_long_array[0] = v_signed_long;
   v_unsigned_long_array[0] = v_unsigned_long;
 
+#ifndef NO_LONG_LONG
+  v_long_long_array[0] = v_long_long;
+  v_signed_long_long_array[0] = v_signed_long_long;
+  v_unsigned_long_long_array[0] = v_unsigned_long_long;
+#endif
+
   v_float_array[0] = v_float;
   v_double_array[0] = v_double;
 
@@ -269,6 +321,12 @@ int main ()
   v_signed_long_pointer = &v_signed_long;
   v_unsigned_long_pointer = &v_unsigned_long;
 
+#ifndef NO_LONG_LONG
+  v_long_long_pointer = &v_long_long;
+  v_signed_long_long_pointer = &v_signed_long_long;
+  v_unsigned_long_long_pointer = &v_unsigned_long_long;
+#endif
+
   v_float_pointer = &v_float;
   v_double_pointer = &v_double;
 
Index: src/gdb/testsuite/gdb.base/whatis.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.base/whatis.exp	2007-10-03 01:28:46.000000000 +0100
+++ src/gdb/testsuite/gdb.base/whatis.exp	2007-10-03 23:21:46.000000000 +0100
@@ -26,10 +26,16 @@ if $tracelevel {
 set prms_id 0
 set bug_id 0
 
+if [target_info exists no_long_long] {
+    set exec_opts [list debug additional_flags=-DNO_LONG_LONG]
+} else {
+    set exec_opts [list debug]
+}
+
 set testfile whatis
 set srcfile ${testfile}.c
 set binfile ${objdir}/${subdir}/${testfile}
-if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable $exec_opts] != "" } {
      untested whatis.exp
      return -1
 }
@@ -124,6 +130,13 @@ gdb_test "whatis v_unsigned_long" \
     "type = (unsigned long|long unsigned int)" \
     "whatis unsigned long"
 
+
+if ![target_info exists no_long_long] {
+    gdb_test "whatis v_unsigned_long_long" \
+	"type = (unsigned long long|long long unsigned int)" \
+	"whatis unsigned long long"
+}
+
 gdb_test "whatis v_float" \
     "type = float" \
     "whatis float"
@@ -188,6 +201,12 @@ gdb_test "whatis v_unsigned_long_array" 
     "type = (unsigned (int|long|long int)|long unsigned int) \\\[2\\\]" \
     "whatis unsigned long array"
 
+if ![target_info exists no_long_long] {
+    gdb_test "whatis v_unsigned_long_long_array" \
+	"type = (unsigned long long|long long unsigned int) \\\[2\\\]" \
+	"whatis unsigned long array"
+}
+
 gdb_test "whatis v_float_array" \
     "type = float \\\[2\\\]" \
     "whatis float array"
@@ -251,6 +270,20 @@ gdb_test "whatis v_unsigned_long_pointer
     "type = (unsigned (int|long|long int)|long unsigned int) \\*" \
     "whatis unsigned long pointer"
 
+if ![target_info exists no_long_long] {
+    gdb_test "whatis v_long_long_pointer" \
+	"type = long long(| int) \\*" \
+	"whatis long long pointer"
+
+    gdb_test "whatis v_signed_long_long_pointer" \
+	"type = (signed |)long long(| int) \\*" \
+	"whatis signed long long pointer"
+
+    gdb_test "whatis v_unsigned_long_long_pointer" \
+	"type = (unsigned long long|long long unsigned int) \\*" \
+	"whatis unsigned long long pointer"
+}
+
 gdb_test "whatis v_float_pointer" \
     "type = float \\*" \
     "whatis float pointer"
@@ -353,6 +386,20 @@ gdb_test "whatis v_unsigned_long_func" \
     "type = (unsigned (int|long|long int)|long unsigned int) \\($void\\)" \
     "whatis unsigned long function"
 
+if ![target_info exists no_long_long] {
+    gdb_test "whatis v_long_long_func" \
+	"type = long long(| int) \\($void\\)" \
+	"whatis long long function"
+
+    gdb_test "whatis v_signed_long_long_func" \
+	"type = (signed |)long long(| int) \\($void\\)" \
+	"whatis signed long long function"
+
+    gdb_test "whatis v_unsigned_long_long_func" \
+	"type = (unsigned long long(| int)|long long unsigned int) \\($void\\)" \
+	"whatis unsigned long long function"
+}
+
 # Sun /bin/cc calls this a function returning double.
 if {!$gcc_compiled} then {setup_xfail "*-sun-sunos4*"}
 gdb_test "whatis v_float_func" \