❦ 15 mai 2016 09:55 +0200, Vincent Bernat <ber...@luffy.cx> : >> I suppose that some new features of gcc started to rely on the >> strict-aliasing rule without taking -fno-strict-aliasing into >> consideration. I didn't find anything in the bugzilla, but it's easy to >> miss something as there are about 500 bugs opened about aliasing. >> >> I'll open a bug about this. > > Here is the bug: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71120 > > With luck, we should get some pointers on why and how.
So, it appears this is not a bug in GCC and it is not an aliasing problem but the fact that the compiler doesn't have to use memcpy() to copy the struct, but can copy individual member, padding excluded. And sin_addr would be in the padding, so not copied. A bug has been filed for the glibc since the bug is in the definition: https://sourceware.org/bugzilla/show_bug.cgi?id=20111 We could either use an union like discussed previously or locally fix this with a memcpy(). I suppose the union would be a longer term (and safer since we can be exhaustive about it) fix but far more invasive. I suspect there are other uses of sockaddr_storage that could lead to some bugs. For example, connection.c:747. ss_family will be set to 0, but eventual port and address can be random. However, it seems that in the remaining of the code, only ss_family is important. But maybe, it would be better to use memset() here. On the other hand, on proto_tcp.c:225, bind_addr is correctly zeroed out with memset. Same on standard.c:829. I also note on proto_http.c:3192, a "struct sockaddr_storage from" that may be more safe to replace by "struct sockaddr_storage *from" since it is used read-only. But maybe the compiler is smart enough to not do a copy and therefore the bug is not triggered here. I didn't review all uses, only uses of "struct sockaddr_storage something". I should check for "struct sockaddr_storage *something" and eventual use of "*something =", but I suspect that it doesn't exist without a "struct sockaddr_storage something_else" around. So, quick fix is a memcpy() in cfgparse.c and in proto_http.c:3192 and using memset() in connection.c:747 for consistency (so that somebody doesn't copy this code for something more general). Can do the patches but union would be more work. -- A light wife doth make a heavy husband. -- Wm. Shakespeare, "The Merchant of Venice"
