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: 902 bytes --]

Pierre Muller wrote:
> 
>>>   For instance, your patch does not complaint about this
>>>         .stabs  "t30:t(0,30)=@s8;r(0,30);02000;0077;",128,0,0,0
>>> but 02000 is -1024 and does not fit into a 8 bit memory.
>>>
>> Right.  Does this really ever happen?  A check can be added then...
>   For good behaving compilers, probably not,
> but it never hurts to be on the sure size and to be able
> to report if some input is malformed.
> 
>   I would really appreciate that report an error
> whenever the number cannot be parsed correctly.
> 
>>>   I agree that there are normally no reasons to have more digits,
>>> but more  leading zeroes should not lead to an error
>> They don't, AFAICS.
>>
>>> ... in the parsing
>>> and any bit set higher that this should trigger an error.
>>>
>> OK ...
>>

Updated patch attached, as per your comments.

Regtested on i386-pc-cygwin, C/C++.

Cheers,
Pedro Alves

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

2007-09-25  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 |   96 +++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 67 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-24 23:03:50.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,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))
+	    {
+	      /* Definitily signed.  */
+	      twos_complement_representation = 1;
+	      sign = -1;
+	    }
+	}
+    }
+
   upper_limit = LONG_MAX / radix;
 
   while ((c = *p++) >= '0' && c < ('0' + radix))
@@ -3736,23 +3766,19 @@ 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 (!saw_first)
+		{
+		  long sn = c - '0' - ((2 * (c - '0')) | (2 << sign_bit));
+		  n = -sn;
+		  saw_first = 1;
+		}
               else
                 {
-                  sn *= radix;
-                  sn += c - '0';
+                  n *= radix;
+                  n += sign * (c - '0');
                 }
-
-              if (sn < 0)
-                n = -sn;
             }
           else
             {
@@ -3796,6 +3822,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 +3844,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 +3855,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 +3980,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 #3: read_huge_number.s --]
[-- Type: text/plain, Size: 1930 bytes --]

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

	.stabs  "t30:t(0,30)=@s8;r(0,30);02000;0077;",128,0,0,0
	
	.stabs	"u8:t(0,22)=r(0,22);00;0377;",128,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: