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