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

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

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