kern/151304: patch - definitions of variables tested by ASSERT_ATOMIC_LOAD_PTR

[Bruce Evans]

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 <sys/cdefs.h>
% #include <stddef.h>
% 
% 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