C language lawyers needed

C language lawyers needed

[Roland Dreier]

Can anyone explain why building the current kernel gives

    drivers/net/mlx4/mcg.c: In function 'mlx4_multicast_attach':
    drivers/net/mlx4/mcg.c:217: warning: integer overflow in expression

(with gcc (Ubuntu 4.3.1-9ubuntu1) 4.3.1 on x86-64), while the patch
below gets rid of the warning?  I can't see what is overflowing in the
expression that is warned out.

MGM_BLCK_LB_BIT is 30, and 1 << 30 is not negative or close to
overflowing.  And MGM_QPN_MASK is 0x00FFFFFF, which is also nowhere near
to integer overflow.

A fairly small test case that I don't understand either is:

unsigned foo(int x)
{
	return (((x & 0xffffff) | (1 << 30)) & 0xff000000) >> 24;
}

just running "gcc -c" (ie no extra warnings enabled) on that produces
the same:

    b.c: In function 'foo':
    b.c:3: warning: integer overflow in expression

I'm sure there's some promotion rule or something that makes sense of
this, but it's a mystery to me...

Thanks,
  Roland

diff --git a/drivers/net/mlx4/mcg.c b/drivers/net/mlx4/mcg.c
index c83f88c..6af5c6d 100644
--- a/drivers/net/mlx4/mcg.c
+++ b/drivers/net/mlx4/mcg.c
@@ -215,7 +215,7 @@ int mlx4_multicast_attach(struct mlx4_dev *dev, struct mlx4_qp *qp, u8 gid[16],
 
 	if (block_mcast_loopback)
 		mgm->qp[members_count++] = cpu_to_be32((qp->qpn & MGM_QPN_MASK) |
-						       (1 << MGM_BLCK_LB_BIT));
+						       (1u << MGM_BLCK_LB_BIT));
 	else
 		mgm->qp[members_count++] = cpu_to_be32(qp->qpn & MGM_QPN_MASK);
 

Re: C language lawyers needed

[H. Peter Anvin]

Roland Dreier wrote:
> 
> A fairly small test case that I don't understand either is:
> 
> unsigned foo(int x)
> {
> 	return (((x & 0xffffff) | (1 << 30)) & 0xff000000) >> 24;
> }
> 
> just running "gcc -c" (ie no extra warnings enabled) on that produces
> the same:
> 
>     b.c: In function 'foo':
>     b.c:3: warning: integer overflow in expression
> 
> I'm sure there's some promotion rule or something that makes sense of
> this, but it's a mystery to me...
> 

Looks like a gcc bug to me.

0xff000000 is unsigned, like any hexadecimal constant.

unsigned foo(int x)
{
	return ((x & 0xffffff) | (1 << 30)) & 0x80000000;
}

... is enough to reproduce the bug -- explicitly casting either side or 
both of the & operator to unsigned doesn't affect the warning, either.

	-hpa

Re: C language lawyers needed

[Al Viro]

On Wed, Aug 27, 2008 at 03:00:01PM -0700, H. Peter Anvin wrote:
> Looks like a gcc bug to me.
>
> 0xff000000 is unsigned, like any hexadecimal constant.

Not any.  Ones that do not fit into range of int but do fit into the range
of unsigned are.

Really, folks, it's not _that_ hard:

Type sequence: int, unsigned int, long, unsigned long, long long,
unsigned long long.
With U suffix => don't bother with signed types
Without U suffix, decimal => don't bother with _un_signed types
With L suffix => don't bother with anything earlier than long
With LL suffix => don't bother with anything earlier than long long
Pick the first type that covers your value; that will be it.

In particular, 0x[0-9A-Fa-f]+ == try *everything*.

Re: C language lawyers needed

[H. Peter Anvin]

Al Viro wrote:
> On Wed, Aug 27, 2008 at 03:00:01PM -0700, H. Peter Anvin wrote:
>> Looks like a gcc bug to me.
>>
>> 0xff000000 is unsigned, like any hexadecimal constant.
> 
> Not any.  Ones that do not fit into range of int but do fit into the range
> of unsigned are.
> 

Crap, I keep having a stale/bogus bit set for that since I don't know when.

	-hpa

Re: C language lawyers needed

[Al Viro]

On Wed, Aug 27, 2008 at 02:43:05PM -0700, Roland Dreier wrote:
> Can anyone explain why building the current kernel gives
> 
>     drivers/net/mlx4/mcg.c: In function 'mlx4_multicast_attach':
>     drivers/net/mlx4/mcg.c:217: warning: integer overflow in expression
> 
> (with gcc (Ubuntu 4.3.1-9ubuntu1) 4.3.1 on x86-64), while the patch
> below gets rid of the warning?  I can't see what is overflowing in the
> expression that is warned out.
> 
> MGM_BLCK_LB_BIT is 30, and 1 << 30 is not negative or close to
> overflowing.  And MGM_QPN_MASK is 0x00FFFFFF, which is also nowhere near
> to integer overflow.
> 
> A fairly small test case that I don't understand either is:
> 
> unsigned foo(int x)
> {
> 	return (((x & 0xffffff) | (1 << 30)) & 0xff000000) >> 24;
> }
> 
> just running "gcc -c" (ie no extra warnings enabled) on that produces
> the same:
> 
>     b.c: In function 'foo':
>     b.c:3: warning: integer overflow in expression
> 
> I'm sure there's some promotion rule or something that makes sense of
> this, but it's a mystery to me...

There isn't.  It's gcc being buggy and inconsistent.

Re: C language lawyers needed

[Roland Dreier]

 > There isn't.  It's gcc being buggy and inconsistent.

Thanks, I'll file a gcc bug I guess.

 - R.

Re: C language lawyers needed

[Roland Dreier]

 >     drivers/net/mlx4/mcg.c:217: warning: integer overflow in expression

In case anyone cares, this was indeed a gcc bug
(http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37261) and it is apparently
already fixed in the upstream gcc repository.

 - R.