svn commit: r203343 - head/sys/netinet

Bruce Evans brde at optusnet.com.au
Tue Feb 2 06:07:51 UTC 2010


On Mon, 1 Feb 2010, M. Warner Losh wrote:

> In message: <20100202012830.B1230 at besplex.bde.org>
>            Bruce Evans <brde at optusnet.com.au> writes:
> : Rev.1.30 attempted to fix this by also adding the __aligned(4)
> : attribute.
> : I haven't checked that this works.  It adds additional unportability
> : and
> : ugliness.
>
> __packed is necessary for ARM where the rules of the ABI we use are
> such that this structure isn't properly packed otherwise.[*]

Is arm's broken ABI really that broken?  _This_ struct's correct size
is a multiple of 4, so it isn't affected by arm's padding at the end.
It takes a broken ABI for bi-fields to give a problem, but arm is not
that broken AFAIK (**).  I think __packed is just FUD for this struct.

> __aligned(4) tells the compiler that pointers to this structure are
> 4-byte aligned, which helps on many platforms generate more optimal
> code.
>
> The root cause of all this mess is that 'C' doesn't have any good,
> easy ways to specify "on the wire" formats of structures.  So we have
> to play these games to make the headers we snarfed off the wire match
> the C structs that we're using to access the fields in those
> structures.  We are necessarily in unportability land here.  It is
> only by the chance alignment of the x86 ABI and the wire format that
> the project was able to ignore the issue for so long...

Not really chance.  No reasonable ABI requires excessive alignment.
This is depended on in 100's of other places, especially in device
drivers.  Some of the other places don't even use __packed.  Their
problems on arm have apparently been limited by a combination of
not using bit-fields and

> [*] Of course, it could be argued that ARM should use a different
> and/or better ABI for newer cores, and I'd agree with that...  But
> until that's fixed, we have to cope.

Even while waiting for the fix, it would be nice to use a local fix
-munbroken-alignment, in one MD place (Makefile.arm, although that
won't work for headers that are (ab)used in userland), instead of
engrotting 100's of "MI" places hacks like __packed.  Of course it's
a hack to use a struct at all.

(**) I couldn't find any documentation of the alignment for bit-fields
in gcc.info, except this for powerpc:

% `-mno-bit-align'
% `-mbit-align'
%      On System V.4 and embedded PowerPC systems do not (do) force
%      structures and unions that contain bit-fields to be aligned to the
%      base type of the bit-field.
% 
%      For example, by default a structure containing nothing but 8
%      `unsigned' bit-fields of length 1 would be aligned to a 4 byte
%      boundary and have a size of 4 bytes.  By using `-mno-bit-align',
%      the structure would be aligned to a 1 byte boundary and be one
%      byte in size.

This says that the -mno-bit-align option may be used to turn off excessive
alignment for the struct, but only for powerpc.  Reading between the lines,
it says that powerpc normally has the same behaviour as i386 -- it is
not so broken as to align the individual 1-bit bit-fields to a 4-byte
boundary.  Whether it adds 3 bytes of useless padding at the end of the
bit-fields to pad them to sizeof(unsigned) is less clear.

Bruce


More information about the svn-src-all mailing list