From owner-freebsd-bugs@FreeBSD.ORG Fri Oct 8 20:20:05 2010 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D0345106566C for ; Fri, 8 Oct 2010 20:20:05 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id A54FF8FC12 for ; Fri, 8 Oct 2010 20:20:05 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id o98KK5e1089149 for ; Fri, 8 Oct 2010 20:20:05 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id o98KK5uU089148; Fri, 8 Oct 2010 20:20:05 GMT (envelope-from gnats) Date: Fri, 8 Oct 2010 20:20:05 GMT Message-Id: <201010082020.o98KK5uU089148@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org From: Bruce Evans Cc: Subject: Re: kern/151304: patch - definitions of variables tested by ASSERT_ATOMIC_LOAD_PTR X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Bruce Evans List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 08 Oct 2010 20:20:05 -0000 The following reply was made to PR kern/151304; it has been noted by GNATS. From: Bruce Evans To: Svatopluk Kraus Cc: freebsd-gnats-submit@freebsd.org, freebsd-bugs@freebsd.org Subject: Re: kern/151304: patch - definitions of variables tested by ASSERT_ATOMIC_LOAD_PTR Date: Sat, 9 Oct 2010 07:16:43 +1100 (EST) On Fri, 8 Oct 2010, Svatopluk Kraus wrote: >> Description: > A macro ASSERT_ATOMIC_LOAD_PTR() hits in colfire port I work on. It is possibly due to use of GNU GCC (4.5.1) compiler -Os option (size optimalization). The macro is applied on four places: Perhaps gcc-4.5.1 -Os is doing invalid packing of structs, or FreeBSD has broken packing of structs using the __packed mistake and gcc-4.5.1 -Os exposes the brokenness by exploiting __packed more. Probably the latter. BTW, gcc-4.2.1 -Os is still completely broken on kernels. It fails to compile some files due to problems with -Wuninitialized, and when this is worked around it produces a kernel that is about 50% larger than one produced by gcc-4.2.1 -O. A few years ago I thought the problem was excessive inlining, but when I tried to fix it a few weeks ago, reducing the inlining caused larger problems and still gave large negative optimizations. >> Fix: > I patch the entries definitions in structures by align attribute, I believe it is better than to do nothing. Moreover, it solved my problem. > > Patch attached with submission follows: > > Index: sys/sys/_rwlock.h > =================================================================== > --- sys/sys/_rwlock.h (revision 213567) > +++ sys/sys/_rwlock.h (working copy) > @@ -37,7 +37,7 @@ > */ > struct rwlock { > struct lock_object lock_object; > - volatile uintptr_t rw_lock; > + volatile uintptr_t rw_lock __aligned(4); > }; > > #endif /* !_SYS__RWLOCK_H_ */ > ... This does nothing good on arches with 64-bit pointers. With gcc-4.2.1 on amd64, it seems to do nothing -- the natural alignment of a uintptr_t on amd64 is 8, and asking for a smaller alignment using __aligned(4) apparently makes no difference. It takes __aligned(more_than_8) or __packed to make a difference. Here is an example of creating an alignment bug due using __packed and just gcc-4.2.1 on amd64: % #include % #include % % struct foo { % int x; % long y __aligned(4); % }; % % struct bar { % char c; % struct foo d; % } __packed; % % int z = offsetof(struct foo, y); % int t = sizeof (struct bar); 'y' has the correct offset of 8, but `struct bar' has size 17, so the `y' nested in it cannot possibly be aligned properly, at least if there is an array of `struct bar' with at least 2 elements. I think this is a compiler bug -- the explicit __aligned(4) in the nested struct should force alignment of container structs (but only to 4 here, not the 8 that is required). Bruce