Date: Fri, 6 Jun 2008 22:15:05 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: "Bruce M. Simpson" <bms@freebsd.org> Cc: freebsd-net@freebsd.org, =?ISO-8859-1?Q?Marc_L=F6rner?= <marc.loerner@hob.de> Subject: Re: Probable Bug in tcp.h Message-ID: <20080606212657.X16250@delplex.bde.org> In-Reply-To: <48480FA5.3020800@FreeBSD.org> References: <200806051712.47048.marc.loerner@hob.de> <48480FA5.3020800@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
This message is in MIME format. The first part should be readable text,
while the remaining parts are likely unreadable without MIME-aware tools.
--0-263961645-1212754505=:16250
Content-Type: TEXT/PLAIN; charset=X-UNKNOWN; format=flowed
Content-Transfer-Encoding: QUOTED-PRINTABLE
On Thu, 5 Jun 2008, Bruce M. Simpson wrote:
> Marc L=F6rner wrote:
>> ..
>> First of all I have the problam of misalignment of th_off. Because in th=
is=20
>> way always 4 bytes are read and the the bits of th_off are replaced. The=
n=20
>> the 4 bytes are written back.=20
>> But should (th_x and th_off) not only be 1 byte in whole -> only read an=
d=20
>> write 1 byte?
1 or 2, since struct tcphdr must be 16-bit aligned due to the shorts in it.
An unportability in gcc's treatment of bit-fields results in gcc thinking
that the struct is 4-byte aligned. Thus alignment traps may occur if
an instance of the struct is not in fact aligned.
> Which machine architecture are you attempting to compile this code on?
[ia64].
> On FreeBSD Tier 1 platforms, the access is probably going to come out of =
L2=20
> cache anyway, so the fields in question will be read by a burst cycle.
>
> It is worth noting that NetBSD changed the base type of tcphdr's bitfield=
s to=20
> uint8_t, however this shuffles the compiler dependency into the treatment=
of=20
> the "char" type. Most modern C compilers support "unsigned char".
No C compilers support bit-fields of type uint8_t (unless _Bool is
uint8_t), since C requires bit-fields to have type a possibly-qualified
version of _Bool, signed int or insigned int. Howver, the behaviour
for other types is unspecified, so the compiler can do anything with
them including what broken software wants, so it is really no C programs
that use bit-fields of type uint8_t.
Bit-fields of type larger than u_int are useful for holding more bits
than a u_int (since C unfortunately limits the number of bits in a
bit-field to the number in the type of the bit-field, so you can't
write u_int foo:64 to get a 64-bit integer), but bit-fields of type
smaller than u_int are not very useful -- compilers are free to treat
them the same as bit fields of larger type.
gcc's actual treatment of bit-fields of type smaller than u_int is unportab=
le
and apparently undocumented. It affects alignment and the struct size but
not padding: e.g., in the following struct:
=09struct {
=09=09unsigned x:1;
=09=09char y;
=09} foo;
This allocates 1 bit for x and pads to a byte boundary, so that y has
offset 1. Any more padding would be bogus. Then, bogusly, due to the
bit-field having type unsigned, gcc makes the whole struct have alignment
requirements that of u_int and pads the struct at the end to have size
a multiple of sizeof(u_int) =3D 32 bits. Then it is justified as accessing
foo.x as part of an an aligned object of size 32 bits, and may trap
if foo.x is not actually aligned. Something like this happens for
struct tcphdr, with the alignment trap actually ocurring on ia64. Some
bug elsewhere is responsible for generating a misaligned struct.
OTOH, if `unsigned' in the above is changed to `unsigned char', giving a
non-C program:
=09struct {
=09=09unsigned char x:1;
=09=09char y;
=09} foo;
then all the padding is non-bogus when this is compiled by the non-C
compiler gcc: the offset of y is unchanged, but now the struct has size 2
and alignment 1.
Due to unportabilities like this, bit-fields should never be used when
the layout of an object must be controlled precisely. This includes
layouts related to hardware which includes most network layouts.
Unportabilities like this are sometimes hacked around using packed structs.
ipv6 headers do this a lot. ipv4 headers are not so bad. ip.h now says
`__packed __aligned(4)' for struct ip where it used to say just `__packed'
__packed forces 1-byte alignment for everything. On ia64, this results
in specially pessimized accesses for all the fields in a struct, with 1-
byte accesses even for the 32-bit fields and large code to combine the
bytes into a word. But struct ip (unlike struct tcphdr?) must always
be 32-bit aligned, and declaring it as __aligned(4) is supposed to restore
the possibility of wide accesses after declaration it as __packed forces it
to have no padding. No padding should occur automatically, and the u_int
bit-fields don't affect this since the size of struct ip is 20, but there
is a another problem with bogus padding at the end (on arm only?).
Bruce
--0-263961645-1212754505=:16250--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080606212657.X16250>