From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1ZFg4Z-0004FD-Sm for mharc-grub-devel@gnu.org; Thu, 16 Jul 2015 06:07:23 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33470) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFg4W-0004DG-77 for grub-devel@gnu.org; Thu, 16 Jul 2015 06:07:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZFg4S-0001XJ-Sf for grub-devel@gnu.org; Thu, 16 Jul 2015 06:07:20 -0400 Received: from mail-wi0-x22a.google.com ([2a00:1450:400c:c05::22a]:37087) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFg4S-0001X5-E7 for grub-devel@gnu.org; Thu, 16 Jul 2015 06:07:16 -0400 Received: by wibud3 with SMTP id ud3so11451675wib.0 for ; Thu, 16 Jul 2015 03:07:15 -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=SEVxIgzltILtE7ff9cvBTzAZtM3bfhed4KEUcd0lT8k=; b=o5FukSHfsO76TholyjCAHLW4SeNQD0U2YvzbkRi1RmL8mmgLdWQTpInhdOSMrHhl22 u4iOhSphSWhyib9Jeexh2qx58kVSujeLm0+L+TNNaG6ux0ty6O2nDG5zPvwr5rFRw8vL V9y6L5CATdcX/9dl6BvX5XCo3uEl+/t5ij33nh+kgUK5G+hZrK/UrapgxXkbh0kgS/cY U1aeGM2miF5JNwPLOQH7Ku9jA/i4d+UAahc8r3LCR+44uXCDyomUSkCU2lINbu/jikUm oG5OuF4oBqbczcXkUFDxdXl+rkFjL3HUfzOTE15WcYqIZ0MsrOqZsctQdkTYnAstKqNq XB7w== X-Received: by 10.180.75.78 with SMTP id a14mr3645821wiw.43.1437041235775; Thu, 16 Jul 2015 03:07:15 -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 fa8sm2269035wib.14.2015.07.16.03.07.14 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 16 Jul 2015 03:07:14 -0700 (PDT) Message-ID: <55A78252.6080804@gmail.com> Date: Thu, 16 Jul 2015 12:07:14 +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="VuwgJhqvQ0dT8qs3xXvhbfCvBXomaIJo0" X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2a00:1450:400c:c05::22a 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: Thu, 16 Jul 2015 10:07:22 -0000 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --VuwgJhqvQ0dT8qs3xXvhbfCvBXomaIJo0 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 Actually I think this patch doesn't work as __attribute__((aligned(X))) can only increase alignment, not decrease it. I'm going to fix it by using char * and grub_get_unaligned64 >> 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 --VuwgJhqvQ0dT8qs3xXvhbfCvBXomaIJo0 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 iF4EAREKAAYFAlWnglIACgkQmBXlbbo5nOsQlAD/XWb3RQhQQeyR2BUzfOjnF/sc M5Kj5/Arq4F3qkFJw0EA/R3kpZfe7gY3RrwPoo0KnrlhOJL5o57QQneKp+MMo/Ip =cVNS -----END PGP SIGNATURE----- --VuwgJhqvQ0dT8qs3xXvhbfCvBXomaIJo0--