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

["Pierre Muller" <muller@ics.u-strasbg.fr>]

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

  As I was unable to understand the current implementation of
the twos_complement_representation
I rewrote it almost completely.
  The code now checks that the most significant
bit of the whole octal representation of the huge number
that is being parsed is exactly at the bit position given by
the twos_complement_bits parameter.

  The attached patch (against 6.6.90 source)
fixes the problem that I describe in the previous
email. I get no complaint for the 'unsigned long long' type
compiled with '-gstabs+' option.

ChangeLog entry:

2007-0921  Pierre Muller <muller@is.u-strasbg.fr>

	* stabsread.c (read_huge_number): fix the parsing
	of octal representation when twos_complement_bits 
	value is set.

  I am sorry, but I am again unable to check
if there are no regressions.
 

Pierre Muller

> -----Original Message-----
> From: gdb-owner@sourceware.org [mailto:gdb-owner@sourceware.org] On
> Behalf Of Pierre Muller
> Sent: Thursday, September 20, 2007 5:30 PM
> To: gdb@sourceware.org
> Cc: 'Michael Snyder'
> Subject: [RFC] Stabs parsing regression from GDB 6.6 to GDB 6.6.90
> 
>   There is a new problem that
> appears in 6.6.90 and was not present in
> 6.6 related to the gcc
> -gstabs+ option.
> 
>   If I compile the following tiny source
> and try to get the type of the variable u,
> I get an error, because
> gdb is not able to handle the extended
> stabs generated with -gstabs+ option
> (if you only use -gstabs, it works OK).
> 
> >>>Source file:
> unsigned long long u;
> 
> int
> main ()
> {
>   u = 15;
>   printf("Value of u is %lu\r\n",u);
>   return 0;
> }
> >>>End of source file
> 
> >>> Compilation
> gcc -gstabs+ -o testll testll.c
> 
> >>> Launch gdb
> 
> gdb6690 ./testll
> and
> type 'ptyp u'
> 
> and you will receive an error...
> 
> The error is caused by:
>         .stabs  "long long unsigned
> int:t(0,7)=@s64;r(0,7);0000000000000;01777777777777777777777;",128,0,0,
> 0
> (With -gstabs, you don't get the '@s64;' part
> which means that the size is 64 bits wide).
> 
> Note that this is parsed without generating errors:
>        .stabs  "long long
> int:t(0,6)=@s64;r(0,6);01000000000000000000000;0777777777777777777777;"
> ,128,
> 0,0,0
> 
> I added Michael Snyder in CC
> because it is the only name that
> I found associated with a recent change in read_huge_number
> stabsread.c function. It seems that
> his change changed the position
> of the assignment of the variable
> twos_complement_representation,
> that was set in earlier version before
> radix could be changed to 8 because of a leading '0'.
> 
> 
>   Thus in 6.6 gdb, twos_complement_representation is
> always equal to zero and the problem appears
> now because it is now set correctly. It seems
> that the code for twos_complement_representation
> is not working properly for 13 '0' as in 'long long unsigned int' type
> above.
> But I am unable to understand the code correctly.
> 
>   Michael, could you please confirm my problem and
> tell us if the
> 
> Pierre Muller
> Pascal language code maintainer
> 


[-- Attachment #2: stabsread.patch --]
[-- Type: application/octet-stream, Size: 4057 bytes --]

--- gdb-6.6.90-orig/gdb/stabsread.c	Thu Sep 20 11:58:55 2007
+++ gdb-6.6.90/gdb/stabsread.c	Fri Sep 21 09:41:32 2007
@@ -3704,6 +3704,7 @@ read_huge_number (char **pp, int end, in
   char *p = *pp;
   int sign = 1;
   int sign_bit;
+  int sign_bit_set = 0;
   long n = 0;
   long sn = 0;
   int radix = 10;
@@ -3728,38 +3729,79 @@ read_huge_number (char **pp, int end, in
     }
 
   twos_complement_representation = radix == 8 && twos_complement_bits > 0;
+  if (twos_complement_representation)
+    {
+      int len, indpos, i, sign_byte, sign_bit_pos;
+      char *endpos;
+      len = strlen(p);
+      endpos = strchr(p,end);
+      if ((endpos != NULL) && (len > endpos - p))
+	len = endpos -p -1;
+      indpos = 0;
+      sign_byte = twos_complement_bits / 3;
+      sign_bit_pos = (twos_complement_bits % 3 + 2) % 3;
+      for (i=len; i>=0; i--)
+	{
+	  c = p[i];
+	  if (c >= '0' && c < ('0' + radix))
+	    {
+	      /* This is a valid numeral.  */
+	      if (indpos == sign_byte)
+		{
+		  int val, uval;
+		  val = c - '0';
+		  if (val & (1 << sign_bit_pos))
+		    {
+		      /* This numeral is signed.  */
+		      sign *= -1;
+		      sign_bit_set = 1;
+		      /* Clear sign bit.  */ 
+		      uval = (val - (1 << sign_bit_pos));
+		      /* No bit higher than the sign bit should be set. */
+		      if (uval > (1 << sign_bit_pos))
+			{
+			  nbits = -1;
+			  if (bits != NULL)
+			    *bits = nbits;
+			  return 0;
+			}
+		      c = '0' + uval;	
+		      p[i] = c;
+			
+		    }
+		}
+	      else if (indpos > sign_byte)
+		{
+		  /* allow only zeroes above sign_byte.  */
+		  if (c != '0')
+		    {
+		      nbits = -1;
+		      if (bits != NULL)
+			*bits = nbits;  
+		      return 0;
+		    };
+		}
+	      indpos++;
+	   }
+	 else
+	   {
+	     nbits = -1;
+	     if (bits != NULL)
+	       *bits = nbits;
+	     return 0;
+	   } 
+       }
+  
+    }
   upper_limit = LONG_MAX / radix;
 
   while ((c = *p++) >= '0' && c < ('0' + radix))
     {
       if (n <= upper_limit)
         {
-          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));
-                }
-              else
-                {
-                  sn *= radix;
-                  sn += c - '0';
-                }
-
-              if (sn < 0)
-                n = -sn;
-            }
-          else
-            {
-              /* unsigned representation */
-              n *= radix;
-              n += c - '0';		/* FIXME this overflows anyway */
-            }
+          /* unsigned representation */
+          n *= radix;
+          n += c - '0';		/* FIXME this overflows anyway */
         }
       else
         overflow = 1;
@@ -3799,7 +3841,7 @@ read_huge_number (char **pp, int end, in
   *pp = p;
   if (overflow)
     {
-      if (nbits == 0)
+      if ((nbits == 0) && (radix != 8))
 	{
 	  /* Large decimal constants are an error (because it is hard to
 	     count how many bits are in them).  */
@@ -3812,6 +3854,8 @@ read_huge_number (char **pp, int end, in
          the number of bits.  */
       if (sign == -1)
 	++nbits;
+      if (sign_bit_set)
+	nbits = twos_complement_bits;
       if (bits)
 	*bits = nbits;
     }
@@ -3819,10 +3863,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;