From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1ZFOv5-0003ju-76 for mharc-grub-devel@gnu.org; Wed, 15 Jul 2015 11:48:27 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58260) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFOv1-0003iJ-6B for grub-devel@gnu.org; Wed, 15 Jul 2015 11:48:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZFOuz-0001ky-W6 for grub-devel@gnu.org; Wed, 15 Jul 2015 11:48:23 -0400 Received: from mail-wi0-x233.google.com ([2a00:1450:400c:c05::233]:34972) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZFOuz-0001ia-Kt for grub-devel@gnu.org; Wed, 15 Jul 2015 11:48:21 -0400 Received: by wiga1 with SMTP id a1so3762815wig.0 for ; Wed, 15 Jul 2015 08:48:20 -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=oFpfZPygsMFDIjtoaQziGJOkuuEk1h6zFdpBNqsItNc=; b=UpHlEhDbikDdBg0dShZ12f6HjYT1cQtQlVlFGfL7HVGrHDgZHVb25imTHAU+fkKPF8 JJtrG3nBhl1/GLFOraVMkhnc0uD1mC8oB+Ff7XKQ4IOWO4vKk/Ps0ymQu0IK1rS4J6vp Hbkh4WbOPB8zyawXi1kJc8VGXe/qHbPpcRJS6CozdKFoH/u9X267xvBslLWC4iHTm64J Z3cScDWJ/oHCWW4ra5LPC+ReQuu4jBXqx4r+8JqdP6etDaY1Zz7/vbs/fBW8DWyw8g9c hZ4QqEQIumyEyp50pcFYpIlPtgrr4IlGDj5hjspKFkNhMVUXBi/P9K7RgHVCEEayKxvZ 5+GQ== X-Received: by 10.194.120.198 with SMTP id le6mr9680078wjb.133.1436975300817; Wed, 15 Jul 2015 08:48:20 -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 gt10sm328633wib.20.2015.07.15.08.48.19 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 15 Jul 2015 08:48:19 -0700 (PDT) Message-ID: <55A680A6.8000707@gmail.com> Date: Wed, 15 Jul 2015 17:47:50 +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> In-Reply-To: <20150707171713.GB8790@bivouac.eciton.net> Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="7L9wDumQeXVTwGdBpVhCXTPI3Qaq8oxdI" X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2a00:1450:400c:c05::233 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 15:48:25 -0000 This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --7L9wDumQeXVTwGdBpVhCXTPI3Qaq8oxdI Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Go ahead 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 require= d >> 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 fix?= >> Or should I raise it with clang? >=20 > Well, the problem is that struct grub_xfs_btree_node is defined with > GRUB_PACKED - forcing a 1-byte alignment requirement as opposed to the > 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. >=20 > 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. >=20 > 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 match > the GRUB_PACKED for structs. Should we? >=20 > 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 >=20 > / > Leif >=20 >> 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, str= uct grub_xfs_dir2_entry *de) >> return (struct grub_xfs_dir2_entry *)(((char *)de) + ALIGN_UP(size,= 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 >=20 > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel >=20 --7L9wDumQeXVTwGdBpVhCXTPI3Qaq8oxdI 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 iF4EAREKAAYFAlWmgMMACgkQmBXlbbo5nOtRawEAnY0/0i6KFoy6DZ8lKNDDi1b9 WehVcqDkAgHVgm1/XGgA/2XECAUKSgIul6LMcgkAbSxw2RVPfm4rWZTt+NnJDfaV =8OEo -----END PGP SIGNATURE----- --7L9wDumQeXVTwGdBpVhCXTPI3Qaq8oxdI--