Compiling kernel with SuSE 8.2/gcc 3.3

Compiling kernel with SuSE 8.2/gcc 3.3

[Stephan von Krawczynski]

Hello all,

during tests with latest SuSE distro 8.2 compiling 2.4.21-pre6 showed a lot of
"comparison between signed and unsigned" warnings. It looks like SuSE ships gcc
3.3 (prerelease). Is this compiler known to work for kernel compilation? Should
therefore all these warnings be fixed?

-- 
Regards,
Stephan

Re: Compiling kernel with SuSE 8.2/gcc 3.3

[Arnd Bergmann]

Stephan von Krawczynski wrote:

> during tests with latest SuSE distro 8.2 compiling 2.4.21-pre6 showed a lot of
> "comparison between signed and unsigned" warnings. It looks like SuSE ships gcc
> 3.3 (prerelease). Is this compiler known to work for kernel compilation? Should
> therefore all these warnings be fixed?

We're building 2.4 and 2.5 kernels for s390 with a gcc-3.3 snapshot and we had to
fix a few places in the kernel source to make that work reliably. See
http://marc.theaimsgroup.com/?t=104611164800008&r=1&w=2 for more about this.
I have also tried using gcc-3.3 for building a 2.4 i386 kernel and needed a
small patch, but ymmv.

        Arnd <><

Re: Compiling kernel with SuSE 8.2/gcc 3.3

[Andi Kleen]

Stephan von Krawczynski <skraw@ithnet.com> writes:

> during tests with latest SuSE distro 8.2 compiling 2.4.21-pre6 showed a lot of
> "comparison between signed and unsigned" warnings. It looks like SuSE ships gcc

The warning is in earlier gccs too, but turned off by default there.

I don't know why it was turned on by default in gcc 3.3 by the gcc maintainers,
but it gives so much noise that I tend to turn it off during the build (-Wno-sign-compare) 

Fixing it is usually not trivial. Yes, you could go through the source
and add casts to the comparisons, but that would miss the point
(assuming there is a point in this warning, I'm not 100% sure on that
;) Really it needs someone to audit these comparisons and determine
what signedness the comparisons should be in.  But that requires some
understanding of the code; it's not a brainless "janitor job" even 
if it looks like this on the first look.

Most of them come from a few headers.
e.g. I already submitted patches for the skbuff.h warnings for 2.5.

> 3.3 (prerelease). Is this compiler known to work for kernel compilation? Should
> therefore all these warnings be fixed?

I use it to build kernels and have not found a compiler caused problem with
it yet. Most of the kernel problems with specific gcc versions used to 
be kernel bugs anyways (like broken inline assembly or missing compile barriers) 
and most of them should be weeded out now from the experience with previous
gcc versions.

It does not accept unprotected new lines in strings anymore. Some
versions of the aic7xxx driver use that. Fixing it is trivial,
also the new aic7xxx driver in 2.4.21pre7 has it now fixed too.

The only thing what's a bit fishy in that compiler is the inlining algorithm
which tends to not inline functions anymore that have been previously inlined.
causing constant propagation to fail and some unnecessary code be generated.
This can be worked around though by putting

#define inline __attribute__((always_inline)) inline
#define __inline __attribute__((always_inline)) inline
#define __inline__ __attribute__((always_inline)) inline

into linux/compiler.h. This is more an code quality than correctness
issue for i386 (x86-64 had one case where it broke code, but that's
fixed already) 

But really, it does generate significantly better code for some things
than 3.2.

-Andi

Compiling kernel with SuSE 8.2/gcc 3.3

[Steve French]

Stephan von Krawczynski <skraw@ithnet.com> writes:

> during tests with latest SuSE distro 8.2 compiling 2.4.21-pre6 showed a lot of
> "comparison between signed and unsigned" warnings. It looks like SuSE ships gcc

I also noticed lots of compiler warnings with gcc 3.3, now default in SuSE, 
and cleaned up most of them for the cifs vfs but there are a few that just
look wrong for gcc to spit out warnings on.   For example the following
local variable definition and the similar ones in the same file
(fs/cifs/inode.c):

	__u64 uid = 0xFFFFFFFFFFFFFFFF;

generates a warning saying the value is too long for a long on 
x86 SuSE 8.2 with gcc 3.3 - which makes no sense.  Any value
above 0xFFFFFFFFF generates the same warning (intuitively
36 bits should fit in an unsigned 64 bit local variable).

Defining the literal with the UL suffix didn't seem to help - and I 
rebelled against the solutions that work for this case ie casting 
the local variable which is already __u64 to __u64 but that 
presumably would work for those three, as would a (__u64)cast 
of -1 which seems equally ugly).  Unfortunately for the CIFS
Unix Extensions these values really are supposed to be 64 bit
on the wire (0xFFFFFFFFFFFFFFFF indicating ignore setting this value).
In the current version of the cifs vfs (http://cifs.bkbits.net/linux-2.5cifs)
the only other case that now generates compiler warnings (in this case
signed/unsigned compares) is the comparison of unsigned local variables
to the literal PAGE_CACHE_SIZE (which presumably is interpreted as 
signed by the compiler).

Any idea what is going on in this weird gcc 3.3 behavior where it thinks
64 bits can't fit in a __u64 local variable? 

Re: Compiling kernel with SuSE 8.2/gcc 3.3

[Andi Kleen]

Steve French <smfrench@austin.rr.com> writes:

> look wrong for gcc to spit out warnings on.   For example the following
> local variable definition and the similar ones in the same file
> (fs/cifs/inode.c):
> 
> 	__u64 uid = 0xFFFFFFFFFFFFFFFF;
> 
> generates a warning saying the value is too long for a long on x86
> SuSE 8.2 with gcc 3.3 - which makes no sense.  Any value
> above 0xFFFFFFFFF generates the same warning (intuitively
> 36 bits should fit in an unsigned 64 bit local variable).
> 
> Defining the literal with the UL suffix didn't seem to help - and I

Define it with ULL   (= long long) 

> Any idea what is going on in this weird gcc 3.3 behavior where it thinks
> 64 bits can't fit in a __u64 local variable? -

AFAIK the problem is that it has no default promotion for constants to 
long long (normally they are int, long, unsigned long etc. depending on
their value) It's some C99 thing. Or maybe a gcc bug. Anyways ULL 
makes it clear that it is unsigned long long.

-Andi

P.S.: The warning is thankfully turned off by default again in later
compilers.

Re: Compiling kernel with SuSE 8.2/gcc 3.3

[Richard B. Johnson]

On Wed, 11 Jun 2003, Steve French wrote:

> Stephan von Krawczynski <skraw@ithnet.com> writes:
>
> > during tests with latest SuSE distro 8.2 compiling 2.4.21-pre6 showed a lot of
> > "comparison between signed and unsigned" warnings. It looks like SuSE ships gcc
>
> I also noticed lots of compiler warnings with gcc 3.3, now default in SuSE,
> and cleaned up most of them for the cifs vfs but there are a few that just
> look wrong for gcc to spit out warnings on.   For example the following
> local variable definition and the similar ones in the same file
> (fs/cifs/inode.c):
>
> 	__u64 uid = 0xFFFFFFFFFFFFFFFF;
>
> generates a warning saying the value is too long for a long on
> x86 SuSE 8.2 with gcc 3.3 - which makes no sense.  Any value
> above 0xFFFFFFFFF generates the same warning (intuitively
> 36 bits should fit in an unsigned 64 bit local variable).
[SNIPPED...]

I think the compiler doesn't have a default type for something
that long. Therefore you have to define is as:

 	__u64 uid = 0xFFFFFFFFFFFFFFFFULL;

Seems dumb, but it even works.

Cheers,
Dick Johnson
Penguin : Linux version 2.4.20 on an i686 machine (797.90 BogoMips).
Why is the government concerned about the lunatic fringe? Think about it.

Re: Compiling kernel with SuSE 8.2/gcc 3.3

[Matthias Andree]

On Wed, 11 Jun 2003, Steve French wrote:

> Stephan von Krawczynski <skraw@ithnet.com> writes:
> 
> >during tests with latest SuSE distro 8.2 compiling 2.4.21-pre6 showed a 
> >lot of
> >"comparison between signed and unsigned" warnings. It looks like SuSE 
> >ships gcc
> 
> I also noticed lots of compiler warnings with gcc 3.3, now default in SuSE, 
> and cleaned up most of them for the cifs vfs but there are a few that just
> look wrong for gcc to spit out warnings on.   For example the following
> local variable definition and the similar ones in the same file
> (fs/cifs/inode.c):

Did you try with a release version of gcc 3.3? The one shipped on SuSE
Linux 8.2 media/FTP is a pre-release version.

(If SuSE would only ship patch-level updates to such essential
components, say, 3.2.2 for 8.1 and the released 3.3 for 8.2.)

-- 
Matthias Andree

Re: Compiling kernel with SuSE 8.2/gcc 3.3

[Richard B. Johnson]

On Wed, 11 Jun 2003, Richard B. Johnson wrote:

> On Wed, 11 Jun 2003, Steve French wrote:
>
> > Stephan von Krawczynski <skraw@ithnet.com> writes:
> >
> > > during tests with latest SuSE distro 8.2 compiling 2.4.21-pre6 showed a lot of
> > > "comparison between signed and unsigned" warnings. It looks like SuSE ships gcc
> >
> > I also noticed lots of compiler warnings with gcc 3.3, now default in SuSE,
> > and cleaned up most of them for the cifs vfs but there are a few that just
> > look wrong for gcc to spit out warnings on.   For example the following
> > local variable definition and the similar ones in the same file
> > (fs/cifs/inode.c):
> >
> > 	__u64 uid = 0xFFFFFFFFFFFFFFFF;
> >
> > generates a warning saying the value is too long for a long on
> > x86 SuSE 8.2 with gcc 3.3 - which makes no sense.  Any value
> > above 0xFFFFFFFFF generates the same warning (intuitively
> > 36 bits should fit in an unsigned 64 bit local variable).
> [SNIPPED...]
>
> I think the compiler doesn't have a default type for something
> that long. Therefore you have to define is as:
>
>  	__u64 uid = 0xFFFFFFFFFFFFFFFFULL;
>
> Seems dumb, but it even works.
>

FYI some Spanish-language mailer is set up for an automatic response.
Please don't auto-respond to a mail-list!

Cheers,
Dick Johnson
Penguin : Linux version 2.4.20 on an i686 machine (797.90 BogoMips).
Why is the government concerned about the lunatic fringe? Think about it.

Re: Compiling kernel with SuSE 8.2/gcc 3.3

[Andreas Schwab]

Steve French <smfrench@austin.rr.com> writes:

|> I also noticed lots of compiler warnings with gcc 3.3, now default in
|> SuSE, and cleaned up most of them for the cifs vfs but there are a few
|> that just
|> look wrong for gcc to spit out warnings on.   For example the following
|> local variable definition and the similar ones in the same file
|> (fs/cifs/inode.c):
|> 
|> 	__u64 uid = 0xFFFFFFFFFFFFFFFF;
|> 
|> generates a warning saying the value is too long for a long on x86 SuSE
|> 8.2 with gcc 3.3 - which makes no sense.  Any value
|> above 0xFFFFFFFFF generates the same warning (intuitively
|> 36 bits should fit in an unsigned 64 bit local variable).

An expression is evaluated independent of the context, ie. the fact that
the left side of the assignment is a 64 bit variable has no significance
at all.  But I agree that the warning should only occur in c89 mode, not
in the default gnu89 mode, where long long is available.  And in fact the
generated code will be correct.

|> Defining the literal with the UL suffix didn't seem to help

Yes, since you need a long long literal.

|> rebelled against the solutions that work for this case ie casting the
|> local variable which is already __u64 to __u64 but that presumably would
|> work for those three, as would a (__u64)cast of -1 which seems equally
|> ugly).

You can just use -1.  The implicit conversion to __u64 will DTRT.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Deutschherrnstr. 15-19, D-90429 Nürnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Compiling kernel with SuSE 8.2/gcc 3.3

[Steve French]

Although it fixes it for building on 32 bit architectures, won't changing


	__u64 uid = 0xFFFFFFFFFFFFFFFF;
to

	__u64 uid = 0xFFFFFFFFFFFFFFFFULL;

generate a type mismatch warning on ppc64 and similar 64 bit
architecutres since __u64 is not a unsigned long long on ppc64 
(it is unsigned long)?  My gut reaction is to just ingore the three
places that cause warnings and the remaining two places that cause 
signed/unsigned compare warnings of unsigned int local variables 
to #defined literals (which presumably are treated as signed by default).

Andi Kleen wrote:

>Steve French <smfrench@austin.rr.com> writes:
>
>>... and the similar ones in the same file
>>(fs/cifs/inode.c):
>>
>>	__u64 uid = 0xFFFFFFFFFFFFFFFF;
>>
>>generates a warning saying the value is too long for a long on x86
>>SuSE 8.2 with gcc 3.3
>>
>
>Define it with ULL   (= long long) 
>
>
>AFAIK the problem is that it has no default promotion for constants to 
>long long (normally they are int, long, unsigned long etc. depending on
>their value) It's some C99 thing. Or maybe a gcc bug. Anyways ULL 
>makes it clear that it is unsigned long long.
>

Re: Compiling kernel with SuSE 8.2/gcc 3.3

[Andrew Morton]

Steve French <smfrench@austin.rr.com> wrote:
>
> Although it fixes it for building on 32 bit architectures, won't changing
> 
> 
>  	__u64 uid = 0xFFFFFFFFFFFFFFFF;
>  to
> 
>  	__u64 uid = 0xFFFFFFFFFFFFFFFFULL;
> 
>  generate a type mismatch warning on ppc64 and similar 64 bit
>  architecutres since __u64 is not a unsigned long long on ppc64 
>  (it is unsigned long)?

	u64 uid = -1;

will work just nicely.

Re: Compiling kernel with SuSE 8.2/gcc 3.3

[Linus Torvalds]

In article <3EE7D659.2000003@austin.rr.com>,
Steve French  <smfrench@austin.rr.com> wrote:
>Although it fixes it for building on 32 bit architectures, won't changing
>
>
>	__u64 uid = 0xFFFFFFFFFFFFFFFF;
>to
>
>	__u64 uid = 0xFFFFFFFFFFFFFFFFULL;
>
>generate a type mismatch warning on ppc64 and similar 64 bit
>architecutres since __u64 is not a unsigned long long on ppc64 
>(it is unsigned long)?

No, why would it? 

If you do

	char c = 1;

do you expect a warning? The right side of the assignent 
is an "int", and the left side is a "char", but it's perfectly ok to
assign a wider type to a narrower one.

And so if "__u64" were to be a plain "unsigned long" on a 64-bit
architecture (and even if "unsigned long long" were to be 128 bits), the
constant 0xFFFFFFFFFFFFFFFFULL is (a) a perfectly valid unsigned long
long value and (b) fits perfectly well even in an "unsigned long", so
the compiler has no reason to complain about the assignment losing bits
(which it otherwise might do).

So I'd much rather make the constants too big than too small. And yes,
Andrew's suggestion about just assigning -1 works, but it's actually a
very subtle cast at that point.

		Linus

RE: Compiling kernel with SuSE 8.2/gcc 3.3

[Riley Williams]

Hi Andrew.

 >> Although it fixes it for building on 32 bit architectures, 
 >> won't changing
 >>
 >>	__u64 uid = 0xFFFFFFFFFFFFFFFF;
 >>
 >> to
 >>
 >>	__u64 uid = 0xFFFFFFFFFFFFFFFFULL;
 >>
 >> generate a type mismatch warning on ppc64 and similar 64
 >> bit architectures since __u64 is not a unsigned long long
 >> on ppc64 (it is unsigned long)?

 >	u64 uid = -1;
 >
 > will work just nicely.

Won't that generate a warning about assigning a signed quantity
to an unsigned variable?

What's really needed is a set of definitions along the lines of

	#define MAX_U32	((__u32) 0xFFFFFFFFUL)
	#define MAX_U64	((__u64) 0xFFFFFFFFFFFFFFFFULL)

but as an intermediate measure, how about...

	__u64 uid = ((__u64) 0xFFFFFFFFFFFFFFFFULL);

Best wishes from Riley.
---
 * Nothing as pretty as a smile, nothing as ugly as a frown.

---
Outgoing mail is certified Virus Free.
Checked by AVG anti-virus system (http://www.grisoft.com).
Version: 6.0.488 / Virus Database: 287 - Release Date: 5-Jun-2003

Re: Compiling kernel with SuSE 8.2/gcc 3.3

[Horst von Brand]

Steve French <smfrench@austin.rr.com> said:
> Although it fixes it for building on 32 bit architectures, won't changing
> 
> 
> 	__u64 uid = 0xFFFFFFFFFFFFFFFF;
> to
> 
> 	__u64 uid = 0xFFFFFFFFFFFFFFFFULL;
> 
> generate a type mismatch warning on ppc64 and similar 64 bit
> architecutres since __u64 is not a unsigned long long on ppc64 
> (it is unsigned long)?  My gut reaction is to just ingore the three
> places that cause warnings and the remaining two places that cause 
> signed/unsigned compare warnings of unsigned int local variables 
> to #defined literals (which presumably are treated as signed by default).

Be careful, the value will get shoehorned into 4 bytes to make the int
constant, which is then assigned to the __u64.
-- 
Dr. Horst H. von Brand                   User #22616 counter.li.org
Departamento de Informatica                     Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria              +56 32 654239
Casilla 110-V, Valparaiso, Chile                Fax:  +56 32 797513