John Dennis wrote: > I wanted to understand the issues surrounding strict aliasing better. I > found the following article to be well written, quite readable, and > informative:
I found a NetBSD post with similar information: http://mail-index.netbsd.org/tech-kern/2003/08/11/0001.html > However, note that this is exactly what fr_socket is doing, &salocal is > a pointer to sockaddr_storage, sa is in one scope is a pointer to > sockaddr_in and in another scope sa is a pointer to sockaddr_in6. Each > of these 3 pointers point to exact same memory location, according to > the above definitions this is a classic case of illegal pointer aliasing. Under ISO C89, and ISO C99. But it appears to be legal under the Posix description for "struct sockaddr_storage". Aren't standards wonderful. > It's a shame it didn't, but I don't think it changes the overarching > issue. Also, in expediency I did not try building with the various GCC > warning flags to see if I could get GCC to emit a warning for this case, > perhaps GCC at some level of verbosity would have complained. I've done some builds with "-fstrict-aliasing -Wstrict-aliasing=2". Ignoring the casts on parameters to functions, I've fixed a number of things, through the simple expedient of using memcpy() to copy from (struct sockaddr_storage) to (struct sockaddr_in), and back. It's ugly, but it SHOULD work. > For what it's worth my reading of the above statements is different than > yours. I read the above as sockaddr_storage provides a type whose size > is sufficient to hold all sockaddr types while respecting the alignment > requirement in any given sockaddr type. The discussion of pointer > casting fails to make explicit such casting must still conform to the > rules of C99. I do not read the second paragraph as meaning pointer > aliasing shall be supported, something outside that scope of that document. At the minimum, the Opengroup specs seem misleading. > Proper type casting is discussed in the above article in the section: > "Casting through a union (1 & 2)" which states "The most commonly > accepted method of converting one type of object to another is by using > a union". > > Note this is what both Jakub and Ulrich independently suggested in the > bug report. Which makes me wonder WTF is the use of 'struct sockaddr_storage'. All of the documentation I've read indicates that it's *supposed* to be the preferred structure to cast back and forth to 'struct sockaddr_in'... but that's forbidden by ISO C89. You'd think that if the Posix people read the ISO C specs, they would have created a 'struct sockaddr_union', which was defined to be the union of all types, and which would have avoided these problems. Maybe I'm naive... > I have created a patch for packet.c which uses a union (the patch is > attached). I built freeradius 2.0.4 with the patch and -O2 optimization > and the random port problem seems to have been resolved. Note, I have > not looked though all the source code to see if there are other code > constructs which may also require a fix. There's a number. Please try CVS head. I've committed a whack of memcpy's, which SHOULD avoid these issues. (And avoid umpteen unions). > Also, does anyone (Alan?) have any problems with updating the bug report > with some of this email dialog to capture the issues for future readers? I'd like to say that for me, the Posix spec is clear: casting between 'struct sockaddr_storage' and other 'struct sockaddr_*' is explicitely allowed. Looking at code on the net, there are LOTS of programs doing this. What is a different about FreeRADIUS is that it's *accessing* the pointer after the cast, and not just passing it to a function. This appears to be explicitly required by Posix: ...pointers to it can be cast as pointers to protocol-specific address structures and used to access the fields of those structures without alignment problems... i.e. TO ACCESS the fields. ACCESS to me means ACCESS. R/W ACCESS. Except that this is forbidden by ISO C89, and no one else does it. <sigh> It serves me right for reading only half of the 10's of 1000's of pages of specs. Alan DeKok. - List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
