From: Nathan Chancellor <nathan@kernel.org> To: Vincent Mailhol <mailhol.vincent@wanadoo.fr> Cc: Marc Kleine-Budde <mkl@pengutronix.de>, kernel test robot <lkp@intel.com>, llvm@lists.linux.dev, kbuild-all@lists.01.org, linux-kernel@vger.kernel.org, linux-can@vger.kernel.org Subject: Re: [PATCH] can: mcp251xfd: silence clang's -Wunaligned-access warning Date: Wed, 18 May 2022 09:05:00 -0700 [thread overview] Message-ID: <YoUZLHIbxPu15/lN@dev-arch.thelio-3990X> (raw) In-Reply-To: <20220518114357.55452-1-mailhol.vincent@wanadoo.fr> Hi Vincent, On Wed, May 18, 2022 at 08:43:57PM +0900, Vincent Mailhol wrote: > clang emits a -Wunaligned-access warning on union > mcp251xfd_tx_ojb_load_buf. > > The reason is that field hw_tx_obj (not declared as packed) is being > packed right after a 16 bits field inside a packed struct: > > | union mcp251xfd_tx_obj_load_buf { > | struct __packed { > | struct mcp251xfd_buf_cmd cmd; > | /* ^ 16 bits fields */ > | struct mcp251xfd_hw_tx_obj_raw hw_tx_obj; > | /* ^ not declared as packed */ > | } nocrc; > | struct __packed { > | struct mcp251xfd_buf_cmd_crc cmd; > | struct mcp251xfd_hw_tx_obj_raw hw_tx_obj; > | __be16 crc; > | } crc; > | } ____cacheline_aligned; > > Starting from LLVM 14, having an unpacked struct nested in a packed > struct triggers a warning. c.f. [1]. > > This is a false positive because the field is always being accessed > with the relevant put_unaligned_*() function. Adding __packed to the > structure declaration silences the warning. > > [1] https://github.com/llvm/llvm-project/issues/55520 > > Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr> > --- > Actually, I do not have llvm 14 installed so I am not able to test > (this check was introduced in v14). But as explained in [1], adding > __packed should fix the warning. Thanks for the patch! This does resolve the warning (verified with LLVM 15). > Because this is a false positive, I did not add a Fixes tag, nor a > Reported-by: kernel test robot. I think that the Reported-by tag should always be included but I agree that a Fixes tag is not necessary for this warning, as we currently have it under W=1, so it should not be visible under normal circumstances. > --- > drivers/net/can/spi/mcp251xfd/mcp251xfd.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h > index 1d43bccc29bf..2b0309fedfac 100644 > --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h > +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h > @@ -441,7 +441,7 @@ struct mcp251xfd_hw_tef_obj { > /* The tx_obj_raw version is used in spi async, i.e. without > * regmap. We have to take care of endianness ourselves. > */ > -struct mcp251xfd_hw_tx_obj_raw { > +struct __packed mcp251xfd_hw_tx_obj_raw { > __le32 id; > __le32 flags; > u8 data[sizeof_field(struct canfd_frame, data)]; > -- > 2.35.1 > > Cheers, Nathan
WARNING: multiple messages have this Message-ID (diff)
From: Nathan Chancellor <nathan@kernel.org> To: kbuild-all@lists.01.org Subject: Re: [PATCH] can: mcp251xfd: silence clang's -Wunaligned-access warning Date: Wed, 18 May 2022 09:05:00 -0700 [thread overview] Message-ID: <YoUZLHIbxPu15/lN@dev-arch.thelio-3990X> (raw) In-Reply-To: <20220518114357.55452-1-mailhol.vincent@wanadoo.fr> [-- Attachment #1: Type: text/plain, Size: 2510 bytes --] Hi Vincent, On Wed, May 18, 2022 at 08:43:57PM +0900, Vincent Mailhol wrote: > clang emits a -Wunaligned-access warning on union > mcp251xfd_tx_ojb_load_buf. > > The reason is that field hw_tx_obj (not declared as packed) is being > packed right after a 16 bits field inside a packed struct: > > | union mcp251xfd_tx_obj_load_buf { > | struct __packed { > | struct mcp251xfd_buf_cmd cmd; > | /* ^ 16 bits fields */ > | struct mcp251xfd_hw_tx_obj_raw hw_tx_obj; > | /* ^ not declared as packed */ > | } nocrc; > | struct __packed { > | struct mcp251xfd_buf_cmd_crc cmd; > | struct mcp251xfd_hw_tx_obj_raw hw_tx_obj; > | __be16 crc; > | } crc; > | } ____cacheline_aligned; > > Starting from LLVM 14, having an unpacked struct nested in a packed > struct triggers a warning. c.f. [1]. > > This is a false positive because the field is always being accessed > with the relevant put_unaligned_*() function. Adding __packed to the > structure declaration silences the warning. > > [1] https://github.com/llvm/llvm-project/issues/55520 > > Signed-off-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr> > --- > Actually, I do not have llvm 14 installed so I am not able to test > (this check was introduced in v14). But as explained in [1], adding > __packed should fix the warning. Thanks for the patch! This does resolve the warning (verified with LLVM 15). > Because this is a false positive, I did not add a Fixes tag, nor a > Reported-by: kernel test robot. I think that the Reported-by tag should always be included but I agree that a Fixes tag is not necessary for this warning, as we currently have it under W=1, so it should not be visible under normal circumstances. > --- > drivers/net/can/spi/mcp251xfd/mcp251xfd.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h > index 1d43bccc29bf..2b0309fedfac 100644 > --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd.h > +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd.h > @@ -441,7 +441,7 @@ struct mcp251xfd_hw_tef_obj { > /* The tx_obj_raw version is used in spi async, i.e. without > * regmap. We have to take care of endianness ourselves. > */ > -struct mcp251xfd_hw_tx_obj_raw { > +struct __packed mcp251xfd_hw_tx_obj_raw { > __le32 id; > __le32 flags; > u8 data[sizeof_field(struct canfd_frame, data)]; > -- > 2.35.1 > > Cheers, Nathan
next prev parent reply other threads:[~2022-05-18 16:05 UTC|newest] Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-05-18 6:45 drivers/net/can/spi/mcp251xfd/mcp251xfd.h:481:34: warning: field hw_tx_obj within 'struct mcp251xfd_tx_obj_load_buf::(unnamed at drivers/net/can/spi/mcp251xfd/mcp251xfd.h:479:2)' is less aligned than 'struct mcp251xfd_hw_tx_obj_raw' and is usually due to kernel test robot 2022-05-18 7:05 ` Marc Kleine-Budde 2022-05-18 7:05 ` Marc Kleine-Budde 2022-05-18 11:43 ` [PATCH] can: mcp251xfd: silence clang's -Wunaligned-access warning Vincent Mailhol 2022-05-18 11:43 ` Vincent Mailhol 2022-05-18 11:58 ` Marc Kleine-Budde 2022-05-18 11:58 ` Marc Kleine-Budde 2022-05-18 16:05 ` Nathan Chancellor [this message] 2022-05-18 16:05 ` Nathan Chancellor 2022-05-18 16:15 ` Vincent MAILHOL 2022-05-18 16:15 ` Vincent MAILHOL 2022-05-18 16:18 ` Nathan Chancellor 2022-05-18 16:18 ` Nathan Chancellor 2022-05-18 20:15 ` Marc Kleine-Budde 2022-05-18 20:15 ` Marc Kleine-Budde
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=YoUZLHIbxPu15/lN@dev-arch.thelio-3990X \ --to=nathan@kernel.org \ --cc=kbuild-all@lists.01.org \ --cc=linux-can@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=lkp@intel.com \ --cc=llvm@lists.linux.dev \ --cc=mailhol.vincent@wanadoo.fr \ --cc=mkl@pengutronix.de \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.