From owner-svn-src-all@FreeBSD.ORG Mon Aug 9 08:42:35 2010 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 668E0106566B; Mon, 9 Aug 2010 08:42:35 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail01.syd.optusnet.com.au (mail01.syd.optusnet.com.au [211.29.132.182]) by mx1.freebsd.org (Postfix) with ESMTP id F2CB98FC13; Mon, 9 Aug 2010 08:42:34 +0000 (UTC) Received: from c122-106-147-41.carlnfd1.nsw.optusnet.com.au (c122-106-147-41.carlnfd1.nsw.optusnet.com.au [122.106.147.41]) by mail01.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id o798gPOK016616 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 9 Aug 2010 18:42:26 +1000 Date: Mon, 9 Aug 2010 18:42:25 +1000 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: Jilles Tjoelker In-Reply-To: <20100808220631.GA86477@stack.nl> Message-ID: <20100809181106.U9102@delplex.bde.org> References: <20100807.205058.260300877050427878.imp@bsdimp.com> <201008082037.o78KbLDt022321@haluter.fromme.com> <20100808.153608.1142818667055052395.imp@bsdimp.com> <20100808220631.GA86477@stack.nl> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, olli@fromme.com, "M. Warner Losh" Subject: Re: svn commit: r211023 - head/usr.sbin/syslogd X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 09 Aug 2010 08:42:35 -0000 On Mon, 9 Aug 2010, Jilles Tjoelker wrote: > On Sun, Aug 08, 2010 at 03:36:08PM -0600, M. Warner Losh wrote: >> The casting that syslogd does with struct sockaddrFOO is what triggers >> it. I'm not sure how to fix it, so there's about 6 or 8 programs in >> the tree that have WARNS lowered to 3 because of it. > > This problem is common with programs that use getaddrinfo() and then > look into the address family specific parts of the address (it was > probably not the intention of the getaddrinfo() API to do this very > often). Obviously, when ai_family is the corresponding value, the > ai_addr really points to that particular kind of sockaddr, but gcc only > knows that struct sockaddr_in and struct sockaddr_in6 have a higher > alignment requirement than struct sockaddr. POSIX even standardized this broken (unimplementable in C) API. AT least in the old 2001 draft 7, it says that sockaddr_un shall include much the same members that FreeBSD's version has, and that applications shall access these in an even more broken way than by casting the pointer: it says that the struct type sockaddr_un shall be cast to struct access for use with the socket functions. I don't even remember seeing code so broken as to expect casting whole structs to an incompatible type to work. However, this is not completely unimplementable -- it just requires the struct types to be compatible, i.e., the same. But the reason for existence of sockaddr_un is to have the struct types not the same. They are not the same in FreeBSD. The pointers can probably made compatible enough in practice by implementing them in non-C using sufficient alignment and/or packing directives. Without these, the alignment and packing for the structs could easily end up incompatible due to gcc wanting to align the larger array of char more than the smaller array of char, like it does for global arrays of char. ABIs may already prevent such extra alignment in practice. > In some cases, the address may already be copied, and making the > destination the family-based type or struct sockaddr_storage (which has > a high alignment requirement) will avoid problems. > > In other cases, I propose adding a cast to void * in between, like > (struct sockaddr_in *)(void *)ai->ai_addr > > Note that this workaround must not be applied mindlessly to avoid > -Wcast-align, but only when it is known from other information that the > object really has that type. Of course it just breaks the warning. > If you don't like this workaround, perhaps copy the data to a variable > of the correct type. Addresses are not particularly big so the > performance hit should not be bad. > > It is unfortunate to have to miss other warnings because of this. Copying is just a fancier way of breaking the warning in this case. I was thinking of packing and aligning everything in the struct to ensure that the struct layouts are identical in their common part, but to just break the warning in a way that is easy to undo, it might be enough to declare the whole structs as __aligned(1). The structs consist of u_chars (sometimes spelled uint8_t and sometimes spelled char), so their natural alignment is 1. But I just remember that arm uses unnattural alignment. __aligned(4) might need to be used for it to preserve the ABI. What is the struct layout for arm? Bruce