From patchwork Tue Jul 7 17:17:13 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Leif Lindholm X-Patchwork-Id: 50848 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-wg0-f70.google.com (mail-wg0-f70.google.com [74.125.82.70]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id B249D22A03 for ; Tue, 7 Jul 2015 17:23:00 +0000 (UTC) Received: by wgjx7 with SMTP id x7sf63058649wgj.3 for ; Tue, 07 Jul 2015 10:22:59 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:date:from:to:subject:message-id :references:mime-version:content-disposition:in-reply-to:user-agent :cc:precedence:reply-to:list-id:list-unsubscribe:list-archive :list-post:list-help:list-subscribe:content-type :content-transfer-encoding:errors-to:sender:x-original-sender :x-original-authentication-results:mailing-list; bh=RdkN2PTbGti7ZhR7GsL8h27lgrYPfH4sKa8AARJh4dk=; b=ARndJrA3+r/RVz2kphkFAvSYPlIPEARX9RAW4jMjx5lGyDVxXOWatjihxwowTOQofF RvN4on9+wyF6W6cUObgeIf1nHjO05TV1NggwalVa2I6cVC8nFCINMXHK0035+c9/5Y3M /o3F09DUPB0qFjcUgVsTBbUT+fAxAb/ub4XyBLwHgJfTV3K4ileJFJlvYwgPlxFNMQ0J FcQ/9LYUBgz5VERdLm9TJA1TTeICzN3enimDKBkWrPNZzteFckfIL3HeX4jl0tAk4V8l a0uQbC4LD2sTXViPK8VGEU4WVYYJLkqphYP4v2gSIkC03W55VAHt9gxxQ5ITmh6KHeEZ Xvyw== X-Gm-Message-State: ALoCoQkZqdGewoeRxtwki1/oAmpfPy7IYwmFDAb19tvjHLcVzfZwJzPtWhQIHoNs94t8dHi7PoJ7 X-Received: by 10.112.189.131 with SMTP id gi3mr2598482lbc.6.1436289779902; Tue, 07 Jul 2015 10:22:59 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.152.44.228 with SMTP id h4ls836003lam.23.gmail; Tue, 07 Jul 2015 10:22:59 -0700 (PDT) X-Received: by 10.112.56.139 with SMTP id a11mr5031302lbq.90.1436289779743; Tue, 07 Jul 2015 10:22:59 -0700 (PDT) Received: from mail-la0-f50.google.com (mail-la0-f50.google.com. [209.85.215.50]) by mx.google.com with ESMTPS id xs2si18609273lac.134.2015.07.07.10.22.59 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 07 Jul 2015 10:22:59 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.50 as permitted sender) client-ip=209.85.215.50; Received: by laar3 with SMTP id r3so205004235laa.0 for ; Tue, 07 Jul 2015 10:22:59 -0700 (PDT) X-Received: by 10.112.126.101 with SMTP id mx5mr5080851lbb.35.1436289779597; Tue, 07 Jul 2015 10:22:59 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.108.230 with SMTP id hn6csp2533590lbb; Tue, 7 Jul 2015 10:22:57 -0700 (PDT) X-Received: by 10.68.235.38 with SMTP id uj6mr11097978pbc.57.1436289777164; Tue, 07 Jul 2015 10:22:57 -0700 (PDT) Received: from lists.gnu.org (lists.gnu.org. [2001:4830:134:3::11]) by mx.google.com with ESMTPS id gw3si35696974pac.117.2015.07.07.10.22.56 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Tue, 07 Jul 2015 10:22:57 -0700 (PDT) Received-SPF: pass (google.com: domain of grub-devel-bounces+patch=linaro.org@gnu.org designates 2001:4830:134:3::11 as permitted sender) client-ip=2001:4830:134:3::11; Received: from localhost ([::1]:59663 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZCWa7-0003yn-60 for patch@linaro.org; Tue, 07 Jul 2015 13:22:55 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57371) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZCWUj-0001x5-NW for grub-devel@gnu.org; Tue, 07 Jul 2015 13:17:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZCWUg-0006oo-9G for grub-devel@gnu.org; Tue, 07 Jul 2015 13:17:21 -0400 Received: from mail-wi0-f174.google.com ([209.85.212.174]:32897) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZCWUg-0006le-3t for grub-devel@gnu.org; Tue, 07 Jul 2015 13:17:18 -0400 Received: by wiwl6 with SMTP id l6so321243700wiw.0 for ; Tue, 07 Jul 2015 10:17:16 -0700 (PDT) X-Received: by 10.180.208.7 with SMTP id ma7mr107208007wic.0.1436289436772; Tue, 07 Jul 2015 10:17:16 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by mx.google.com with ESMTPSA id d5sm910644wiz.24.2015.07.07.10.17.15 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Tue, 07 Jul 2015 10:17:15 -0700 (PDT) Date: Tue, 7 Jul 2015 18:17:13 +0100 From: Leif Lindholm To: The development of GNU GRUB Subject: Re: [PATCH] zfs: fix compilation failure with clang due to alignment Message-ID: <20150707171713.GB8790@bivouac.eciton.net> References: <1435950347-20073-1-git-send-email-arvidjaar@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1435950347-20073-1-git-send-email-arvidjaar@gmail.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.85.212.174 Cc: jack@suse.cz X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: List-Unsubscribe: , List-Archive: List-Post: , List-Help: , List-Subscribe: , Errors-To: grub-devel-bounces+patch=linaro.org@gnu.org Sender: grub-devel-bounces+patch=linaro.org@gnu.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: leif.lindholm@linaro.org X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.215.50 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 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 required > alignment from 1 to 8 [-Werror,-Wcast-align] > grub_uint64_t *keys = (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? 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. 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 match the GRUB_PACKED for structs. Should we? If so, something like the following could be added to your patch for a more complete fix: / 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, struct grub_xfs_dir2_entry *de) > return (struct grub_xfs_dir2_entry *)(((char *)de) + ALIGN_UP(size, 8)); > } > > -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 = (grub_uint64_t *)(leaf + 1); > + char *keys = (char *)leaf + sizeof (*leaf); > > if (data->hascrc) > - keys += 6; /* skip crc, uuid, ... */ > + keys += 6 * sizeof (grub_uint64_t); /* skip crc, uuid, ... */ > return keys; > } > > -- > 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 --- 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 == XFS_INODE_FORMAT_BTREE) { struct grub_xfs_btree_root *root; - const grub_uint64_t *keys; + const grub_uint64_t *keys GRUB_UNALIGNED; int recoffset; leaf = 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 +#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