This is a set of patches that I hope to get into v4.9 in some form
in order to turn on the -Wmaybe-uninitialized warnings again.
After talking to Linus in person at Linaro Connect about this, I
spent some time on finding all the remaining warnings, and this
is the resulting patch series. More details are in the description
of the last patch that actually enables the warning.
Let me know if there are other warnings that I missed, and whether
you think these are still appropriate for v4.9 or not.
A couple of patches are non-obvious, and could use some more
detailed review.
Arnd
Arnd Bergmann (28):
[v2] netfilter: nf_tables: avoid uninitialized variable warning
[v2] mtd: mtk: avoid warning in mtk_ecc_encode
[v2] infiniband: shut up a maybe-uninitialized warning
f2fs: replace a build-time warning with runtime WARN_ON
ext2: avoid bogus -Wmaybe-uninitialized warning
NFSv4.1: work around -Wmaybe-uninitialized warning
ceph: avoid false positive maybe-uninitialized warning
staging: lustre: restore initialization of return code
staging: lustre: remove broken dead code in
cfs_cpt_table_create_pattern
UBI: fix uninitialized access of vid_hdr pointer
block: rdb: false-postive gcc-4.9 -Wmaybe-uninitialized
[media] rc: print correct variable for z8f0811
[media] dib0700: fix uninitialized data on 'repeat' event
iio: accel: sca3000_core: avoid potentially uninitialized variable
crypto: aesni: avoid -Wmaybe-uninitialized warning
pcmcia: fix return value of soc_pcmcia_regulator_set
spi: fsl-espi: avoid processing uninitalized data on error
drm: avoid uninitialized timestamp use in wait_vblank
brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap
net: bcm63xx: avoid referencing uninitialized variable
net/hyperv: avoid uninitialized variable
x86: apm: avoid uninitialized data
x86: mark target address as output in 'insb' asm
x86: math-emu: possible uninitialized variable use
s390: pci: don't print uninitialized data for debugging
nios2: fix timer initcall return value
rocker: fix maybe-uninitialized warning
Kbuild: bring back -Wmaybe-uninitialized warning
Makefile | 10 +-
arch/arc/Makefile | 4 +-
arch/nios2/kernel/time.c | 1 +
arch/s390/pci/pci_dma.c | 2 +-
arch/x86/crypto/aesni-intel_glue.c | 121 +++++++++++++--------
arch/x86/include/asm/io.h | 4 +-
arch/x86/kernel/apm_32.c | 5 +-
arch/x86/math-emu/Makefile | 4 +-
arch/x86/math-emu/reg_compare.c | 16 +--
drivers/block/rbd.c | 1 +
drivers/gpu/drm/drm_irq.c | 4 +-
drivers/infiniband/core/cma.c | 56 +++++-----
drivers/media/i2c/ir-kbd-i2c.c | 2 +-
drivers/media/usb/dvb-usb/dib0700_core.c | 10 +-
drivers/mtd/nand/mtk_ecc.c | 19 ++--
drivers/mtd/ubi/eba.c | 2 +-
drivers/net/ethernet/broadcom/bcm63xx_enet.c | 3 +-
drivers/net/ethernet/rocker/rocker_ofdpa.c | 4 +-
drivers/net/hyperv/netvsc_drv.c | 2 +-
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 2 +-
drivers/pcmcia/soc_common.c | 2 +-
drivers/spi/spi-fsl-espi.c | 2 +-
drivers/staging/iio/accel/sca3000_core.c | 2 +
.../staging/lustre/lnet/libcfs/linux/linux-cpu.c | 7 --
drivers/staging/lustre/lustre/lov/lov_pack.c | 2 +
fs/ceph/super.c | 3 +-
fs/ext2/inode.c | 7 +-
fs/f2fs/data.c | 7 ++
fs/nfs/nfs4session.c | 10 +-
net/netfilter/nft_range.c | 10 +-
scripts/Makefile.ubsan | 4 +
31 files changed, 187 insertions(+), 141 deletions(-)
--
Cc: x86@kernel.org
Cc: linux-media@vger.kernel.org
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Martin Schwidefsky <redacted>
Cc: linux-s390@vger.kernel.org
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-mtd@lists.infradead.org
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: linux-crypto@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: ceph-devel@vger.kernel.org
Cc: linux-f2fs-devel@lists.sourceforge.net
Cc: linux-ext4@vger.kernel.org
Cc: netfilter-devel@vger.kernel.org
2.9.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
The newly added nft_range_eval() function handles the two possible
nft range operations, but as the compiler warning points out,
any unexpected value would lead to the 'mismatch' variable being
used without being initialized:
net/netfilter/nft_range.c: In function 'nft_range_eval':
net/netfilter/nft_range.c:45:5: error: 'mismatch' may be used uninitialized in this function [-Werror=maybe-uninitialized]
This removes the variable in question and instead moves the
condition into the switch itself, which is potentially more
efficient than adding a bogus 'default' clause as in my
first approach, and is nicer than using the 'uninitialized_var'
macro.
Fixes: 0f3cd9b36977 ("netfilter: nf_tables: add range expression")
Link: http://patchwork.ozlabs.org/patch/677114/
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
net/netfilter/nft_range.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
A bugfix added a sanity check around the assignment and use of the
'is_11d' variable, which looks correct to me, but as the function is
rather complex already, this confuses the compiler to the point where
it can no longer figure out if the variable is always initialized
correctly:
brcm80211/brcmfmac/cfg80211.c: In function ‘brcmf_cfg80211_start_ap’:
brcm80211/brcmfmac/cfg80211.c:4586:10: error: ‘is_11d’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
This adds an initialization for the newly introduced case in which
the variable should not really be used, in order to make the warning
go away.
Fixes: b3589dfe0212 ("brcmfmac: ignore 11d configuration errors")
Cc: Hante Meuleman <redacted>
Cc: Arend van Spriel <arend.vanspriel@broadcom.com>
Cc: Kalle Valo <redacted>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
The hdr_offset variable is only if we deal with a TCP or UDP packet,
but as the check surrounding its usage tests for skb_is_gso()
instead, the compiler has no idea if the variable is initialized
or not at that point:
drivers/net/hyperv/netvsc_drv.c: In function ‘netvsc_start_xmit’:
drivers/net/hyperv/netvsc_drv.c:494:42: error: ‘hdr_offset’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
This adds an additional check for the transport type, which
tells the compiler that this path cannot happen. Since the
get_net_transport_info() function should always be inlined
here, I don't expect this to result in additional runtime
checks.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/hyperv/netvsc_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
gcc found a reference to an uninitialized variable in the error handling
of bcm_enet_open, introduced by a recent cleanup:
drivers/net/ethernet/broadcom/bcm63xx_enet.c: In function 'bcm_enet_open'
drivers/net/ethernet/broadcom/bcm63xx_enet.c:1129:2: warning: 'phydev' may be used uninitialized in this function [-Wmaybe-uninitialized]
This makes the use of that variable conditional, so we only reference it
here after it has been used before. Unlike my normal patches, I have not
build-tested this one, as I don't currently have mips test in my
randconfig setup.
Fixes: 625eb8667d6f ("net: ethernet: broadcom: bcm63xx: use phydev from struct net_device")
Cc: Philippe Reynes <redacted>
Reported-by: kbuild test robot <redacted>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ethernet/broadcom/bcm63xx_enet.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
In some rare configurations, we get a warning about the 'index' variable
being used without an initialization:
drivers/net/ethernet/rocker/rocker_ofdpa.c: In function ‘ofdpa_port_fib_ipv4.isra.16.constprop’:
drivers/net/ethernet/rocker/rocker_ofdpa.c:2425:92: warning: ‘index’ may be used uninitialized in this function [-Wmaybe-uninitialized]
This is a false positive, the logic is just a bit too complex for gcc
to follow here. Moving the intialization of 'index' a little further
down makes it clear to gcc that the function always returns an error
if it is not initialized.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ethernet/rocker/rocker_ofdpa.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Traditionally, we have always had warnings about uninitialized variables
enabled, as this is part of -Wall, and generally a good idea [1], but it
also always produced false positives, mainly because this is a variation
of the halting problem and provably impossible to get right in all cases
[2].
Various people have identified cases that are particularly bad for false
positives, and in commit e74fc973b6e5 ("Turn off -Wmaybe-uninitialized
when building with -Os"), I turned off the warning for any build that
was done with CC_OPTIMIZE_FOR_SIZE. This drastically reduced the number
of false positive warnings in the default build but unfortunately had
the side effect of turning the warning off completely in 'allmodconfig'
builds, which in turn led to a lot of warnings (both actual bugs, and
remaining false positives) to go in unnoticed.
With commit 877417e6ffb9 ("Kbuild: change CC_OPTIMIZE_FOR_SIZE
definition") enabled the warning again for allmodconfig builds in v4.7
and in v4.8-rc1, I had finally managed to address all warnings I get in
an ARM allmodconfig build and most other maybe-uninitialized warnings
for ARM randconfig builds.
However, commit 6e8d666e9253 ("Disable "maybe-uninitialized" warning
globally") was merged at the same time and disabled it completely for
all configurations, because of false-positive warnings on x86 that
I had not addressed until then. This caused a lot of actual bugs to
get merged into mainline, and I sent several dozen patches for these
during the v4.9 development cycle. Most of these are actual bugs,
some are for correct code that is safe because it is only called
under external constraints that make it impossible to run into
the case that gcc sees, and in a few cases gcc is just stupid and
finds something that can obviously never happen.
I have now done a few thousand randconfig builds on x86 and collected
all patches that I needed to address every single warning I got
(I can provide the combined patch for the other warnings if anyone
is interested), so I hope we can get the warning back and let people
catch the actual bugs earlier.
Note that the majority of the patches I created are for the third kind
of problem (stupid false-positives), for one of two reasons:
- some of them only get triggered in certain combinations of config
options, so we don't always run into them, and
- the actual bugs tend to get addressed much quicker as they also
lead to incorrect runtime behavior.
These 27 patches address the warnings that either occur in one of the more
common configurations (defconfig, allmodconfig, or something built by the
kbuild robot or kernelci.org), or they are about a real bug. It would be
good to get these all into v4.9 if we want to turn on the warning again.
I have tested these extensively with gcc-4.9 and gcc-6 and done a bit
of testing with gcc-5, and all of these should now be fine. gcc-4.8
is much worse about the false-positive warnings and is also fairly old
now, so I'm leaving the warning disabled with that version. gcc-4.7 and
older don't understand the -Wno-maybe-uninitialized option and are not
affected by this patch either way.
I have another (smaller) series of patches for warnings that are both
harmless and not as easy to trigger, and I will send them for inclusion
in v4.10.
Link: https://rusty.ozlabs.org/?p=232 [1]
Link: https://gcc.gnu.org/wiki/Better_Uninitialized_Warnings [2]
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Makefile | 10 ++++++----
arch/arc/Makefile | 4 +++-
scripts/Makefile.ubsan | 4 ++++
3 files changed, 13 insertions(+), 5 deletions(-)
Cc: x86@kernel.org
Cc: linux-media@vger.kernel.org
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Martin Schwidefsky <redacted>
Cc: linux-s390@vger.kernel.org
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-mtd@lists.infradead.org
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: linux-crypto@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: ceph-devel@vger.kernel.org
Cc: linux-f2fs-devel@lists.sourceforge.net
Cc: linux-ext4@vger.kernel.org
Cc: netfilter-devel@vger.kernel.org
@@ -71,7 +71,9 @@ cflags-$(CONFIG_ARC_DW2_UNWIND) += -fasynchronous-unwind-tables $(cfi)ifndef CONFIG_CC_OPTIMIZE_FOR_SIZE# Generic build system uses -O2, we want -O3# Note: No need to add to cflags-y as that happens anyways-ARCH_CFLAGS+=-O3+#+# Disable the false maybe-uninitialized warings gcc spits out at -O3+ARCH_CFLAGS+=-O3$(callcc-disable-warning,maybe-uninitialized,)endif# small data is default for elf32 tool-chain. If not usable, disable it
@@ -17,4 +17,8 @@ endififdef CONFIG_UBSAN_NULLCFLAGS_UBSAN+=$(callcc-option,-fsanitize=null)endif++ # -fsanitize=* options makes GCC less smart than usual and+ # increase number of 'maybe-uninitialized false-positives+CFLAGS_UBSAN+=$(callcc-option,-Wno-maybe-uninitialized)endif
From: Christoph Hellwig <hch@infradead.org> Date: 2016-10-18 05:08:16
On Tue, Oct 18, 2016 at 12:03:28AM +0200, Arnd Bergmann wrote:
This is a set of patches that I hope to get into v4.9 in some form
in order to turn on the -Wmaybe-uninitialized warnings again.
Hi Arnd,
I jsut complained to Geert that I was introducing way to many
bugs or pointless warnings for some compilers lately, but gcc didn't
warn me about them. From a little research the lack of
-Wmaybe-uninitialized seems to be the reason for it, so I'm all
for re-enabling it.
From: Pablo Neira Ayuso <pablo@netfilter.org> Date: 2016-10-18 15:24:16
On Tue, Oct 18, 2016 at 12:05:30AM +0200, Arnd Bergmann wrote:
The newly added nft_range_eval() function handles the two possible
nft range operations, but as the compiler warning points out,
any unexpected value would lead to the 'mismatch' variable being
used without being initialized:
net/netfilter/nft_range.c: In function 'nft_range_eval':
net/netfilter/nft_range.c:45:5: error: 'mismatch' may be used uninitialized in this function [-Werror=maybe-uninitialized]
This removes the variable in question and instead moves the
condition into the switch itself, which is potentially more
efficient than adding a bogus 'default' clause as in my
first approach, and is nicer than using the 'uninitialized_var'
macro.
gcc found a reference to an uninitialized variable in the error handling
of bcm_enet_open, introduced by a recent cleanup:
drivers/net/ethernet/broadcom/bcm63xx_enet.c: In function 'bcm_enet_open'
drivers/net/ethernet/broadcom/bcm63xx_enet.c:1129:2: warning: 'phydev' may be used uninitialized in this function [-Wmaybe-uninitialized]
This makes the use of that variable conditional, so we only reference it
here after it has been used before. Unlike my normal patches, I have not
build-tested this one, as I don't currently have mips test in my
randconfig setup.
Fixes: 625eb8667d6f ("net: ethernet: broadcom: bcm63xx: use phydev from struct net_device")
Cc: Philippe Reynes <redacted>
Reported-by: kbuild test robot <redacted>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
The hdr_offset variable is only if we deal with a TCP or UDP packet,
but as the check surrounding its usage tests for skb_is_gso()
instead, the compiler has no idea if the variable is initialized
or not at that point:
drivers/net/hyperv/netvsc_drv.c: In function ‘netvsc_start_xmit’:
drivers/net/hyperv/netvsc_drv.c:494:42: error: ‘hdr_offset’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
This adds an additional check for the transport type, which
tells the compiler that this path cannot happen. Since the
get_net_transport_info() function should always be inlined
here, I don't expect this to result in additional runtime
checks.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
In some rare configurations, we get a warning about the 'index' variable
being used without an initialization:
drivers/net/ethernet/rocker/rocker_ofdpa.c: In function ‘ofdpa_port_fib_ipv4.isra.16.constprop’:
drivers/net/ethernet/rocker/rocker_ofdpa.c:2425:92: warning: ‘index’ may be used uninitialized in this function [-Wmaybe-uninitialized]
This is a false positive, the logic is just a bit too complex for gcc
to follow here. Moving the intialization of 'index' a little further
down makes it clear to gcc that the function always returns an error
if it is not initialized.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
From: Kalle Valo <hidden> Date: 2016-10-27 15:07:03
Arnd Bergmann [off-list ref] wrote:
A bugfix added a sanity check around the assignment and use of the
'is_11d' variable, which looks correct to me, but as the function is
rather complex already, this confuses the compiler to the point where
it can no longer figure out if the variable is always initialized
correctly:
brcm80211/brcmfmac/cfg80211.c: In function ‘brcmf_cfg80211_start_ap’:
brcm80211/brcmfmac/cfg80211.c:4586:10: error: ‘is_11d’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
This adds an initialization for the newly introduced case in which
the variable should not really be used, in order to make the warning
go away.
Fixes: b3589dfe0212 ("brcmfmac: ignore 11d configuration errors")
Cc: Hante Meuleman <hante.meuleman-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Cc: Arend van Spriel <arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Cc: Kalle Valo <redacted>
Signed-off-by: Arnd Bergmann <redacted>