Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 20 Jul 2023 10:48:32 +0200
From:   Miquel Raynal <miquel.raynal@bootlin.com>
To:     Tudor Ambarus <tudor.ambarus@linaro.org>
Cc:     Arnd Bergmann <arnd@kernel.org>,
        Pratyush Yadav <pratyush@kernel.org>,
        Richard Weinberger <richard@nod.at>,
        Vignesh Raghavendra <vigneshr@ti.com>,
        Arnd Bergmann <arnd@arndb.de>,
        Peter Foley <pefoley2@pefoley.com>,
        Pedro Falcato <pedro.falcato@gmail.com>,
        Michael Walle <michael@walle.cc>,
        Mark Brown <broonie@kernel.org>,
        Takahiro Kuwano <Takahiro.Kuwano@infineon.com>,
        Dhruva Gole <d-gole@ti.com>, linux-mtd@lists.infradead.org,
        linux-kernel@vger.kernel.org, linux-spi@vger.kernel.org
Subject: Re: [PATCH] mtd: spi-nor: avoid holes in struct spi_mem_op
Message-ID: <20230720104832.15a4d4ab@xps-13>
In-Reply-To: <598bd9d8-249e-125c-bde3-7a63ba6dc5f7@linaro.org>
References: <20230719190045.4007391-1-arnd@kernel.org>
        <598bd9d8-249e-125c-bde3-7a63ba6dc5f7@linaro.org>
Organization: Bootlin
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Tudor,

tudor.ambarus@linaro.org wrote on Thu, 20 Jul 2023 07:50:33 +0100:

> On 7/19/23 20:00, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> >=20
> > gcc gets confused when -ftrivial-auto-var-init=3Dpattern is used on spa=
rse
> > bit fields such as 'struct spi_mem_op', which caused the previous false
> > positive warning about an uninitialized variable:
> >=20
> > drivers/mtd/spi-nor/spansion.c: error: 'op' is used uninitialized [-Wer=
ror=3Duninitialized]
> >=20
> > 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.
> >=20
> > A previous workaround added another initialization, which ended up
> > shutting up the warning in spansion.c, though it apparently still happe=
ns
> > 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.
> >=20
> > 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.
> >=20
> > 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.
> >=20
> > Cc: Peter Foley <pefoley2@pefoley.com>
> > Cc: Pedro Falcato <pedro.falcato@gmail.com>
> > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D110743
> > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D108402
> > Link: https://godbolt.org/z/efMMsG1Kx
> > Fixes: 420c4495b5e56 ("mtd: spi-nor: spansion: make sure local struct d=
oes not contain garbage")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de> =20
>=20
> Acked-by: Tudor Ambarus <tudor.ambarus@linaro.org>
>=20
> Miquel, would you please take this through mtd/fixes?

I will!

Thanks,
Miqu=C3=A8l