Dead code in winedbg

[Francois Gouget]

Hi,

I have noticed that expr_alloc_uconstant() is unused in winedbg.
Is that normal?

It causes all of the code in the attached patch to essentially be dead 
code since expr.type is never set to EXPR_TYPE_U_CONST. Should we just 
remove all that code? Just the expr_alloc_uconstant() function? Or 
is this unused just because of a bug? Or maybe it will be used 'soon'?

[Eric Pouech]

Francois Gouget a écrit :
> Hi,
>
> I have noticed that expr_alloc_uconstant() is unused in winedbg.
> Is that normal?

the main point is that the lexer only returns signed integers, while it 
should return both signed and unsigned integers
therefore this code was left to handle possibly unsigned:s from 
0x80000000 up to 0xfffffffff (which current code doesn't)
the proper fix would be to fix the lexer to return both signed and 
unsigned integers

for example:
WineDbg> p 0x80000000
-2147483648
while it should be
2147483648

A+

[Francois Gouget]

On Wed, 14 Jan 2009, Eric Pouech wrote:
> Francois Gouget a écrit :
> > Hi,
> >
> > I have noticed that expr_alloc_uconstant() is unused in winedbg.
> > Is that normal?
>
> the main point is that the lexer only returns signed integers, while it 
> should return both signed and unsigned integers

Actually, as far as I can tell, the lexer can only match unsigned 
integers:

DIGIT	   [0-9]
HEXDIGIT   [0-9a-fA-F]
"0x"{HEXDIGIT}+    { sscanf(yytext, "%x", &dbg_lval.integer); return tNUM; }
{DIGIT}+           { sscanf(yytext, "%d", &dbg_lval.integer); return tNUM; }

Both 'regexps' only match unsigned integers. And yet they are scanned 
with %d instead of %u. And then this continues in the grammar:

tNUM                      { $$ = expr_alloc_sconstant($1); }

So maybe we should treat all these as unsigned, use 
expr_alloc_uconstant(), and then it is expr_alloc_sconstant() that would 
be unused?

Or should we really accept negative integers in the lexer?

[Eric Pouech]

2009/1/15 Francois Gouget
> On Wed, 14 Jan 2009, Eric Pouech wrote:
> > Francois Gouget a écrit :
> > > Hi,
> > >
> > > I have noticed that expr_alloc_uconstant() is unused in winedbg.
> > > Is that normal?
> >
> > the main point is that the lexer only returns signed integers, while it
> > should return both signed and unsigned integers
>
> Actually, as far as I can tell, the lexer can only match unsigned
> integers:
>
> DIGIT      [0-9]
> HEXDIGIT   [0-9a-fA-F]
> "0x"{HEXDIGIT}+    { sscanf(yytext, "%x", &dbg_lval.integer); return tNUM;
> }
> {DIGIT}+           { sscanf(yytext, "%d", &dbg_lval.integer); return tNUM;
> }
>
> Both 'regexps' only match unsigned integers. And yet they are scanned
> with %d instead of %u. And then this continues in the grammar:
>  tNUM                      { $$ = expr_alloc_sconstant($1); }
>
> So maybe we should treat all these as unsigned, use
> expr_alloc_uconstant(), and then it is expr_alloc_sconstant() that would
> be unused?
>
> Or should we really accept negative integers in the lexer?

actually, doing what you suggest would help the lexer, but will put the
issue a bit later in the expression computation package, where only signed
ints are computed (and moreover that code is likely to be broken on a 64bit
CPU)
so, moving to a unsigned requipes also to fix expr.c
A+
> --
>
> Eric Pouech