drivers/net/can/spi/mcp251xfd/mcp251xfd.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
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.
Because this is a false positive, I did not add a Fixes tag, nor a
Reported-by: kernel test robot.
---
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.1Hi 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
On Tue. 19 May 2022 at 01:08, Nathan Chancellor <nathan@kernel.org> wrote:
> 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).
Great, thanks for the check! Does this mean we can add you Tested-by
(I assume yes, c.f. below, if not the case, please raise your voice).
> > 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.
ACK.
Marc, can you directly add below tags to the patch:
Reported-by: kernel test robot <lkp@intel.com>
Tested-by: Nathan Chancellor <nathan@kernel.org>
Or do you want me to send a v2?
Yours sincerely,
Vincent Mailhol
On Thu, May 19, 2022 at 01:15:04AM +0900, Vincent MAILHOL wrote:
> On Tue. 19 May 2022 at 01:08, Nathan Chancellor <nathan@kernel.org> wrote:
> > 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).
>
> Great, thanks for the check! Does this mean we can add you Tested-by
> (I assume yes, c.f. below, if not the case, please raise your voice).
Sure, see below.
> > > 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.
>
> ACK.
> Marc, can you directly add below tags to the patch:
>
> Reported-by: kernel test robot <lkp@intel.com>
> Tested-by: Nathan Chancellor <nathan@kernel.org>
Please use:
Tested-by: Nathan Chancellor <nathan@kernel.org> # build
To make it clear that I didn't perform anything more than a build test
to see that the warning is fixed.
Cheers,
Nathan
On 18.05.2022 09:18:19, Nathan Chancellor wrote: > > > > 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. > > > > ACK. > > Marc, can you directly add below tags to the patch: > > > > Reported-by: kernel test robot <lkp@intel.com> > > Tested-by: Nathan Chancellor <nathan@kernel.org> > > Please use: > > Tested-by: Nathan Chancellor <nathan@kernel.org> # build > > To make it clear that I didn't perform anything more than a build test > to see that the warning is fixed. I've updated the tags! Thanks, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Embedded Linux | https://www.pengutronix.de | Vertretung West/Dortmund | Phone: +49-231-2826-924 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
On 18.05.2022 20:43:57, 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>
Thank! Applies to linux-can-next/testing.
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung West/Dortmund | Phone: +49-231-2826-924 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
© 2016 - 2026 Red Hat, Inc.