Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 13 Nov 2006 02:38:07 +0300
From:      Ruslan Ermilov <ru@freebsd.org>
To:        Sam Leffler <sam@errno.com>
Cc:        arm@freebsd.org, current@freebsd.org
Subject:   Re: [head tinderbox] failure on arm/arm
Message-ID:  <20061112233806.GD45238@rambler-co.ru>
In-Reply-To: <4557825E.3070009@errno.com>
References:  <20061112144230.GC2331@kobe.laptop> <20061112145151.GC49703@rambler-co.ru> <20061112151150.GA2988@kobe.laptop> <20061112155723.GB50349@rambler-co.ru> <20061112165904.GP6501@plum.flirble.org> <20061112171436.GF50349@rambler-co.ru> <20061112180758.GD4237@kobe.laptop> <20061112132105.6bac38d6@kan.dnsalias.net> <20061112192810.GC1173@rambler-co.ru> <4557825E.3070009@errno.com>

next in thread | previous in thread | raw e-mail | index | archive | help

--E13BgyNx05feLLmH
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Sun, Nov 12, 2006 at 12:21:50PM -0800, Sam Leffler wrote:
> Ruslan Ermilov wrote:
> > On Sun, Nov 12, 2006 at 01:21:05PM -0500, Alexander Kabaev wrote:
> >> GCC expects 4-byte aligned structured on ARM but does not necessarily
> >> have to. We can change the default at the expense of possible more
> >> inefficient code generated and lost binary compatibility with other ARM
> >> OSes out there. So this is trade off between unclear performance
> >> penalty and an unspecified but certainly sizable number of other
> >> landmines like this lurking on the code.
> >>
> >> We should decide what evil we regard as lesser.
> >>
> > This is the only buildworld problem so far on FreeBSD/ARM, so my
> > feeling is that we can actually benefit from leaving it "as is",
> > as it has a potential of making our code more portable.  Of course
> > if binary compatibility for structs across platforms is an issue,
> > a structure should be "packed", because otherwise the C standard
> > says that "Each non-bit-field member of a structure or union object
> > is aligned in an implementation-defined manner appropriate to its
> > type."
> >=20
> > On the other hand, having GCC align "struct foo { char x[11]; }"
> > on a four-byte boundary (other than for backward compatibility)
> > doesn't make too much sense to me.
> >=20
> > I don't know GCC rules for alignment of structure members.  For
> > example, if it's guaranteed (in GCC) that offsetof(struct foo, bar)
> > is always 1 for "struct foo { char foo; char bar; }" (without the
> > "packed" attribute) on all platforms and OSes GCC supports?
> > I'd expect the latter to be "4" for FreeBSD/ARM but fortunately
> > it stays "1", i.e., only the structure alignment is affected,
> > and not of structure members (which is POLA but makes the 4 byte
> > for structure alignment questionable).
>=20
> This issue appears to have broken if_bridge.  On my avila board I align
> rx packets to be aligned s.t. the ip header lands on a 32-bit boundary.
>  This results in the ethernet header being 2-byte aligned which is ok on
> other platforms.  But the compiler takes this code in bridge_forward:
>=20
>         /*
>          * If the interface is learning, and the source
>          * address is valid and not multicast, record
>          * the address.
>          */
>         if ((bif->bif_flags & IFBIF_LEARNING) !=3D 0 &&
>             ETHER_IS_MULTICAST(eh->ether_shost) =3D=3D 0 &&
>             (eh->ether_shost[0] =3D=3D 0 &&
>              eh->ether_shost[1] =3D=3D 0 &&
>              eh->ether_shost[2] =3D=3D 0 &&
>              eh->ether_shost[3] =3D=3D 0 &&
>              eh->ether_shost[4] =3D=3D 0 &&
>              eh->ether_shost[5] =3D=3D 0) =3D=3D 0) {
>                 (void) bridge_rtupdate(sc, eh->ether_shost,
>                     src_if, 0, IFBAF_DYNAMIC);
>         }
>=20
> and converts the 6 byte compares to a 32-bit and 16-bit compare.  Since
> the data is only 16-bit aligned the 32-bit load faults.
>=20
> So the point is that just because things compile doesn't necessarily
> mean they work.  And worse code that works on many/most other
> architectures may not work.
>=20
No, this is only because our kernels are compiled without -Wcast-align.
Otherwise it notices that mtod() casts "char *" into "struct ether_header *=
":

$ pwd              =20
/home/ru/w/freebsd/src/sys/modules/if_bridge
$ make WARNS=3D6 2>&1 |grep -A1 forward
/home/ru/w/freebsd/src/sys/modules/if_bridge/../../net/if_bridge.c: In func=
tion `bridge_forward':
/home/ru/w/freebsd/src/sys/modules/if_bridge/../../net/if_bridge.c:1888: wa=
rning: cast increases required alignment of target type


Cheers,
--=20
Ruslan Ermilov
ru@FreeBSD.org
FreeBSD committer

--E13BgyNx05feLLmH
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (FreeBSD)

iD8DBQFFV7BeqRfpzJluFF4RAkKgAJ983jddAbAprxVnOPJiouzXozTMmwCaAjGN
n1Mu+nBMZUMsWhyS8loG/cI=
=EgaF
-----END PGP SIGNATURE-----

--E13BgyNx05feLLmH--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20061112233806.GD45238>