Using macro CMSG_NXTHDR generates warnings with CLANG

Using macro CMSG_NXTHDR generates warnings with CLANG

[Jan Vorlicek]

[-- Attachment #1: Type: text/plain, Size: 1580 bytes --]

Trying to build a piece of code that uses CMSG_NXTHDR macro using CLANG (tested with CLANG 3.8) with all warnings enabled using -Weverything generates the following warnings:

clang++ -Weverything ./nettest.cpp -c -o nettest.o

./nettest.cpp:5:12: warning: cast from 'unsigned char *' to 'struct cmsghdr *' increases required alignment from 1 to 4 [-Wcast-align]
    return CMSG_NXTHDR(mhdr, cmsg);
           ^~~~~~~~~~~~~~~~~~~~~~~
/usr/include/sys/socket.h:270:8: note: expanded from macro 'CMSG_NXTHDR'
        ? 0 : (struct cmsghdr *)__CMSG_NEXT(cmsg))
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./nettest.cpp:5:12: warning: comparison of integers of different signs: 'unsigned long' and 'long' [-Wsign-compare]
    return CMSG_NXTHDR(mhdr, cmsg);
           ^~~~~~~~~~~~~~~~~~~~~~~
/usr/include/sys/socket.h:269:44: note: expanded from macro 'CMSG_NXTHDR'
        __CMSG_LEN(cmsg) + sizeof(struct cmsghdr) >= __MHDR_END(mhdr) - (unsigned char *)(cmsg) \
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2 warnings generated.

The testing source is below:

#include <sys/socket.h>
cmsghdr* GET_CMSG_NXTHDR(msghdr* mhdr, cmsghdr* cmsg);

cmsghdr* GET_CMSG_NXTHDR(msghdr* mhdr, cmsghdr* cmsg)
{
    return CMSG_NXTHDR(mhdr, cmsg);
}

Would it be possible to fix it so that no warnings are generated? We are building our application with -Weverything and currently we need to disable these two warnings around the CMSG_NXTHDR macro invocation.
Thank you in advance for considering that!

Jan

[-- Attachment #2: Type: text/html, Size: 4782 bytes --]

Re: Using macro CMSG_NXTHDR generates warnings with CLANG

[Markus Wichmann]

On Mon, Oct 10, 2016 at 10:09:38PM +0000, Jan Vorlicek wrote:
> Trying to build a piece of code that uses CMSG_NXTHDR macro using CLANG (tested with CLANG 3.8) with all warnings enabled using -Weverything generates the following warnings:
> 
> clang++ -Weverything ./nettest.cpp -c -o nettest.o
> 
> ./nettest.cpp:5:12: warning: cast from 'unsigned char *' to 'struct cmsghdr *' increases required alignment from 1 to 4 [-Wcast-align]
>     return CMSG_NXTHDR(mhdr, cmsg);
>            ^~~~~~~~~~~~~~~~~~~~~~~
> /usr/include/sys/socket.h:270:8: note: expanded from macro 'CMSG_NXTHDR'
>         ? 0 : (struct cmsghdr *)__CMSG_NEXT(cmsg))
>               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ./nettest.cpp:5:12: warning: comparison of integers of different signs: 'unsigned long' and 'long' [-Wsign-compare]
>     return CMSG_NXTHDR(mhdr, cmsg);
>            ^~~~~~~~~~~~~~~~~~~~~~~
> /usr/include/sys/socket.h:269:44: note: expanded from macro 'CMSG_NXTHDR'
>         __CMSG_LEN(cmsg) + sizeof(struct cmsghdr) >= __MHDR_END(mhdr) - (unsigned char *)(cmsg) \
>         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 2 warnings generated.
> 
> The testing source is below:
> 
> #include <sys/socket.h>
> cmsghdr* GET_CMSG_NXTHDR(msghdr* mhdr, cmsghdr* cmsg);
> 
> cmsghdr* GET_CMSG_NXTHDR(msghdr* mhdr, cmsghdr* cmsg)
> {
>     return CMSG_NXTHDR(mhdr, cmsg);
> }
> 
> Would it be possible to fix it so that no warnings are generated? We are building our application with -Weverything and currently we need to disable these two warnings around the CMSG_NXTHDR macro invocation.
> Thank you in advance for considering that!
> 
> Jan

Both of these appear to be unavoidable if the computation is to be done
inline, as it is at the moment. I wondered how glibc does it and they
just have a function (__cmsg_nxthdr()) taking care of it. I doubt most
contributors here would like to add a function with the sole purpose of
calculating a macro, just to shut up the compiler.

The first warning is unavoidable because the next control message header
can only be reached by skipping over the entire control message, the
length of which is recorded in bytes. We do take care of the alignment
doing that, but apparently clang is incapable of recognizing that.

The second one could be avoided with an explicit type conversion.

However, isn't our stance that warnings in system headers are broken,
anyway? And warnings resulting from expansions of macros out of system
headers just the same.

Ciao,
Markus

Re: Using macro CMSG_NXTHDR generates warnings with CLANG

[Rich Felker]

On Mon, Oct 10, 2016 at 10:09:38PM +0000, Jan Vorlicek wrote:
> Trying to build a piece of code that uses CMSG_NXTHDR macro using
> CLANG (tested with CLANG 3.8) with all warnings enabled using
> -Weverything generates the following warnings:
> 
> clang++ -Weverything ./nettest.cpp -c -o nettest.o
> 
> ../nettest.cpp:5:12: warning: cast from 'unsigned char *' to 'struct cmsghdr *' increases required alignment from 1 to 4 [-Wcast-align]
>     return CMSG_NXTHDR(mhdr, cmsg);
>            ^~~~~~~~~~~~~~~~~~~~~~~
> /usr/include/sys/socket.h:270:8: note: expanded from macro 'CMSG_NXTHDR'
>         ? 0 : (struct cmsghdr *)__CMSG_NEXT(cmsg))
>               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../nettest.cpp:5:12: warning: comparison of integers of different signs: 'unsigned long' and 'long' [-Wsign-compare]
>     return CMSG_NXTHDR(mhdr, cmsg);
>            ^~~~~~~~~~~~~~~~~~~~~~~
> /usr/include/sys/socket.h:269:44: note: expanded from macro 'CMSG_NXTHDR'
>         __CMSG_LEN(cmsg) + sizeof(struct cmsghdr) >= __MHDR_END(mhdr) - (unsigned char *)(cmsg) \
>         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 2 warnings generated.
> 
> The testing source is below:
> 
> #include <sys/socket.h>
> cmsghdr* GET_CMSG_NXTHDR(msghdr* mhdr, cmsghdr* cmsg);
> 
> cmsghdr* GET_CMSG_NXTHDR(msghdr* mhdr, cmsghdr* cmsg)
> {
>     return CMSG_NXTHDR(mhdr, cmsg);
> }
> 
> Would it be possible to fix it so that no warnings are generated? We
> are building our application with -Weverything and currently we need
> to disable these two warnings around the CMSG_NXTHDR macro
> invocation.
> Thank you in advance for considering that!

As these are system headers, the compiler should not be producing any
warnings from them. If it does that's a compiler bug. Are you perhaps
using an odd setup where musl's headers aren't in the default system
include path but instead passed in via -I rather than -isystem? If you
have a minimal test file I could see if the same warnings appear with
clang on Alpine Linux.

Rich

Re: Using macro CMSG_NXTHDR generates warnings with CLANG

[Alexander Monakov]

On Tue, 11 Oct 2016, Rich Felker wrote:
> As these are system headers, the compiler should not be producing any
> warnings from them. If it does that's a compiler bug. Are you perhaps
> using an odd setup where musl's headers aren't in the default system
> include path but instead passed in via -I rather than -isystem? If you
> have a minimal test file I could see if the same warnings appear with
> clang on Alpine Linux.

As mentioned on IRC some time ago, Clang suppresses warnings in macros expanded
from definitions in system headers on a case-by-case basis, i.e. some warnings
are suppressed (if someone deliberately enabled suppression), some are not. As
seen here, -Wcast-align is among those warnings that are not suppressed.

Alexander

Re: Using macro CMSG_NXTHDR generates warnings with CLANG

[Rich Felker]

On Tue, Oct 11, 2016 at 06:17:48PM +0300, Alexander Monakov wrote:
> On Tue, 11 Oct 2016, Rich Felker wrote:
> > As these are system headers, the compiler should not be producing any
> > warnings from them. If it does that's a compiler bug. Are you perhaps
> > using an odd setup where musl's headers aren't in the default system
> > include path but instead passed in via -I rather than -isystem? If you
> > have a minimal test file I could see if the same warnings appear with
> > clang on Alpine Linux.
> 
> As mentioned on IRC some time ago, Clang suppresses warnings in macros expanded
> from definitions in system headers on a case-by-case basis, i.e. some warnings
> are suppressed (if someone deliberately enabled suppression), some are not. As
> seen here, -Wcast-align is among those warnings that are not suppressed.

Ah. I think this should just be considered a clang bug then...

Rich

RE: Using macro CMSG_NXTHDR generates warnings with CLANG

[Jan Vorlicek]

The simple repro was in my email - just cut and paste the 6 lines of code into a file named nettest.cpp and then run clang++ -Weverything ./nettest.cpp -c -o nettest.o.
I have hit this issue on Alpine Linux 3.4.3 in a docker container.

The same code compiles without any warnings with the same clang compiler e.g. on Ubuntu 14.04 with glibc. I have noticed a difference though. On Alpine, the CMSG_NXTHDR is a macro defined in /usr/include/sys/socket.h. On Ubuntu, it is defined in /usr/include/x86_64-linux-gnu/bits/socket.h. The definition is also different:
Alpine:
#define CMSG_NXTHDR(mhdr, cmsg) ((cmsg)->cmsg_len < sizeof (struct cmsghdr) || \
        __CMSG_LEN(cmsg) + sizeof(struct cmsghdr) >= __MHDR_END(mhdr) - (unsigned char *)(cmsg) \
        ? 0 : (struct cmsghdr *)__CMSG_NEXT(cmsg))

Ubuntu:
#define CMSG_NXTHDR(mhdr, cmsg) __cmsg_nxthdr (mhdr, cmsg)
Where the __cmsg_nxthdr is defined as an inline function in the same header file as the CMSG_NXTHDR.

Thanks,

Jan

-----Original Message-----
From: Rich Felker [mailto:dalias@aerifal.cx] On Behalf Of Rich Felker
Sent: Tuesday, October 11, 2016 5:23 PM
To: musl@lists.openwall.com
Subject: Re: [musl] Using macro CMSG_NXTHDR generates warnings with CLANG

On Tue, Oct 11, 2016 at 06:17:48PM +0300, Alexander Monakov wrote:
> On Tue, 11 Oct 2016, Rich Felker wrote:
> > As these are system headers, the compiler should not be producing 
> > any warnings from them. If it does that's a compiler bug. Are you 
> > perhaps using an odd setup where musl's headers aren't in the 
> > default system include path but instead passed in via -I rather than 
> > -isystem? If you have a minimal test file I could see if the same 
> > warnings appear with clang on Alpine Linux.
> 
> As mentioned on IRC some time ago, Clang suppresses warnings in macros 
> expanded from definitions in system headers on a case-by-case basis, 
> i.e. some warnings are suppressed (if someone deliberately enabled 
> suppression), some are not. As seen here, -Wcast-align is among those warnings that are not suppressed.

Ah. I think this should just be considered a clang bug then...

Rich

Re: Using macro CMSG_NXTHDR generates warnings with CLANG

[Szabolcs Nagy]

* Rich Felker <dalias@libc.org> [2016-10-11 11:09:01 -0400]:
> On Mon, Oct 10, 2016 at 10:09:38PM +0000, Jan Vorlicek wrote:
> > The testing source is below:
> > 
> > #include <sys/socket.h>
> > cmsghdr* GET_CMSG_NXTHDR(msghdr* mhdr, cmsghdr* cmsg);
> > 

this is invalid c code (you cannot leave 'struct' off).

> > cmsghdr* GET_CMSG_NXTHDR(msghdr* mhdr, cmsghdr* cmsg)
> > {
> >     return CMSG_NXTHDR(mhdr, cmsg);
> > }
> > 
> > Would it be possible to fix it so that no warnings are generated? We
> > are building our application with -Weverything and currently we need
> > to disable these two warnings around the CMSG_NXTHDR macro
> > invocation.
> > Thank you in advance for considering that!
> 
> As these are system headers, the compiler should not be producing any
> warnings from them. If it does that's a compiler bug. Are you perhaps
> using an odd setup where musl's headers aren't in the default system
> include path but instead passed in via -I rather than -isystem? If you
> have a minimal test file I could see if the same warnings appear with
> clang on Alpine Linux.
> 

fwiw i see the warnings with clang -c -Wcast-align test.c

i assume they cannot easily tell if they should warn when a macro
defined in a system header expanded with user arguments.

i think this warning is problematic: it warns about casting char*,
but not about casting void* which has the exact same issue.

so if you have apis that uses char* instead of void* for generic
pointers (e.g. for abi compat reasons), then you get loads of
false positives (which is what happens here).

it can be worked around by adding a void* cast (but in my
opinion that makes things worse: users will add more casts
to get rid of the warning which has the opposite effect than
what you would want).

RE: Using macro CMSG_NXTHDR generates warnings with CLANG

[Jan Vorlicek]

My test was a c++ code :-). That's why the struct was not there.

Thanks,

Jan

-----Original Message-----
From: Szabolcs Nagy [mailto:nsz@port70.net] 
Sent: Tuesday, October 11, 2016 5:31 PM
To: musl@lists.openwall.com
Subject: Re: [musl] Using macro CMSG_NXTHDR generates warnings with CLANG

* Rich Felker <dalias@libc.org> [2016-10-11 11:09:01 -0400]:
> On Mon, Oct 10, 2016 at 10:09:38PM +0000, Jan Vorlicek wrote:
> > The testing source is below:
> > 
> > #include <sys/socket.h>
> > cmsghdr* GET_CMSG_NXTHDR(msghdr* mhdr, cmsghdr* cmsg);
> > 

this is invalid c code (you cannot leave 'struct' off).

> > cmsghdr* GET_CMSG_NXTHDR(msghdr* mhdr, cmsghdr* cmsg) {
> >     return CMSG_NXTHDR(mhdr, cmsg);
> > }
> > 
> > Would it be possible to fix it so that no warnings are generated? We 
> > are building our application with -Weverything and currently we need 
> > to disable these two warnings around the CMSG_NXTHDR macro 
> > invocation.
> > Thank you in advance for considering that!
> 
> As these are system headers, the compiler should not be producing any 
> warnings from them. If it does that's a compiler bug. Are you perhaps 
> using an odd setup where musl's headers aren't in the default system 
> include path but instead passed in via -I rather than -isystem? If you 
> have a minimal test file I could see if the same warnings appear with 
> clang on Alpine Linux.
> 

fwiw i see the warnings with clang -c -Wcast-align test.c

i assume they cannot easily tell if they should warn when a macro defined in a system header expanded with user arguments.

i think this warning is problematic: it warns about casting char*, but not about casting void* which has the exact same issue.

so if you have apis that uses char* instead of void* for generic pointers (e.g. for abi compat reasons), then you get loads of false positives (which is what happens here).

it can be worked around by adding a void* cast (but in my opinion that makes things worse: users will add more casts to get rid of the warning which has the opposite effect than what you would want).

Re: Using macro CMSG_NXTHDR generates warnings with CLANG

[Szabolcs Nagy]

* Jan Vorlicek <janvorli@microsoft.com> [2016-10-11 15:38:38 +0000]:
> My test was a c++ code :-). That's why the struct was not there.
> 

don't top post.

including sys/socket.h (or any posix header) in c++ code is undefined
(neither iso c++ nor posix defines the behaviour) so you are on your own.

Re: Using macro CMSG_NXTHDR generates warnings with CLANG

[Rich Felker]

On Tue, Oct 11, 2016 at 06:43:21PM +0200, Szabolcs Nagy wrote:
> * Jan Vorlicek <janvorli@microsoft.com> [2016-10-11 15:38:38 +0000]:
> > My test was a c++ code :-). That's why the struct was not there.
> > 
> 
> don't top post.
> 
> including sys/socket.h (or any posix header) in c++ code is undefined
> (neither iso c++ nor posix defines the behaviour) so you are on your own.

C++ isn't the issue here. The header is obviously expected to work in
C++, even though there's no formal spec for it (although of course you
should have extern "C" around it). The issue at hand is the clang
warnings and I think they happen just the same in equivalent C code.

Rich

Re: Using macro CMSG_NXTHDR generates warnings with CLANG

[Jacob Welsh]

Rich Felker wrote:
> On Tue, Oct 11, 2016 at 06:43:21PM +0200, Szabolcs Nagy wrote:
> > * Jan Vorlicek <janvorli@microsoft.com> [2016-10-11 15:38:38 +0000]:
> > > My test was a c++ code :-). That's why the struct was not there.
> > > 
> > 
> > don't top post.
> > 
> > including sys/socket.h (or any posix header) in c++ code is undefined
> > (neither iso c++ nor posix defines the behaviour) so you are on your own.
> 
> C++ isn't the issue here. The header is obviously expected to work in
> C++, even though there's no formal spec for it (although of course you
> should have extern "C" around it). The issue at hand is the clang
> warnings and I think they happen just the same in equivalent C code.

Reviving this thread to confirm this does come up in C code, including
with GCC 4.7 for one of the two warnings:

#include <sys/socket.h>
struct cmsghdr *test(struct msghdr *m, struct cmsghdr *c) {
    return CMSG_NXTHDR(m,c);
}

$ gcc -c cmsg.c -Wall -Wextra
cmsg.c: In function 'test':
cmsg.c:3:12: warning: comparison between signed and unsigned integer
 expressions [-Wsign-compare]

I came across this in nginx which uses -Werror by default.

J. Welsh