From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30024 invoked by alias); 24 Sep 2007 00:43:18 -0000 Received: (qmail 30011 invoked by uid 22791); 24 Sep 2007 00:43:16 -0000 X-Spam-Check-By: sourceware.org Received: from fk-out-0910.google.com (HELO fk-out-0910.google.com) (209.85.128.187) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 24 Sep 2007 00:43:11 +0000 Received: by fk-out-0910.google.com with SMTP id 26so1403199fkx for ; Sun, 23 Sep 2007 17:43:08 -0700 (PDT) Received: by 10.82.152.16 with SMTP id z16mr8227913bud.1190594588422; Sun, 23 Sep 2007 17:43:08 -0700 (PDT) Received: from ?88.210.68.113? ( [88.210.68.113]) by mx.google.com with ESMTPS id c5sm7088177nfi.2007.09.23.17.43.02 (version=TLSv1/SSLv3 cipher=RC4-MD5); Sun, 23 Sep 2007 17:43:05 -0700 (PDT) Message-ID: <46F707C3.1090105@portugalmail.pt> Date: Mon, 24 Sep 2007 00:43:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; pt-BR; rv:1.8.1.6) Gecko/20070728 Thunderbird/2.0.0.6 Mnenhy/0.7.5.0 MIME-Version: 1.0 To: Pierre Muller CC: gdb-patches@sourceware.org Subject: Re: [RFC] Stabs parsing regression from GDB 6.6 to GDB 6.6.90 References: <000e01c7fb9b$22e600f0$68b202d0$@u-strasbg.fr> <000601c7fc25$98110430$c8330c90$@u-strasbg.fr> <46F486B4.6050900@portugalmail.pt> <46F56F04.6070601@portugalmail.pt> In-Reply-To: <46F56F04.6070601@portugalmail.pt> Content-Type: multipart/mixed; boundary="------------030403030403000200080201" X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2007-09/txt/msg00332.txt.bz2 This is a multi-part message in MIME format. --------------030403030403000200080201 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 3376 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 --------------030403030403000200080201 Content-Type: text/x-diff; name="read_huge_number2.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="read_huge_number2.diff" Content-length: 4779 2007-09-24 Pedro Alves * 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); } --------------030403030403000200080201 Content-Type: text/plain; name="read_huge_number.s" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="read_huge_number.s" Content-length: 1822 .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: --------------030403030403000200080201--