IN6_IS_ADDR_* macros use invalid type punning?

[Maxim Dounin]

Hello!

On Fri, Jun 07, 2013 at 08:08:37AM +0200, Matthias Andree wrote:

> Am 07.06.2013 01:29, schrieb Maxim Dounin:
> 
> > [...]
> > 
> >> try.c:9:5: warning: dereferencing type-punned pointer will break
> >> strict-aliasing rules [-Wstrict-aliasing]
> >>      int r = IN6_IS_ADDR_V4MAPPED((&sin6.sin6_addr));
> >>      ^
> >> try.c:9:5: warning: dereferencing type-punned pointer will break
> >> strict-aliasing rules [-Wstrict-aliasing]
> > 
> > [...]
> 
> > Gleb already committed a fix for this 16 months ago 
> > (unfortunately, correct patch description was lost in transit):
> > 
> > http://svnweb.freebsd.org/base?view=revision&revision=230584
> 
> Great.  Thank you for the pointer.  I could have checked head/ first
> indeed.
> 
> Looking at
> <http://svnweb.freebsd.org/base/head/sys/netinet6/in6.h?r1=230584&r2=230583&pathrev=230584>:
> The code committed at that time is lucky that htonl() and ntohl() are
> implemented the same way; all changed macros should be changed to use ==
> htonl(1) or == htonl(0x0000ffff), just to get the proper meaning across.
> 
> ntohl would have to be applied to the __u6_addr instead, but is less
> efficient because it is not open to compile-time evaluation, unlike
> htonl(CONSTANT_ADDRESS).

This is how it works since 1999, and seems to be completely 
separate issue:

http://www.kame.net/dev/cvsweb2.cgi/kame/kame/sys/netinet6/in6.h.diff?r1=1.1;r2=1.2

It's mostly cosmetic though.

> And indeed the commit log is a bit less compelling than might have
> fostered the propagation.  It looks like it were only about qualifiers,
> but it is also about violating aliasing rules per ISO 9899.

Yes, as I said, correct patch description was, unfortunately, lost 
in transit.  Original intention was to fix aliasing warnings as 
produced by gcc 4.6+ with -O2.  To don't reinvent the wheel patch 
from NetBSD for the problem was used,

http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/netinet6/in6.h#rev1.67
http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/netinet6/in6.h.diff?r1=1.66&r2=1.67

> > Probably it's a good idea to MFC the fix.
> 
> Please let's get this MFC'd and MFS'd (while it won't make releng/8.4 at
> least we can have stable/8 fixed, too) and get the system-headers
> induced warning done away with on all supported branches.

I personally don't think that stable/8 needs to be touched, but it's 
up to Gleb to decide.

-- 
Maxim Dounin
http://nginx.org/en/donation.html