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

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

[Pierre Muller]

[-- 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;

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

[Pedro Alves]

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

Hi,

This afects Cygwin badly, as it still uses stabs by default.
I've seen this problem here, but thought it was caused by a
few gcc changes I was making ...

Pierre Muller wrote:
>   As I was unable to understand the current implementation of
> the twos_complement_representation

I can't really see how it was supposed to work either.
Parsing 000000000 is broken currently.

> I rewrote it almost completely.

You are only counting the bits.  This function should
return the parsed number if it fits in a long:

/* If TWOS_COMPLEMENT_BITS is set to a strictly positive value and if
     the number is represented in an octal representation, assume that
     it is represented in a 2's complement representation with a size of
     TWOS_COMPLEMENT_BITS.

     If the number fits in a long, set *BITS to 0 and return the value.
     If not, set *BITS to be the number of bits in the number and return 0.

     If encounter garbage, set *BITS to -1 and return 0.  */


>   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.
> 

Thanks for pointing in the right direction!

(Forgive me for counter patching, but I had started on this
  here too yesterday :( )

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.

Just tested that it also fixes the problem.  I'll give it a
testsuite spin for C/C++ on Cygwin tomorrow, but the testcase
that suposedly tests this is in ADA, which I don't have a
setup for...

Cheers,
Pedro Alves



[-- Attachment #2: stabsread.c.diff --]
[-- Type: text/x-diff, Size: 2477 bytes --]

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

	* stabsread.c (read_huge_number): Remove special parsing of octal
	two's complement representation.  If just parsed a negative number
	in octal two's complement representation, sign extend the result
	to a long.

Index: stabsread.c
===================================================================
RCS file: /cvs/src/src/gdb/stabsread.c,v
retrieving revision 1.95
diff -u -p -r1.95 stabsread.c
--- stabsread.c	10 Aug 2007 22:08:22 -0000	1.95
+++ stabsread.c	22 Sep 2007 02:28:15 -0000
@@ -3737,32 +3737,9 @@ read_huge_number (char **pp, int end, in
     {
       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;
@@ -3823,7 +3800,25 @@ read_huge_number (char **pp, int end, in
       if (bits)
 	*bits = 0;
       if (twos_complement_representation)
-        return sn;
+	{
+	  if (nbits == twos_complement_bits)
+	    {
+	      /* N is signed.  TWOS_COMPLEMENT_BITS may be less than
+		 what fits in a long.  Since we can't assume a host
+		 with two's complement long, extract the module value
+		 of N, and multiply it with -1.
+
+		 eg: with TWOS_COMPLEMENT_BITS == 16, and sizeof long == 32,
+		 0x00008111 => 0xffff7eef => 0x00007eef => 0xffff8111.  */
+
+	      unsigned long l = (unsigned long) n;
+	      l = ~l + 1;
+	      l &= (1L << twos_complement_bits) - 1;
+	      l *= -1;
+	      n = l;
+	    }
+	  return n;
+	}
       else
         return n * sign;
     }

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

[Joel Brobecker]

> Just tested that it also fixes the problem.  I'll give it a
> testsuite spin for C/C++ on Cygwin tomorrow, but the testcase
> that suposedly tests this is in ADA, which I don't have a
> setup for...

Which testcase is this? I can give it a whirl and see what happens.

-- 
Joel

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

[Pedro Alves]

Joel Brobecker wrote:
>> Just tested that it also fixes the problem.  I'll give it a
>> testsuite spin for C/C++ on Cygwin tomorrow, but the testcase
>> that suposedly tests this is in ADA, which I don't have a
>> setup for...
> 
> Which testcase is this? I can give it a whirl and see what happens.
> 

gdb.ada/fixed_points.exp

Original patch:
http://www.cygwin.com/ml/gdb-patches/2004-11/msg00448.html

Testcase:
http://www.cygwin.com/ml/gdb-patches/2004-11/msg00467.html

Cheers,
Pedro Alves

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

[Pedro Alves]

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

Pedro Alves 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.
> 
> Just tested that it also fixes the problem.  I'll give it a
> testsuite spin for C/C++ on Cygwin tomorrow, but the testcase
> that suposedly tests this is in ADA, which I don't have a
> setup for...
> 

Just tested the slightly updated patch on Cygwin, and it doesn't
bring about any new problem.

I also stepped through read_huge_number using the attached
read_huge_number.s as a test.  I don't know how to force gcc to
output a range type in octal that triggers
'TWOS_COMPLEMENT_BITS < sizeof (long) * HOST_CHAR_BIT', so I've hacked
the stabs myself:

Replaced:
.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

By:
.stabs	"short int:t(0,8)=@s16;r(0,8);0100000;077777;",128,0,0,0
.stabs	"short unsigned int:t(0,9)=@s16;r(0,9);0000000;0177777;",128,0,0,0

'ptype main' triggers the read_range_type call, which calls
read_huge_number twice for the range.

I've confirmed that we read 0100000 as (long)-32768.

Don't know how to convert this to a testcase.
'maint print type short int' doesn't show these ranges,
are they stored anywhere?

Cheers,
Pedro Alves


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

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

	* stabsread.c (read_huge_number): Remove special parsing of octal
	two's complement representation.  If just parsed a negative number
	in octal two's complement representation, sign extend the result
	to a long.

---
 gdb/stabsread.c |   49 ++++++++++++++++++++++---------------------------
 1 file changed, 22 insertions(+), 27 deletions(-)

Index: src/gdb/stabsread.c
===================================================================
--- src.orig/gdb/stabsread.c	2007-09-22 17:09:20.000000000 +0100
+++ src/gdb/stabsread.c	2007-09-22 18:19:26.000000000 +0100
@@ -3734,32 +3734,9 @@ read_huge_number (char **pp, int end, in
     {
       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;
@@ -3820,7 +3797,25 @@ read_huge_number (char **pp, int end, in
       if (bits)
 	*bits = 0;
       if (twos_complement_representation)
-        return sn;
+	{
+	  if (n & 1L << (twos_complement_bits - 1))
+	    {
+	      /* N is signed.  TWOS_COMPLEMENT_BITS may be less than
+		 what fits in a long.  Since we can't assume a host
+		 with two's complement long, extract the module value
+		 of N, and multiply it with -1.
+
+		 eg: with TWOS_COMPLEMENT_BITS == 16, and sizeof long == 32,
+		 0x00008111 => 0xffff7eef => 0x00007eef => 0xffff8111.  */
+
+	      unsigned long l = (unsigned long) n;
+	      l = ~l + 1;
+	      l &= (1L << twos_complement_bits) - 1;
+	      n = (long) l;
+	      n *= -1;
+	    }
+	  return n;
+	}
       else
         return n * sign;
     }


[-- Attachment #3: read_huge_number.s --]
[-- Type: text/plain, Size: 1475 bytes --]

	.stabs	"read_huge_number.s",100,0,0,Ltext0
	.text
Ltext0:
	.stabs	"gcc2_compiled.",60,0,0,0
	
	.stabs	"short int:t(0,8)=@s16;r(0,8);0100000;077777;",128,0,0,0
	.stabs	"short unsigned int:t(0,9)=@s16;r(0,9);0000000;0177777;",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	"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

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

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

[Joel Brobecker]

> Don't know how to convert this to a testcase.
> 'maint print type short int' doesn't show these ranges,
> are they stored anywhere?

Can we perhaps handle stabs testcases the same way as we do for
dwarf testcases? We provide an assembly file. The assembly file
isn't going to work on all platforms, but we'll cover enough
to make the testcase useful.

-- 
Joel

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

[Daniel Jacobowitz]

On Sat, Sep 22, 2007 at 03:58:08PM -0700, Joel Brobecker wrote:
> > Don't know how to convert this to a testcase.
> > 'maint print type short int' doesn't show these ranges,
> > are they stored anywhere?
> 
> Can we perhaps handle stabs testcases the same way as we do for
> dwarf testcases? We provide an assembly file. The assembly file
> isn't going to work on all platforms, but we'll cover enough
> to make the testcase useful.

There is already a gdb.stabs directory.  The question is how to
verify that GDB did the right thing...

Can we trigger this code path on the bounds for an array?

-- 
Daniel Jacobowitz
CodeSourcery

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

[Joel Brobecker]

> There is already a gdb.stabs directory.  The question is how to
> verify that GDB did the right thing...

I was thinking that not having the error that was originially reported
would a good first step. If we could simply store the assembly file
produced by Pierre's example and then verify that "ptype u" works...
But now that I actually tried Pierre's example, I get no error at all,
even after having verified that I do get an @s64-etc stabs. I'm not sure
why...

-- 
Joel

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

[Pedro Alves]

Joel Brobecker wrote:
> 
> I was thinking that not having the error that was originially reported
> would a good first step. If we could simply store the assembly file
> produced by Pierre's example and then verify that "ptype u" works...
> But now that I actually tried Pierre's example, I get no error at all,
> even after having verified that I do get an @s64-etc stabs. I'm not sure
> why...
> 

Are you on an LP64 host?

Changing this in read_huge_number:

-upper_limit = LONG_MAX / radix;
+upper_limit = 0x7fffffff / radix;

... will probably trigger it.

Cheers,
Pedro Alves

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

[Pedro Alves]

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

Daniel Jacobowitz wrote:

> There is already a gdb.stabs directory.  

... and all this time, it wasn't running on Cygwin, although
the official compiler uses stabs.

This patch fixes it.

This is what I get from running weird.exp

# of expected passes            190
# of expected failures          7

This is what I get from running exclfwd.exp

# of expected passes            1
# of known failures             2

Both with and without the read_huge_number patch.

weird.def seems like a good place to add a new testcase.

OK?

Cheers,
Pedro Alves


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

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

	* configure.ac: Do gdb.stabs tests by default on Cygwin and MinGW
	targets.
	* configure: Regenerate.

---
 gdb/testsuite/configure    |    2 ++
 gdb/testsuite/configure.ac |    2 ++
 2 files changed, 4 insertions(+)

Index: src/gdb/testsuite/configure
===================================================================
--- src.orig/gdb/testsuite/configure	2007-03-31 18:35:56.000000000 +0100
+++ src/gdb/testsuite/configure	2007-09-23 12:31:32.000000000 +0100
@@ -1457,6 +1457,8 @@ case $target in
  | *-sun-*        \
  | hppa*-*-*      \
  | *-*-elf*       \
+ | *-pc-cygwin*   \
+ | *-pc-mingw*    \
  )
     with_stabs=yes ;;
  *)
Index: src/gdb/testsuite/configure.ac
===================================================================
--- src.orig/gdb/testsuite/configure.ac	2007-09-22 17:09:56.000000000 +0100
+++ src/gdb/testsuite/configure.ac	2007-09-23 12:29:14.000000000 +0100
@@ -46,6 +46,8 @@ case $target in
  | *-sun-*        \
  | hppa*-*-*      \
  | *-*-elf*       \
+ | *-pc-cygwin*   \
+ | *-pc-mingw*    \
  )
     with_stabs=yes ;;
  *)

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

[Daniel Jacobowitz]

On Sun, Sep 23, 2007 at 12:38:40PM +0100, Pedro Alves wrote:
> 2007-09-23  Pedro Alves  <pedro_alves@portugalmail.pt>
> 
> 	* configure.ac: Do gdb.stabs tests by default on Cygwin and MinGW
> 	targets.
> 	* configure: Regenerate.

OK.  Although, do you really need to check for 'pc' there?

-- 
Daniel Jacobowitz
CodeSourcery

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

[Pedro Alves]

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

Daniel Jacobowitz wrote:
> On Sun, Sep 23, 2007 at 12:38:40PM +0100, Pedro Alves wrote:
>> 2007-09-23  Pedro Alves  <pedro_alves@portugalmail.pt>
>>
>> 	* configure.ac: Do gdb.stabs tests by default on Cygwin and MinGW
>> 	targets.
>> 	* configure: Regenerate.
> 
> OK.  Although, do you really need to check for 'pc' there?
> 

No, not really.

Committed as attached.

Cheers,
Pedro Alves


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

Index: ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/ChangeLog,v
retrieving revision 1.1453
diff -u -p -r1.1453 ChangeLog
--- ChangeLog	23 Sep 2007 07:56:22 -0000	1.1453
+++ ChangeLog	23 Sep 2007 13:52:02 -0000
@@ -1,3 +1,9 @@
+2007-09-23  Pedro Alves  <pedro_alves@portugalmail.pt>
+
+	* configure.ac: Do gdb.stabs tests by default on Cygwin and MinGW
+	targets.
+	* configure: Regenerate.
+
 2007-09-23  Vladimir Prus  <vladimir@codesourcery.com>
 	
 	* gdb.base/annota1.exp: Adjust for 'info break'
Index: configure
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/configure,v
retrieving revision 1.23
diff -u -p -r1.23 configure
--- configure	23 Jan 2007 17:11:54 -0000	1.23
+++ configure	23 Sep 2007 13:52:05 -0000
@@ -1457,6 +1457,8 @@ case $target in
  | *-sun-*        \
  | hppa*-*-*      \
  | *-*-elf*       \
+ | *-*-cygwin*    \
+ | *-*-mingw*     \
  )
     with_stabs=yes ;;
  *)
Index: configure.ac
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/configure.ac,v
retrieving revision 1.7
diff -u -p -r1.7 configure.ac
--- configure.ac	23 Aug 2007 17:58:44 -0000	1.7
+++ configure.ac	23 Sep 2007 13:52:05 -0000
@@ -46,6 +46,8 @@ case $target in
  | *-sun-*        \
  | hppa*-*-*      \
  | *-*-elf*       \
+ | *-*-cygwin*    \
+ | *-*-mingw*     \
  )
     with_stabs=yes ;;
  *)

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

[Pedro Alves]

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

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


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

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);
     }
 



[-- Attachment #3: read_huge_number.s --]
[-- Type: text/plain, Size: 1822 bytes --]

	.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:

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

[Pierre Muller]

  Did you change the read_huge_number.s file manually?
It is as if there is an typing error for the
u16 type definition.
	.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
the ';' before 'r(0,25)' should be an equal sign, no?

  One difference between my patch and yours is
that I think that your patch will mishandle 
any octal number having more digits than needed,
because you are always considering that the 
first char after the leading zero (to trigger octal notation)
contains the sign bit.
  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.

  I agree that there are normally no reasons to have more digits,
but more  leading zeroes should not lead to an error in the parsing
and any bit set higher that this should trigger an error.

  By the way, why is read_huge_number returning a 'long'?
It should be much better to return a LONGEST 
 LONGEST_MAX also exists, so switching 
the type of that function to read_huge_number
would allow to get correct results much more often.

  The main interest would be to really get the 
real upper and lower bounds of range type
even if they do not fit into 'long' type.
  These fields are currently put into
TYPE_FIELD_BITPOS which is a 'long' type
but this is in a union so adding one more field 
on type 'LONGEST' and name 'bound',
adding a TYPE_FIELD_BOUND macro would be 
easier to read but would increase the 'field_location'
element of the 'type' structure.
  Another solution would be to
create a pointer to a 'LONGEST' value,
which would remove an increase of the general
'type' struct size.

Pierre Muller

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

[Pedro Alves]

Pierre Muller wrote:
>   Did you change the read_huge_number.s file manually?

Yes.

> It is as if there is an typing error for the
> u16 type definition.
>         .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
> the ';' before 'r(0,25)' should be an equal sign, no?
>

Looks like so.  You sharp eyed :)

>   One difference between my patch and yours is
> that I think that your patch will mishandle
> any octal number having more digits than needed,
> because you are always considering that the
> first char after the leading zero (to trigger octal notation)
> contains the sign bit.
>   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...
One can do it upfront like your patch does, or checking for
'n bits parsed' > size_type (is size_type > 0) after the parse loop should be
enough, I guess.

>   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 ...

I didn't mention why I didn't take your approach, but followed what I
believe the
original code intended to do:
  - It changes the input string.  I don't believe that is correct.
Read only data
  comes to mind.
  - It parses 01777777777777777776340 as an overflow (doesn't it?)

Cheers,
Pedro Alves

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

[Pierre Muller]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Pedro Alves
> Sent: Monday, September 24, 2007 12:22 PM
> To: Pierre Muller
> Cc: gdb-patches@sourceware.org
> Subject: Re: [RFC] Stabs parsing regression from GDB 6.6 to GDB 6.6.90
> 
> Pierre Muller wrote:
> >   Did you change the read_huge_number.s file manually?
> 
> Yes.
> 
> > It is as if there is an typing error for the
> > u16 type definition.
> >         .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
> > the ';' before 'r(0,25)' should be an equal sign, no?
> >
> 
> Looks like so.  You sharp eyed :)
> 
> >   One difference between my patch and yours is
> > that I think that your patch will mishandle
> > any octal number having more digits than needed,
> > because you are always considering that the
> > first char after the leading zero (to trigger octal notation)
> > contains the sign bit.
> >   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 tested the parser at command line and it seems like there 
is the same kind of flaw:
[on a i686-pc-cygwin 
(gdb) p 0x8000000000000001
$26 = 9223372036854775809
(gdb) p 0x80000000000000001
Numeric constant too large.
(gdb) p 0x800000000000000001
$27 = 1

  Even though the parser does not use read_huge_number
function (which is stabs specific) there is a similar problem 
somewhere in the parsing of constants.
  I did not get any error nor complaint for the last input...

> One can do it upfront like your patch does, or checking for
> 'n bits parsed' > size_type (is size_type > 0) after the parse loop
> should be
> enough, I guess.

  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 ...
> 
> I didn't mention why I didn't take your approach, but followed what I
> believe the
> original code intended to do:
>   - It changes the input string.  I don't believe that is correct.
> Read only data
>   comes to mind.
>   - It parses 01777777777777777776340 as an overflow (doesn't it?)
  I did not check, but I agree with you that my patch
was wrong on that part, I would just like to have the
check for the exact position of the sign bit and
that there are no bits set higher than that one.


Pierre Muller

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

[Pedro Alves]

[-- 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:

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

[Pedro Alves]

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

Hi,

Here is a new slightly updated patch, and a companion test case
patch, to add long long tests to whatis.exp.  I didn't add the
same tests to ptype.exp, as it has a comment about basic types tests
going into whatis.exp.

This is what one gets before the patch on native cygwin:

FAIL: gdb.base/whatis.exp: whatis using typedef type name

                 === gdb Summary ===

# of expected passes            73
# of unexpected failures        1

----

With the whatis.exp patch installed, one gets:

FAIL: gdb.base/whatis.exp: whatis unsigned long long
FAIL: gdb.base/whatis.exp: whatis unsigned long array
FAIL: gdb.base/whatis.exp: whatis unsigned long long pointer
FAIL: gdb.base/whatis.exp: whatis unsigned long long function
FAIL: gdb.base/whatis.exp: whatis using typedef type name

                 === gdb Summary ===

# of expected passes            77
# of unexpected failures        5

----

The read_huge_number fix then brings the failures back down:

FAIL: gdb.base/whatis.exp: whatis using typedef type name

                 === gdb Summary ===

# of expected passes            81
# of unexpected failures        1

----

Is this enough for now?
Are the patches OK for head/branch?

gdb/
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.

testsuite/
2007-09-26  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_sslong_long_addr, v_long_long_pointer)
	(v_signed_long_long_pointer, v_unsigned_long_long_pointer): 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.



Cheers,
Pedro Alves

[-- Attachment #2: read_huge_number2.diff --]
[-- Type: text/x-diff, Size: 5161 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 |   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 #3: whatis_long_long.diff --]
[-- Type: text/x-diff, Size: 8164 bytes --]

2007-09-26  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_sslong_long_addr, v_long_long_pointer)
	(v_signed_long_long_pointer, v_unsigned_long_long_pointer): 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-09-25 23:56:30.000000000 +0100
+++ src/gdb/testsuite/gdb.base/whatis.c	2007-09-25 23:58:40.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_addr a_sslong_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-09-25 23:56:30.000000000 +0100
+++ src/gdb/testsuite/gdb.base/whatis.exp	2007-09-26 00:26:04.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" \

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

[Pierre Muller]

   
> Is this enough for now?
> Are the patches OK for head/branch?

  I definitively think that this patch should go both into
head and branch, but it needs approval from someone else...

Elena Zannoni is marked as stabs reader maintainer,
but I didn't see any email from her for a while.
Is she still involved in GDB?


Pierre Muller

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

[Jim Blandy]

"Pierre Muller" <muller at ics.u-strasbg.fr> writes:
>> Is this enough for now?
>> Are the patches OK for head/branch?
>
>   I definitively think that this patch should go both into
> head and branch, but it needs approval from someone else...
>
> Elena Zannoni is marked as stabs reader maintainer,
> but I didn't see any email from her for a while.
> Is she still involved in GDB?

She's at Oracle now; I think the address above is hers.  Elena, what
are you up to these days?

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

[Joel Brobecker]

> I definitively think that this patch should go both into
> head and branch, but it needs approval from someone else...

Unless someone like Elena who knows this code very well says it's safe,
I would prefer if the patch didn't get applied to the branch. I'm a bit
concerned about it given the few iterations that you both had to go
through in order to get it to that point. It just shows that it is
not obvious.

-- 
Joel

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

[Daniel Jacobowitz]

On Wed, Oct 03, 2007 at 11:41:56AM -0700, Joel Brobecker wrote:
> > I definitively think that this patch should go both into
> > head and branch, but it needs approval from someone else...
> 
> Unless someone like Elena who knows this code very well says it's safe,
> I would prefer if the patch didn't get applied to the branch. I'm a bit
> concerned about it given the few iterations that you both had to go
> through in order to get it to that point. It just shows that it is
> not obvious.

I think it's a bad idea to ship a release of GDB with a new bug that
we know about, which wasn't present in the previous release.

I read through the patch.  It seems fine to me, though I am mostly
trusting Pedro, Pierre, and their test cases.

-- 
Daniel Jacobowitz
CodeSourcery

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

[Joel Brobecker]

> I think it's a bad idea to ship a release of GDB with a new bug that
> we know about, which wasn't present in the previous release.

I try to maintain the right balance. A hard-to-fix minor and infrequent
regression is probably better than a fix that might cause other
regressions. The stabsread patch is somewhere in the middle between
and obvious fix, and the hard-to-fix, so it was not obvious to me
which decision to take.

I also trust Pedro and Pierre, this is not an issue for me. But I would
like an extra pair of eyes like yours to confirm that this patch is safe
for the branch. With that in mind, it becomes perfectly reasonable
to put this patch in the branch as well.

Note that the 6.7 release is currently scheduled in a week from now.
If this is an important issue, we might have to delay it if the patch
hasn't been approved at that time.

-- 
Joel

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

[Pedro Alves]

Daniel Jacobowitz wrote:
> On Wed, Oct 03, 2007 at 11:41:56AM -0700, Joel Brobecker wrote:
>>> I definitively think that this patch should go both into
>>> head and branch, but it needs approval from someone else...
>> Unless someone like Elena who knows this code very well says it's safe,
>> I would prefer if the patch didn't get applied to the branch. I'm a bit
>> concerned about it given the few iterations that you both had to go
>> through in order to get it to that point. It just shows that it is
>> not obvious.
> 

That's because the original code to read signed octals was never
triggered from day 1.  I suspect it was due to a bad/incomplete
merge from whatever tree it came from, and a testcase that
doesn't really trigger it.

> I think it's a bad idea to ship a release of GDB with a new bug that
> we know about, which wasn't present in the previous release.
> 

Another option is to revert this on the branch:

2007-08-04  Michael Snyder

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

Since nobody complained until then (except for Coverity).  The signed
octal parsing was dead code Before Michail's Patch (BMP), and is broken
after it.  BMP, the basic types happened to be recorded with the
correct bit sizes, but ADA ranges that have a negative edge (-50, 50)
would be misrecorded .  That was what the original code attempted to
fix, but I don't really know where it makes a difference.  Perhaps
a print command which sets a value whose type is covered by a
range, with an out-of-range value, will trigger a warning:

print var = -51
warning: -51 is out of range for type x [-50, 50].

If gdb doesn't do it yet, it could be useful anyway.

What I know of ADA is what one can learn from a 10 minutes HOWTO reading.

> I read through the patch.  It seems fine to me, though I am mostly
> trusting Pedro, Pierre, and their test cases.
> 

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

[Daniel Jacobowitz]

On Wed, Oct 03, 2007 at 10:34:17PM +0100, Pedro Alves wrote:
> Another option is to revert this on the branch:
> 
> 2007-08-04  Michael Snyder
> 
> 	* stabsread.c (read_huge_number): Attempt to compute value before
> 	values that it depends on.

That seems sensible to me.

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.

-- 
Daniel Jacobowitz
CodeSourcery

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

[Pedro Alves]

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.
> 

It will have to be more like 5 or 6, since I'm travelling until monday.
Unless someone cares to take it over, of course.  :-)

Cheers,
Pedro Alves

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

[Pedro Alves]

[-- 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" \

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

[Pierre Muller]

Pedro,

you patch to main leads to a warning when compiling stabsread.o
because sign_bit might be used uninitialized.
  Even though this is a false alarm,
as sign_bit is set if two_complement_representation is,
and used later only if two_complement_representation is set;
we need to correct this, as it leads to a compilation failure if
-Werror is used.

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/../includ
e   -DMI_OUT=1 -DTUI=1  -Wall -Wdeclaration-after-statement -Wpointer-arith
-Wfo
rmat-nonliteral -Wno-unused -Wno-switch -Wno-char-subscripts -Werror
version.c
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/../includ
e   -DMI_OUT=1 -DTUI=1  -Wall -Wdeclaration-after-statement -Wpointer-arith
-Wfo
rmat-nonliteral -Wno-unused -Wno-switch -Wno-char-subscripts -Werror
../../src/g
db/stabsread.c
../../src/gdb/stabsread.c: In function `read_huge_number':
../../src/gdb/stabsread.c:3736: warning: 'sign_bit' might be used
uninitialized
in this function
make[1]: *** [stabsread.o] Error 1
make[1]: Leaving directory `/usr/local/src/cvs/build/gdb'
make: *** [all-gdb] Error 2

Declaring it with initial value = 0
is enough to fix the compilation failure.

Pierre

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

[Pedro Alves]

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

Pierre Muller wrote:
> Pedro,
> 
> you patch to main leads to a warning when compiling stabsread.o
> because sign_bit might be used uninitialized.
>   Even though this is a false alarm,
> as sign_bit is set if two_complement_representation is,
> and used later only if two_complement_representation is set;
> we need to correct this, as it leads to a compilation failure if
> -Werror is used.
> 

Ah, sorry and thanks.  I built with -O0 only ...

Checked the attached in, as obvious.

-- 

Cheers,
Pedro Alves


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

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

	* stabsread.c (read_huge_number): Initialize local variable to 0.

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

Index: src/gdb/stabsread.c
===================================================================
--- src.orig/gdb/stabsread.c	2007-10-09 09:58:42.000000000 +0100
+++ src/gdb/stabsread.c	2007-10-09 09:58:52.000000000 +0100
@@ -3703,7 +3703,7 @@ read_huge_number (char **pp, int end, in
 {
   char *p = *pp;
   int sign = 1;
-  int sign_bit;
+  int sign_bit = 0;
   long n = 0;
   int radix = 10;
   char overflow = 0;

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

[Joel Brobecker]

> Just tested that it also fixes the problem.  I'll give it a
> testsuite spin for C/C++ on Cygwin tomorrow, but the testcase
> that suposedly tests this is in ADA, which I don't have a
> setup for...

I double-check that the testcase still passes after having applied
your patch.

-- 
Joel

PS: Just FYI, it's "Ada". The language names comes from the name of a lady.

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

[Pierre Muller]

  

> -----Original Message-----
> From: Pedro Alves [mailto:alves.ped@gmail.com] On Behalf Of Pedro Alves
> Sent: Saturday, September 22, 2007 5:06 AM
> To: Pierre Muller
> Cc: gdb-patches@sourceware.org
> Subject: Re: [RFC] Stabs parsing regression from GDB 6.6 to GDB 6.6.90
> 
> Hi,
> 
> This afects Cygwin badly, as it still uses stabs by default.
> I've seen this problem here, but thought it was caused by a few gcc
> changes I was making ...
> 
> Pierre Muller wrote:
> >   As I was unable to understand the current implementation of the
> > twos_complement_representation
> 
> I can't really see how it was supposed to work either.
> Parsing 000000000 is broken currently.
> 
> > I rewrote it almost completely.
> 
> You are only counting the bits.  This function should return the parsed
> number if it fits in a long:

  My idea was to clear the sign bit and
let the normal code handle the parsing of the value,
but I completely forgot that two complement notation
is not the same as the multiplication by -1.
  So I should probably have let the sign bit and parse it
as an unsigned integer.


> /* If TWOS_COMPLEMENT_BITS is set to a strictly positive value and if
>      the number is represented in an octal representation, assume that
>      it is represented in a 2's complement representation with a size
> of
>      TWOS_COMPLEMENT_BITS.
> 
>      If the number fits in a long, set *BITS to 0 and return the value.
>      If not, set *BITS to be the number of bits in the number and
> return 0.
> 
>      If encounter garbage, set *BITS to -1 and return 0.  */
> 
> 
> >   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.
> >
> 
> Thanks for pointing in the right direction!
> 
> (Forgive me for counter patching, but I had started on this
>   here too yesterday :( )

 As nobody reacted on my first email,
I thought that nobody cared.
  I am glad you propose a new patch.
As you already proposed a second one,
I will try to comment on that one directly.

Thanks

Pierre Muller