Impossible to use GCC with -Wconversion and standard socket macros?

johan162 · 11-02-2010, 12:32 PM

When coding I like to enable many warnings in GCC, for example Wall, -Wextra, -Wconversion etc.

However, i discovered one problem with standard macros which generates warning when -Wconversion is enabled and there doesn't seem any way around this except rewriting the standard macros.

Standard socket code often have variations of these lines

Code:

int    fd;
fd_set fds;
...
FD_ZERO (&fds);
FD_SET (fd, &fds);
...

However, this can not be compiled cleanly with -Wconversion and no user level type casting can eliminate these warnings (as far as I can understand having looked at the macros)

What have other people done in this situation ?

P.S Using #pragma GCC diagnostic ignored "-Wconversion" doesn't seem to disable these warnings.

Johan

JohnGraham · 11-02-2010, 01:17 PM

Quote:

Originally Posted by johan162

Code:

int    fd;
fd_set fds;
...
FD_ZERO (&fds);
FD_SET (fd, &fds);
...

However, this can not be compiled cleanly with -Wconversion and no user level type casting can eliminate these warnings (as far as I can understand having looked at the macros)

What version of gcc are you using? The following compiles just fine for me without any warnings:

Code:

$ cat test.c 
#include <sys/select.h>

int main(void)
{
    fd_set fds;
    int    fd = 0;
    FD_ZERO(&fds);
    FD_SET(fd, &fds);
    return 0;
}

$ make test
cc -std=gnu99 -Wall -Wextra -Werror -g -Waggregate-return -Wall -Wbad-function-cast -Wcast-qual -Wdisabled-optimization -Werror -Wfloat-equal -Winline -Wlong-long -Wmissing-declarations -Wmissing-noreturn -Wmissing-prototypes -Wnested-externs -Wredundant-decls -Wshadow -Wsign-compare -Wstrict-prototypes -Wundef -Wwrite-strings -Wconversion    test.c   -o test
$ echo $?
0
$ gcc --version
gcc (Ubuntu 4.4.3-4ubuntu5) 4.4.3
Copyright (C) 2009 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

johan162 · 11-02-2010, 04:14 PM

Interesting. If I compile this code (under the name wtst.c) I get:

Code:

gcc -Werror -Wconversion wtst.c
cc1: warnings being treated as errors
wtst.c: In function ‘main’:
wtst.c:8: error: conversion to ‘unsigned int’ from ‘int’ may change the sign of the result
wtst.c:8: error: conversion to ‘unsigned int’ from ‘int’ may change the sign of the result

the version of gcc I use is

Code:

gcc --version
gcc (SUSE Linux) 4.4.1 [gcc-4_4-branch revision 150839]
Copyright (C) 2009 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Interesting ...

JohnGraham · 11-02-2010, 04:48 PM

Well, it could be that FD_SET is different for me. Mine expands into:

Code:

(((&fds)->__fds_bits)[((fd) / (8 * (int) sizeof (__fd_mask)))] |=
   ((__fd_mask) 1 << ((fd) % (8 * (int) sizeof (__fd_mask)))));

If I remove the (int) casts I get the same errors you get - however, replacing "fd" with "((unsigned int)fd)" gets rid of the errors, so using FD_SET((unsigned int)fd, &fds) should solve your problem.

johan162 · 11-02-2010, 05:56 PM

Unfortunately not. That cast was the first thing I tried and it gets rid of one of the warnings but one remains.

The version of GNU Clib included on SuSE 11.2 (which this is tested on) expands the macros to inline assembler. So for example the

Code:

FD_SET ((unsigned int)fd, &fds);

gets expanded to

Code:

  __asm__ __volatile__ ("btsl %1,%0" : "=m" (((&fds)->__fds_bits)[(((unsigned int)fd) / (8 * sizeof (__fd_mask)))]) : "r" (((int) ((unsigned int)fd)) % (8 * sizeof (__fd_mask))) : "cc","memory");

As far as can tell the core problem is thát sizeof() has return type size_t (which usually is an unsigned type, and most definitely on X86 arch) and this gets automatically downcasted to an int by gcc in the expressions above and this is the two warnings.

The reason the first warning goes away is that in this case (as can be seen from the expression above) the first conversion is fine but in the second the macro has an explicit (int) cast which is the problematic part.

So, without changing the macro manually this cannot be fixed. I consider this a bug within the gnu clib but I might be overlooking something.

JohnGraham · 11-03-2010, 06:35 AM

Quote:

Originally Posted by johan162

The reason the first warning goes away is that in this case (as can be seen from the expression above) the first conversion is fine but in the second the macro has an explicit (int) cast which is the problematic part.

Yeah, it seems weird that they'd give one occurrence of fd an explicit cast but not the other...

Either way, yeah, I can't see any way to get over it cleanly. Not too sure how you'd go about reporting that. I know if you run a glibc library (e.g. run "/lib/libc.so.6" at a shell) it'll print version info that'll be important when reporting it. Maybe you're best reporting it to the SUSE people...???

JohnGraham · 11-03-2010, 06:43 AM

Oh, and one (very hacky) workaround would be to force sizeof to be cast to an int:

Code:

#define sizeof(X) ((int)sizeof(X))
    FD_SET(fd, &fds);
#undef sizeof

Like I said, not pretty, but it'll allow -Wconversion to be used on the rest of the file.

johan162 · 11-04-2010, 03:28 AM

In relation to this I also discovered another signed:ness optimization bug in gcc.

The following (legitimate) code will also generate a warning when both -O2 and -Wconversion is enabled but not when only -Wconversion is enabled. The optimization is to clever for its own good

Code:

#include <arpa/inet.h>
// Nonsense code to illustrate problem
int main(void) {
    uint16_t portnbr=0;
    uint16_t n_portnbr = htons(portnbr);
    return n_portnbr;
}

With optimization

Code:

$> gcc -std=gnu99 -Wconversion -Werror -O2 -c tsthtons.c
cc1: warnings being treated as errors
tsthtons.c: In function ‘main’:
tsthtons.c:6: error: conversion to ‘short unsigned int’ from ‘int’ may alter its value

and without

Code:

$>gcc -std=gnu99 -Wconversion -Werror  -c tsthtons.c
$>

in both cases

Quote:

gcc --version
gcc (SUSE Linux) 4.4.1 [gcc-4_4-branch revision 150839]
Copyright (C) 2009 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

To me it seems like the gcc -Wconversion flag (while in principle good and can probably help discover some subtle once-in-a-blue-moon bugs) has not really been through enough tests in real life code. Looking at some various large code bases this flag is never used. (Probably because of the huge number of false positives that people consider irritating, like the pattern "int n=strnlen(buffer,MAX_BUFFLEN);" that will generate a warning that is technically correct but on code that generally is agreed on as acceptable.

In most places I worked there has in general been a rule that all code should be "silent" with maximum warnings enabled and that any warning will break the build (an the culprit submitting breaking code buys everyone beer in the local)