* Suggested warning: "negating an expression of unsigned type does not yield a negative value"
@ 2003-10-06 12:46 Falk Hueffner
2003-10-06 16:00 ` Joe Buck
0 siblings, 1 reply; 6+ messages in thread
From: Falk Hueffner @ 2003-10-06 12:46 UTC (permalink / raw)
To: gcc
Hi,
I just found yet another bug of the kind:
int f (int *p, unsigned x) { return p[-x]; }
which only manifests on 64 bit platforms, because most (all?)
platforms have wrapping address arithmetic.
So I was wondering about a general warning about negating unsigned
values, since I couldn't really think of a legitimate application. A
quick check with the gcc source turned up:
gengtype-lex.l:
char *namestart;
size_t namelen;
[...]
for (namelen = 1; !ISSPACE (namestart[-namelen]); namelen++)
This looks actually invalid to me, although it will probably work
everywhere.
In fold_const.c, there's
case RSHIFT_EXPR:
int2l = -int2l;
also "invalid but works" since it's later passed to a function taking
int.
Then there's everybody's favourite idiom "x &= -x", but it can be
expressed clearer as "x &= ~x + 1".
Then there's constant folding in neg_double. Hm. Damn. I can't think
of any reformulation which does not obscure the code. So this warning
should probably not be turned on by -W. But it seems generally useful.
Any opinions?
--
Falk
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Suggested warning: "negating an expression of unsigned type does not yield a negative value"
2003-10-06 12:46 Suggested warning: "negating an expression of unsigned type does not yield a negative value" Falk Hueffner
@ 2003-10-06 16:00 ` Joe Buck
2003-10-06 16:11 ` Falk Hueffner
0 siblings, 1 reply; 6+ messages in thread
From: Joe Buck @ 2003-10-06 16:00 UTC (permalink / raw)
To: Falk Hueffner; +Cc: gcc
On Mon, Oct 06, 2003 at 02:46:43PM +0200, Falk Hueffner wrote:
> I just found yet another bug of the kind:
>
> int f (int *p, unsigned x) { return p[-x]; }
>
> which only manifests on 64 bit platforms, because most (all?)
> platforms have wrapping address arithmetic.
The C and C++ standards require that unsigned values obey modulo 2**N
arithmetic, so the value of -x is rigorously defined.
> So I was wondering about a general warning about negating unsigned
> values, since I couldn't really think of a legitimate application.
There are legitimate applications, and I've used them in my code.
> quick check with the gcc source turned up:
>
> gengtype-lex.l:
> char *namestart;
> size_t namelen;
> [...]
> for (namelen = 1; !ISSPACE (namestart[-namelen]); namelen++)
>
> This looks actually invalid to me, although it will probably work
> everywhere.
It's valid everywhere.
> In fold_const.c, there's
>
> case RSHIFT_EXPR:
> int2l = -int2l;
>
> also "invalid but works" since it's later passed to a function taking
> int.
Again, this is valid everywhere.
> Then there's everybody's favourite idiom "x &= -x", but it can be
> expressed clearer as "x &= ~x + 1".
Again, it's fine as is. Just the fact that your proposed warning will
turn on at least four complaints against correct usage in gcc shows
that it is not a good idea.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Suggested warning: "negating an expression of unsigned type does not yield a negative value"
2003-10-06 16:00 ` Joe Buck
@ 2003-10-06 16:11 ` Falk Hueffner
2003-10-06 17:23 ` Jamie Lokier
2003-10-06 17:48 ` Joe Buck
0 siblings, 2 replies; 6+ messages in thread
From: Falk Hueffner @ 2003-10-06 16:11 UTC (permalink / raw)
To: Joe Buck; +Cc: gcc
Joe Buck <jbuck@synopsys.com> writes:
> On Mon, Oct 06, 2003 at 02:46:43PM +0200, Falk Hueffner wrote:
> > I just found yet another bug of the kind:
> >
> > int f (int *p, unsigned x) { return p[-x]; }
> >
> > which only manifests on 64 bit platforms, because most (all?)
> > platforms have wrapping address arithmetic.
>
> The C and C++ standards require that unsigned values obey modulo 2**N
> arithmetic, so the value of -x is rigorously defined.
Sure it is. But it is not what is intended. Example: x = 5, then
-x=4294967291, i.e., p will be advanced by 4294967291 bytes, which is
way beyond the legal range of p, but happens to work anyway on 32 bit
architectures (but not on 64 bit architectures).
> > char *namestart;
> > size_t namelen;
> > [...]
> > for (namelen = 1; !ISSPACE (namestart[-namelen]); namelen++)
> >
> > This looks actually invalid to me, although it will probably work
> > everywhere.
>
> It's valid everywhere.
I'm pretty sure it's not. -namelen is, again, something like
4294967291 (or 18446744073709551611), which is not a legal array
index.
> > In fold_const.c, there's
> >
> > case RSHIFT_EXPR:
> > int2l = -int2l;
> >
> > also "invalid but works" since it's later passed to a function taking
> > int.
>
> Again, this is valid everywhere.
No, this produces an unsigned value which cannot be represented in a
signed value of same width, but is converted to signed, which is
undefined according to the standard.
> > Then there's everybody's favourite idiom "x &= -x", but it can be
> > expressed clearer as "x &= ~x + 1".
>
> Again, it's fine as is.
I agree with that.
--
Falk
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Suggested warning: "negating an expression of unsigned type does not yield a negative value"
2003-10-06 16:11 ` Falk Hueffner
@ 2003-10-06 17:23 ` Jamie Lokier
2003-10-06 17:48 ` Joe Buck
1 sibling, 0 replies; 6+ messages in thread
From: Jamie Lokier @ 2003-10-06 17:23 UTC (permalink / raw)
To: Falk Hueffner; +Cc: Joe Buck, gcc
Falk Hueffner wrote:
> > The C and C++ standards require that unsigned values obey modulo 2**N
> > arithmetic, so the value of -x is rigorously defined.
>
> Sure it is. But it is not what is intended. Example: x = 5, then
> -x=4294967291, i.e., p will be advanced by 4294967291 bytes, which is
> way beyond the legal range of p, but happens to work anyway on 32 bit
> architectures (but not on 64 bit architectures).
Good point. That's elusive enough that I saw ptr[-x] and thought it
was was fine; I am now humbly corrected.
-- Jamie
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Suggested warning: "negating an expression of unsigned type does not yield a negative value"
2003-10-06 16:11 ` Falk Hueffner
2003-10-06 17:23 ` Jamie Lokier
@ 2003-10-06 17:48 ` Joe Buck
2003-10-06 17:52 ` Falk Hueffner
1 sibling, 1 reply; 6+ messages in thread
From: Joe Buck @ 2003-10-06 17:48 UTC (permalink / raw)
To: Falk Hueffner; +Cc: gcc
On Mon, Oct 06, 2003 at 06:11:05PM +0200, Falk Hueffner wrote:
> Joe Buck <jbuck@synopsys.com> writes:
> > The C and C++ standards require that unsigned values obey modulo 2**N
> > arithmetic, so the value of -x is rigorously defined.
>
> Sure it is. But it is not what is intended. Example: x = 5, then
> -x=4294967291, i.e., p will be advanced by 4294967291 bytes, which is
> way beyond the legal range of p, but happens to work anyway on 32 bit
> architectures (but not on 64 bit architectures).
Good catch. It seems that the real problem is not negation of unsigned
values, but negation followed by extension to a potentially larger word size.
If you can figure out how to warn specifically about that, it might
be worth adding. But it would be tricky.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Suggested warning: "negating an expression of unsigned type does not yield a negative value"
2003-10-06 17:48 ` Joe Buck
@ 2003-10-06 17:52 ` Falk Hueffner
0 siblings, 0 replies; 6+ messages in thread
From: Falk Hueffner @ 2003-10-06 17:52 UTC (permalink / raw)
To: Joe Buck; +Cc: gcc
Joe Buck <jbuck@synopsys.com> writes:
> On Mon, Oct 06, 2003 at 06:11:05PM +0200, Falk Hueffner wrote:
> > Joe Buck <jbuck@synopsys.com> writes:
> > > The C and C++ standards require that unsigned values obey modulo 2**N
> > > arithmetic, so the value of -x is rigorously defined.
> >
> > Sure it is. But it is not what is intended. Example: x = 5, then
> > -x=4294967291, i.e., p will be advanced by 4294967291 bytes, which is
> > way beyond the legal range of p, but happens to work anyway on 32 bit
> > architectures (but not on 64 bit architectures).
>
> Good catch. It seems that the real problem is not negation of
> unsigned values, but negation followed by extension to a potentially
> larger word size. If you can figure out how to warn specifically
> about that, it might be worth adding. But it would be tricky.
I'll give it a try, but I'm not sure how to do that right now, warning
about it every time was a lot easier :)
--
Falk
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2003-10-06 17:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-10-06 12:46 Suggested warning: "negating an expression of unsigned type does not yield a negative value" Falk Hueffner
2003-10-06 16:00 ` Joe Buck
2003-10-06 16:11 ` Falk Hueffner
2003-10-06 17:23 ` Jamie Lokier
2003-10-06 17:48 ` Joe Buck
2003-10-06 17:52 ` Falk Hueffner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).