From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1ZFQiR-0003id-0D for mharc-grub-devel@gnu.org; Wed, 15 Jul 2015 13:43:31 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45911) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFQiO-0003iU-5Y for grub-devel@gnu.org; Wed, 15 Jul 2015 13:43:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZFQiM-0007yr-W1 for grub-devel@gnu.org; Wed, 15 Jul 2015 13:43:28 -0400 Received: from mail-wi0-x232.google.com ([2a00:1450:400c:c05::232]:34249) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFQiM-0007xr-Ld for grub-devel@gnu.org; Wed, 15 Jul 2015 13:43:26 -0400 Received: by wibud3 with SMTP id ud3so8981434wib.1 for ; Wed, 15 Jul 2015 10:43:26 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=message-id:date:from:user-agent:mime-version:to:subject:references :in-reply-to:content-type; bh=o2lpLW8FcP4R4xR86w9DKlaEtUbKBKCv97xG9VSEOLI=; b=aWg9tLDubjIM4J4LEHSpICy6j0SHJcW/DO5OK0PVN1Lt2vW1R1GueLWSJvWAwjzQqa JZPptGk53PKlZ51p8Hj5wcYOynFMUShlfaaKRjv/kGIoZeRg3Ewx57RO+9swz0yqYLpc tVeqn97PEHnKxruFE9Lxp0P6swzjrIfEi87iySOptyBo1FngYmTgqbp+XZQMg9Gx7Rw/ qUdeek/rdoWiic/ScDL2L5Ayzb14GcTIxPR2TZpHUHCN4i8MotAW2hL6+Voxhba0fUYj isBn3Ba/gzJDBNTtucTsBDGNYUfYs88K7ZZmngD4ZmzSMxJPP5ISRz02BhT8Nzjxzf+5 2z4w== X-Received: by 10.194.82.167 with SMTP id j7mr10634230wjy.123.1436982205995; Wed, 15 Jul 2015 10:43:25 -0700 (PDT) Received: from ?IPv6:2620:0:105f:fd00:863a:4bff:fe50:abc4? ([2620:0:105f:fd00:863a:4bff:fe50:abc4]) by smtp.gmail.com with ESMTPSA id df1sm999871wib.12.2015.07.15.10.43.25 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 15 Jul 2015 10:43:25 -0700 (PDT) Message-ID: <55A69BBC.1070604@gmail.com> Date: Wed, 15 Jul 2015 19:43:24 +0200 From: =?UTF-8?B?VmxhZGltaXIgJ8+GLWNvZGVyL3BoY29kZXInIFNlcmJpbmVua28=?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.7.0 MIME-Version: 1.0 To: The development of GNU GRUB Subject: Re: [PATCH] zfs: fix compilation failure with clang due to alignment References: <1435950347-20073-1-git-send-email-arvidjaar@gmail.com> <20150707171713.GB8790@bivouac.eciton.net> <55A680A6.8000707@gmail.com> <20150715165228.GH8790@bivouac.eciton.net> In-Reply-To: <20150715165228.GH8790@bivouac.eciton.net> Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="coO28eS9WkQGPhuQ9KG4eWSdR1SifO0qp" X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2a00:1450:400c:c05::232 X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 15 Jul 2015 17:43:29 -0000 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --coO28eS9WkQGPhuQ9KG4eWSdR1SifO0qp Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 15.07.2015 18:52, Leif Lindholm wrote: > On Wed, Jul 15, 2015 at 05:47:50PM +0200, Vladimir '=CF=86-coder/phcode= r' Serbinenko wrote: >> Go ahead >=20 > The below was more of an RFC than something committable - are you OK > with me splitting the types.h changes out as a separate patch? >=20 Yes >> On 07.07.2015 19:17, Leif Lindholm wrote: >>> On Fri, Jul 03, 2015 at 10:05:47PM +0300, Andrei Borzenkov wrote: >>>> I do not claim I understand why clang complains, but this patch does= >>>> fix it. >>>> >>>> fs/xfs.c:452:25: error: cast from 'struct grub_xfs_btree_node *' to >>>> 'grub_uint64_t *' (aka 'unsigned long long *') increases requi= red >>>> alignment from 1 to 8 [-Werror,-Wcast-align] >>>> grub_uint64_t *keys =3D (grub_uint64_t *)(leaf + 1); >>>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~ >>>> 1 error generated. >>>> >>>> --- >>>> >>>> Jan, do you have any idea what's wrong and whether this is proper fi= x? >>>> Or should I raise it with clang? >>> >>> Well, the problem is that struct grub_xfs_btree_node is defined with >>> GRUB_PACKED - forcing a 1-byte alignment requirement as opposed to th= e >>> 8-byte requirement that would naturally be enforced by the struct >>> contents. And apparently clang objects to this, whereas gcc thinks >>> everything is fine ... even though -Wcast-align is explicitly used. >>> >>> Now, grub_xfs_btree_keys() is only called by grub_xfs_read_block(), >>> where it is immediately stuffed back into another 8-byte aligned >>> pointer. And then the alignment is immediately discarded again by >>> casting it to a (char *) for an arithmetic operation. >>> >>> If the alignment is indeed not required, it may be worth explicitly >>> marking that pointer as one to a potentially unaligned location. >>> But we don't currently appear to have a GRUB_UNALIGNED macro, to matc= h >>> the GRUB_PACKED for structs. Should we? >>> >>> If so, something like the following could be added to your patch for = a >>> more complete fix: >>> --- a/grub-core/fs/xfs.c >>> +++ b/grub-core/fs/xfs.c >>> @@ -488,7 +488,7 @@ grub_xfs_read_block (grub_fshelp_node_t node, >>> grub_disk_addr_t fileblock) >>> if (node->inode.format =3D=3D XFS_INODE_FORMAT_BTREE) >>> { >>> struct grub_xfs_btree_root *root; >>> - const grub_uint64_t *keys; >>> + const grub_uint64_t *keys GRUB_UNALIGNED; >>> int recoffset; >>> =20 >>> leaf =3D grub_malloc (node->data->bsize); >>> diff --git a/include/grub/types.h b/include/grub/types.h >>> index e732efb..720e236 100644 >>> --- a/include/grub/types.h >>> +++ b/include/grub/types.h >>> @@ -30,6 +30,8 @@ >>> #define GRUB_PACKED __attribute__ ((packed)) >>> #endif >>> =20 >>> +#define GRUB_UNALIGNED __attribute__ ((aligned (1))) >>> + >>> #ifdef GRUB_BUILD >>> # define GRUB_CPU_SIZEOF_VOID_P BUILD_SIZEOF_VOID_P >>> # define GRUB_CPU_SIZEOF_LONG BUILD_SIZEOF_LONG >>> >>> / >>> Leif >>> >>>> grub-core/fs/xfs.c | 6 +++--- >>>> 1 file changed, 3 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/grub-core/fs/xfs.c b/grub-core/fs/xfs.c >>>> index 7249291..ea8cf7e 100644 >>>> --- a/grub-core/fs/xfs.c >>>> +++ b/grub-core/fs/xfs.c >>>> @@ -445,14 +445,14 @@ grub_xfs_next_de(struct grub_xfs_data *data, s= truct grub_xfs_dir2_entry *de) >>>> return (struct grub_xfs_dir2_entry *)(((char *)de) + ALIGN_UP(siz= e, 8)); >>>> } >>>> =20 >>>> -static grub_uint64_t * >>>> +static void * >>>> grub_xfs_btree_keys(struct grub_xfs_data *data, >>>> struct grub_xfs_btree_node *leaf) >>>> { >>>> - grub_uint64_t *keys =3D (grub_uint64_t *)(leaf + 1); >>>> + char *keys =3D (char *)leaf + sizeof (*leaf); >>>> =20 >>>> if (data->hascrc) >>>> - keys +=3D 6; /* skip crc, uuid, ... */ >>>> + keys +=3D 6 * sizeof (grub_uint64_t); /* skip crc, uuid, ... */= >>>> return keys; >>>> } >>>> =20 >>>> --=20 >>>> tg: (7a21030..) u/xfs-clang-align (depends on: master) >>>> >>>> _______________________________________________ >>>> Grub-devel mailing list >>>> Grub-devel@gnu.org >>>> https://lists.gnu.org/mailman/listinfo/grub-devel >>> >>> _______________________________________________ >>> Grub-devel mailing list >>> Grub-devel@gnu.org >>> https://lists.gnu.org/mailman/listinfo/grub-devel >>> >> >> >=20 >=20 >=20 >> _______________________________________________ >> Grub-devel mailing list >> Grub-devel@gnu.org >> https://lists.gnu.org/mailman/listinfo/grub-devel >=20 >=20 > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel >=20 --coO28eS9WkQGPhuQ9KG4eWSdR1SifO0qp Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iF4EAREKAAYFAlWmm7wACgkQmBXlbbo5nOvpsAEAidyUkYEA79nqYlFTjmram0NM dcemgt5LVBVgOISAgukA/2HsU3CVUq7vAf0PgzPQECk5Xm1BshSYOHLgNZyOZzvB =eNqC -----END PGP SIGNATURE----- --coO28eS9WkQGPhuQ9KG4eWSdR1SifO0qp--