Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

mtd: spi-nor: avoid holes in struct spi_mem_op

gcc gets confused when -ftrivial-auto-var-init=pattern is used on sparse
bit fields such as 'struct spi_mem_op', which caused the previous false
positive warning about an uninitialized variable:

drivers/mtd/spi-nor/spansion.c: error: 'op' is used uninitialized [-Werror=uninitialized]

In fact, the variable is fully initialized and gcc does not see it being
used, so the warning is entirely bogus. The problem appears to be
a misoptimization in the initialization of single bit fields when the
rest of the bytes are not initialized.

A previous workaround added another initialization, which ended up
shutting up the warning in spansion.c, though it apparently still happens
in other files as reported by Peter Foley in the gcc bugzilla. The
workaround of adding a fake initialization seems particularly bad
because it would set values that can never be correct but prevent the
compiler from warning about actually missing initializations.

Revert the broken workaround and instead pad the structure to only
have bitfields that add up to full bytes, which should avoid this
behavior in all drivers.

I also filed a new bug against gcc with what I found, so this can
hopefully be addressed in future gcc releases. At the moment, only
gcc-12 and gcc-13 are affected.

Cc: Peter Foley <pefoley2@pefoley.com>
Cc: Pedro Falcato <pedro.falcato@gmail.com>
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110743
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108402
Link: https://godbolt.org/z/efMMsG1Kx
Fixes: 420c4495b5e56 ("mtd: spi-nor: spansion: make sure local struct does not contain garbage")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Mark Brown <broonie@kernel.org>
Acked-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Link: https://lore.kernel.org/linux-mtd/20230719190045.4007391-1-arnd@kernel.org

authored by

Arnd Bergmann and committed by
Miquel Raynal
71c8f9cf d3053b4a

+6 -2
+2 -2
drivers/mtd/spi-nor/spansion.c
··· 361 361 */ 362 362 static int cypress_nor_set_addr_mode_nbytes(struct spi_nor *nor) 363 363 { 364 - struct spi_mem_op op = {}; 364 + struct spi_mem_op op; 365 365 u8 addr_mode; 366 366 int ret; 367 367 ··· 492 492 const struct sfdp_parameter_header *bfpt_header, 493 493 const struct sfdp_bfpt *bfpt) 494 494 { 495 - struct spi_mem_op op = {}; 495 + struct spi_mem_op op; 496 496 int ret; 497 497 498 498 ret = cypress_nor_set_addr_mode_nbytes(nor);
+4
include/linux/spi/spi-mem.h
··· 101 101 u8 nbytes; 102 102 u8 buswidth; 103 103 u8 dtr : 1; 104 + u8 __pad : 7; 104 105 u16 opcode; 105 106 } cmd; 106 107 ··· 109 108 u8 nbytes; 110 109 u8 buswidth; 111 110 u8 dtr : 1; 111 + u8 __pad : 7; 112 112 u64 val; 113 113 } addr; 114 114 ··· 117 115 u8 nbytes; 118 116 u8 buswidth; 119 117 u8 dtr : 1; 118 + u8 __pad : 7; 120 119 } dummy; 121 120 122 121 struct { 123 122 u8 buswidth; 124 123 u8 dtr : 1; 125 124 u8 ecc : 1; 125 + u8 __pad : 6; 126 126 enum spi_mem_data_dir dir; 127 127 unsigned int nbytes; 128 128 union {