This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


I wrote:
What about the attached? Isn't it simpler?

We get passed the max number of bits the number we're parsing
can hold in TWOS_COMPLEMENT_BITS.

Just parse the number as unsigned, if it overflows, it doesn't
matter, we just account for the bits.
If it doesn't overflow (host has a big long),
    and TWOS_COMPLEMENT_BITS < sizeof (long) * HOST_CHAR_BIT
    and number is signed according to TWOS_COMPLEMENT_BITS - 1
    bit being set, sign extend the number into a long.


Blah, that doesn't work quite right in all the cases. Since with that, we always read the number as unsigned while we're deciding if we'll overflow, and we will overflow on small 64-bit negative numbers, on 32-bit hosts, while previously we wouldn't. See below. (BTW, why the heck isn't the return of read_huge_number a LONGEST?)

I looked at what the fixed_points.exp is doing:
A -50 .. 50 range with delta 1.0 / 16.0,

Which translates into a -800 .. 800 range like:
"s800:t(0,23)=@s64;r(0,23);01777777777777777776340;0000000001440;",128,0,0,0

Current CVS doesn't overflow on 01777777777777777776340, but both my
patch, and Pierre's did.

The real problem with the original code is that it assumes
that when twos_complement_bits > 0 and the number is in octal,
it must be negative.  That isn't true always, as can be seen on the
case that Pierre showed:

.stabs	"long long unsigned
int:t(0,7)=@s64;r(0,7);0000000000000;01777777777777777777777;",128,0,0,0

We would parse octal 0000000000000 (, with
twos_complement_bits == 64, due to that @s64), until we hit
-268435456 (0x10000000), at which point is was considered an overflow,
and only the n of bits where taken care of.

So, this new patch, attached, checks to see if enough
algarisms are there for signess, and if so only checks the
sign bit on the first iteration.

Another problem then shows up:  Prior to Michael's fix, we always
read the octals as *unsigned*, so, on i386-pc-cygwin, the
01777777777777777777777 on the "long long unsigned" case was
always considered overflow as it didn't fit in a long.  That case
is then handled by read_range_type not by looking at value of
01777777777777777777777, but to the n of bits needed to represent
the number (64).  With the twos_complement_representation code
fixed, the code parses  01777777777777777777777,size_type=64 (s64)
as -1, thus returning  n2=0,n3=-1,n2bits=0,n3bits=0.  But that case
isn't handled correctly later in in read_range_type, where it is assumed
to represent an unsigned int (32-bit).

/* If the upper bound is -1, it must really be an unsigned int. */

    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,
                          TYPE_FLAG_UNSIGNED, NULL, objfile);
      }

The patch fixes it by calling init_type with
'type_size / TARGET_CHAR_BIT', if type_size > 0.  Anyone has
such an old version of gcc around to test if this brakes it?

We now correcly parse what I could find gcc outputting, and few
things more - see attached .s file.

I've also ran the testsuite wheel a couple of times,
and it all looks fine.

Cheers,
Pedro Alves

2007-09-24  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 |   82 ++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 54 insertions(+), 28 deletions(-)

Index: src/gdb/stabsread.c
===================================================================
--- src.orig/gdb/stabsread.c	2007-09-23 17:08:30.000000000 +0100
+++ src/gdb/stabsread.c	2007-09-24 00:41:12.000000000 +0100
@@ -3704,14 +3704,14 @@ read_huge_number (char **pp, int end, in
   char *p = *pp;
   int sign = 1;
   int sign_bit;
+  int saw_first = 0;
   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 +3727,30 @@ 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.  */
+	  twos_complement_representation = 1;
+	  sign_bit = (twos_complement_bits % 3 + 2) % 3;
+	}
+    }
+
   upper_limit = LONG_MAX / radix;
 
   while ((c = *p++) >= '0' && c < ('0' + radix))
@@ -3736,23 +3759,23 @@ 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.
+		 signed_bit is the position of the sign bit in the
+		 first three bits.  */
+	      if (!saw_first && (c & (1 << sign_bit)))
+		{
+		  long sn = c - '0' - ((2 * (c - '0')) | (2 << sign_bit));
+		  sign = -1;
+		  n = -sn;
+		}
               else
                 {
-                  sn *= radix;
-                  sn += c - '0';
+                  n *= radix;
+                  n += sign * (c - '0');
                 }
 
-              if (sn < 0)
-                n = -sn;
+	      saw_first = 1;
             }
           else
             {
@@ -3809,8 +3832,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 represantion octals can't
+	 have a '-' in front.  */
+      if (sign == -1 && !twos_complement_representation)
 	++nbits;
       if (bits)
 	*bits = nbits;
@@ -3819,10 +3843,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 +3968,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 it's 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);
     }
 


	.stabs	"read_huge_number.s",100,0,0,Ltext0
	.text
Ltext0:
	.stabs	"gcc2_compiled.",60,0,0,0

	.stabs	"long long unsigned int:t(0,7)=@s64;r(0,7);0000000000000;01777777777777777777777;",128,0,0,0
	.stabs	"int:t(0,1)=r(0,1);-2147483648;2147483647;",128,0,0,0
	.stabs	"char:t(0,2)=r(0,2);0;127;",128,0,0,0
	.stabs	"long int:t(0,3)=r(0,3);-2147483648;2147483647;",128,0,0,0
	.stabs	"unsigned int:t(0,4)=r(0,4);0000000000000;0037777777777;",128,0,0,0
	.stabs	"long unsigned int:t(0,5)=r(0,5);0000000000000;0037777777777;",128,0,0,0
	.stabs	"long long int:t(0,6)=@s64;r(0,6);01000000000000000000000;0777777777777777777777;",128,0,0,0
	.stabs	"long long unsigned int:t(0,7)=@s64;r(0,7);0000000000000;01777777777777777777777;",128,0,0,0
	.stabs	"short int:t(0,8)=@s16;r(0,8);-32768;32767;",128,0,0,0
	.stabs	"short unsigned int:t(0,9)=@s16;r(0,9);0;65535;",128,0,0,0
	.stabs	"signed char:t(0,10)=@s8;r(0,10);-128;127;",128,0,0,0
	.stabs	"unsigned char:t(0,11)=@s8;r(0,11);0;255;",128,0,0,0
	.stabs	"float:t(0,12)=r(0,1);4;0;",128,0,0,0
	.stabs	"double:t(0,13)=r(0,1);8;0;",128,0,0,0
	.stabs	"long double:t(0,14)=r(0,1);12;0;",128,0,0,0
	.stabs	"complex int:t(0,15)=s8real:(0,1),0,32;imag:(0,1),32,32;;",128,0,0,0
	.stabs	"complex float:t(0,16)=R3;8;0;",128,0,0,0
	.stabs	"complex double:t(0,17)=R3;16;0;",128,0,0,0
	.stabs	"complex long double:t(0,18)=R3;24;0;",128,0,0,0
	.stabs	"void:t(0,19)=(0,19)",128,0,0,0
	.stabs	"__builtin_va_list:t(0,20)=*(0,2)",128,0,0,0
	.stabs	"_Bool:t(0,21)=@s8;-16",128,0,0,0

	.stabs	"s8:t(0,22)=@s8;r(0,22);0200;0177;",128,0,0,0
	.stabs	"s800:t(0,23)=@s64;r(0,23);01777777777777777776340;0000000001440;",128,0,0,0
	
	.stabs	"s16:t(0,24)=@s16;r(0,24);0100000;077777;",128,0,0,0
	.stabs	"u16:t(0,25);r(0,25);0000000;0177777;",128,0,0,0

	.text
	.stabs	"main:F(0,1)",36,0,5,_main
.globl _main
_main:

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]