* [PATCH 01/28] [v2] netfilter: nf_tables: avoid uninitialized variable warning
2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
@ 2016-10-17 22:05 ` Arnd Bergmann
2016-10-18 15:23 ` Pablo Neira Ayuso
2016-10-17 22:13 ` [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap Arnd Bergmann
` (5 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:05 UTC (permalink / raw)
To: Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik,
David S. Miller
Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, netfilter-devel,
coreteam, netdev
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>
diff --git a/net/netfilter/nft_range.c b/net/netfilter/nft_range.c
index c6d5358..2dd80f4 100644
--- a/net/netfilter/nft_range.c
+++ b/net/netfilter/nft_range.c
@@ -28,22 +28,20 @@ static void nft_range_eval(const struct nft_expr *expr,
const struct nft_pktinfo *pkt)
{
const struct nft_range_expr *priv = nft_expr_priv(expr);
- bool mismatch;
int d1, d2;
d1 = memcmp(®s->data[priv->sreg], &priv->data_from, priv->len);
d2 = memcmp(®s->data[priv->sreg], &priv->data_to, priv->len);
switch (priv->op) {
case NFT_RANGE_EQ:
- mismatch = (d1 < 0 || d2 > 0);
+ if (d1 < 0 || d2 > 0)
+ regs->verdict.code = NFT_BREAK;
break;
case NFT_RANGE_NEQ:
- mismatch = (d1 >= 0 && d2 <= 0);
+ if (d1 >= 0 && d2 <= 0)
+ regs->verdict.code = NFT_BREAK;
break;
}
-
- if (mismatch)
- regs->verdict.code = NFT_BREAK;
}
static const struct nla_policy nft_range_policy[NFTA_RANGE_MAX + 1] = {
--
2.9.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 01/28] [v2] netfilter: nf_tables: avoid uninitialized variable warning
2016-10-17 22:05 ` [PATCH 01/28] [v2] netfilter: nf_tables: avoid uninitialized variable warning Arnd Bergmann
@ 2016-10-18 15:23 ` Pablo Neira Ayuso
0 siblings, 0 replies; 16+ messages in thread
From: Pablo Neira Ayuso @ 2016-10-18 15:23 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Patrick McHardy, Jozsef Kadlecsik, David S. Miller,
Linus Torvalds, linux-kernel, netfilter-devel, coreteam, netdev
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.
Applied to the nf tree, thanks Arnd.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap
2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
2016-10-17 22:05 ` [PATCH 01/28] [v2] netfilter: nf_tables: avoid uninitialized variable warning Arnd Bergmann
@ 2016-10-17 22:13 ` Arnd Bergmann
2016-10-26 6:49 ` Kalle Valo
[not found] ` <20161017221355.1861551-7-arnd-r2nGTMty4D4@public.gmane.org>
2016-10-17 22:16 ` [PATCH 20/28] net: bcm63xx: avoid referencing uninitialized variable Arnd Bergmann
` (4 subsequent siblings)
6 siblings, 2 replies; 16+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:13 UTC (permalink / raw)
To: Arend van Spriel
Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, Hante Meuleman,
Kalle Valo, Franky Lin, Pieter-Paul Giesberts,
Franky (Zhenhui) Lin, Rafał Miłecki, linux-wireless,
brcm80211-dev-list.pdl, netdev
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@broadcom.com>
Cc: Arend van Spriel <arend.vanspriel@broadcom.com>
Cc: Kalle Valo <kvalo@codeaurora.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index b777e1b..78d9966 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -4516,7 +4516,7 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev,
/* store current 11d setting */
if (brcmf_fil_cmd_int_get(ifp, BRCMF_C_GET_REGULATORY,
&ifp->vif->is_11d)) {
- supports_11d = false;
+ is_11d = supports_11d = false;
} else {
country_ie = brcmf_parse_tlvs((u8 *)settings->beacon.tail,
settings->beacon.tail_len,
--
2.9.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap
2016-10-17 22:13 ` [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap Arnd Bergmann
@ 2016-10-26 6:49 ` Kalle Valo
2016-10-26 9:57 ` Arnd Bergmann
[not found] ` <20161017221355.1861551-7-arnd-r2nGTMty4D4@public.gmane.org>
1 sibling, 1 reply; 16+ messages in thread
From: Kalle Valo @ 2016-10-26 6:49 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Arend van Spriel, Linus Torvalds, linux-kernel, Hante Meuleman,
Franky Lin, Pieter-Paul Giesberts, Franky (Zhenhui) Lin,
Rafał Miłecki, linux-wireless, brcm80211-dev-list.pdl,
netdev
Arnd Bergmann <arnd@arndb.de> writes:
> 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@broadcom.com>
> Cc: Arend van Spriel <arend.vanspriel@broadcom.com>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Via which tree are you planning to submit this? Should I take it?
--
Kalle Valo
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap
2016-10-26 6:49 ` Kalle Valo
@ 2016-10-26 9:57 ` Arnd Bergmann
2016-10-26 11:11 ` Kalle Valo
0 siblings, 1 reply; 16+ messages in thread
From: Arnd Bergmann @ 2016-10-26 9:57 UTC (permalink / raw)
To: Kalle Valo
Cc: Arend van Spriel, Linus Torvalds, linux-kernel, Hante Meuleman,
Franky Lin, Pieter-Paul Giesberts, Franky (Zhenhui) Lin,
Rafał Miłecki, linux-wireless, brcm80211-dev-list.pdl,
netdev
On Wednesday, October 26, 2016 9:49:58 AM CEST Kalle Valo wrote:
> Arnd Bergmann <arnd@arndb.de> writes:
>
> > 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@broadcom.com>
> > Cc: Arend van Spriel <arend.vanspriel@broadcom.com>
> > Cc: Kalle Valo <kvalo@codeaurora.org>
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> Via which tree are you planning to submit this? Should I take it?
I'd prefer if you can take it and forward it along with your other
bugfixes. I'll try to take care of the ones that nobody else
picked up.
Arnd
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap
2016-10-26 9:57 ` Arnd Bergmann
@ 2016-10-26 11:11 ` Kalle Valo
0 siblings, 0 replies; 16+ messages in thread
From: Kalle Valo @ 2016-10-26 11:11 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Arend van Spriel, Linus Torvalds,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Hante Meuleman, Franky Lin,
Pieter-Paul Giesberts, Franky (Zhenhui) Lin,
Rafał Miłecki, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
brcm80211-dev-list.pdl-dY08KVG/lbpWk0Htik3J/w,
netdev-u79uwXL29TY76Z2rM5mHXA
Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> writes:
> On Wednesday, October 26, 2016 9:49:58 AM CEST Kalle Valo wrote:
>> Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> writes:
>>
>> > 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 <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> > Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
>>
>> Via which tree are you planning to submit this? Should I take it?
>
> I'd prefer if you can take it and forward it along with your other
> bugfixes. I'll try to take care of the ones that nobody else
> picked up.
Ok, I'll take it. I'm planning to push this to 4.9.
--
Kalle Valo
^ permalink raw reply [flat|nested] 16+ messages in thread
[parent not found: <20161017221355.1861551-7-arnd-r2nGTMty4D4@public.gmane.org>]
* Re: [19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap
[not found] ` <20161017221355.1861551-7-arnd-r2nGTMty4D4@public.gmane.org>
@ 2016-10-27 15:05 ` Kalle Valo
0 siblings, 0 replies; 16+ messages in thread
From: Kalle Valo @ 2016-10-27 15:05 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Arend van Spriel, Linus Torvalds,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Arnd Bergmann,
Hante Meuleman, Franky Lin, Pieter-Paul Giesberts,
Franky (Zhenhui) Lin, Rafał Miłecki,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
brcm80211-dev-list.pdl-dY08KVG/lbpWk0Htik3J/w,
netdev-u79uwXL29TY76Z2rM5mHXA
Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> 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 <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
Patch applied to wireless-drivers.git, thanks.
d3532ea6ce4e brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap
--
https://patchwork.kernel.org/patch/9380763/
Documentation about submitting wireless patches and checking status
from patchwork:
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 20/28] net: bcm63xx: avoid referencing uninitialized variable
2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
2016-10-17 22:05 ` [PATCH 01/28] [v2] netfilter: nf_tables: avoid uninitialized variable warning Arnd Bergmann
2016-10-17 22:13 ` [PATCH 19/28] brcmfmac: avoid maybe-uninitialized warning in brcmf_cfg80211_start_ap Arnd Bergmann
@ 2016-10-17 22:16 ` Arnd Bergmann
2016-10-18 18:21 ` David Miller
2016-10-17 22:16 ` [PATCH 21/28] net/hyperv: avoid " Arnd Bergmann
` (3 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:16 UTC (permalink / raw)
To: David S. Miller
Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, Philippe Reynes,
Florian Fainelli, bcm-kernel-feedback-list, Andrew Lunn, netdev,
linux-arm-kernel
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 <tremyfr@gmail.com>
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/ethernet/broadcom/bcm63xx_enet.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bcm63xx_enet.c b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
index ae364c7..5370909 100644
--- a/drivers/net/ethernet/broadcom/bcm63xx_enet.c
+++ b/drivers/net/ethernet/broadcom/bcm63xx_enet.c
@@ -1126,7 +1126,8 @@ static int bcm_enet_open(struct net_device *dev)
free_irq(dev->irq, dev);
out_phy_disconnect:
- phy_disconnect(phydev);
+ if (priv->has_phy)
+ phy_disconnect(phydev);
return ret;
}
--
2.9.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 20/28] net: bcm63xx: avoid referencing uninitialized variable
2016-10-17 22:16 ` [PATCH 20/28] net: bcm63xx: avoid referencing uninitialized variable Arnd Bergmann
@ 2016-10-18 18:21 ` David Miller
0 siblings, 0 replies; 16+ messages in thread
From: David Miller @ 2016-10-18 18:21 UTC (permalink / raw)
To: arnd
Cc: torvalds, linux-kernel, tremyfr, f.fainelli,
bcm-kernel-feedback-list, andrew, netdev, linux-arm-kernel
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 18 Oct 2016 00:16:08 +0200
> 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 <tremyfr@gmail.com>
> Reported-by: kbuild test robot <fengguang.wu@intel.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Applied.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 21/28] net/hyperv: avoid uninitialized variable
2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
` (2 preceding siblings ...)
2016-10-17 22:16 ` [PATCH 20/28] net: bcm63xx: avoid referencing uninitialized variable Arnd Bergmann
@ 2016-10-17 22:16 ` Arnd Bergmann
2016-10-18 18:21 ` David Miller
2016-10-17 22:16 ` [PATCH 27/28] rocker: fix maybe-uninitialized warning Arnd Bergmann
` (2 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:16 UTC (permalink / raw)
To: K. Y. Srinivasan, Haiyang Zhang
Cc: Arnd Bergmann, netdev, linux-kernel, devel, Linus Torvalds,
David S. Miller
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(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index f0919bd..5d6e75a 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -447,7 +447,7 @@ static int netvsc_start_xmit(struct sk_buff *skb, struct net_device *net)
* Setup the sendside checksum offload only if this is not a
* GSO packet.
*/
- if (skb_is_gso(skb)) {
+ if ((net_trans_info & (INFO_TCP | INFO_UDP)) && skb_is_gso(skb)) {
struct ndis_tcp_lso_info *lso_info;
rndis_msg_size += NDIS_LSO_PPI_SIZE;
--
2.9.0
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 21/28] net/hyperv: avoid uninitialized variable
2016-10-17 22:16 ` [PATCH 21/28] net/hyperv: avoid " Arnd Bergmann
@ 2016-10-18 18:21 ` David Miller
0 siblings, 0 replies; 16+ messages in thread
From: David Miller @ 2016-10-18 18:21 UTC (permalink / raw)
To: arnd
Cc: kys, haiyangz, torvalds, linux-kernel, vkuznets, stephen, sixiao,
devel, netdev
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 18 Oct 2016 00:16:09 +0200
> 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>
Applied.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 27/28] rocker: fix maybe-uninitialized warning
2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
` (3 preceding siblings ...)
2016-10-17 22:16 ` [PATCH 21/28] net/hyperv: avoid " Arnd Bergmann
@ 2016-10-17 22:16 ` Arnd Bergmann
2016-10-18 18:21 ` David Miller
2016-10-17 22:19 ` [PATCH 28/28] Kbuild: bring back -Wmaybe-uninitialized warning Arnd Bergmann
2016-10-18 5:08 ` [PATCH 00/28] Reenable maybe-uninitialized warnings Christoph Hellwig
6 siblings, 1 reply; 16+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:16 UTC (permalink / raw)
To: Jiri Pirko
Cc: Linus Torvalds, linux-kernel, Arnd Bergmann, David S. Miller,
Ido Schimmel, Dan Carpenter, netdev
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(-)
diff --git a/drivers/net/ethernet/rocker/rocker_ofdpa.c b/drivers/net/ethernet/rocker/rocker_ofdpa.c
index 431a608..4ca4613 100644
--- a/drivers/net/ethernet/rocker/rocker_ofdpa.c
+++ b/drivers/net/ethernet/rocker/rocker_ofdpa.c
@@ -1493,8 +1493,6 @@ static int ofdpa_port_ipv4_nh(struct ofdpa_port *ofdpa_port,
spin_lock_irqsave(&ofdpa->neigh_tbl_lock, lock_flags);
found = ofdpa_neigh_tbl_find(ofdpa, ip_addr);
- if (found)
- *index = found->index;
updating = found && adding;
removing = found && !adding;
@@ -1508,9 +1506,11 @@ static int ofdpa_port_ipv4_nh(struct ofdpa_port *ofdpa_port,
resolved = false;
} else if (removing) {
ofdpa_neigh_del(trans, found);
+ *index = found->index;
} else if (updating) {
ofdpa_neigh_update(found, trans, NULL, false);
resolved = !is_zero_ether_addr(found->eth_dst);
+ *index = found->index;
} else {
err = -ENOENT;
}
--
2.9.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 27/28] rocker: fix maybe-uninitialized warning
2016-10-17 22:16 ` [PATCH 27/28] rocker: fix maybe-uninitialized warning Arnd Bergmann
@ 2016-10-18 18:21 ` David Miller
0 siblings, 0 replies; 16+ messages in thread
From: David Miller @ 2016-10-18 18:21 UTC (permalink / raw)
To: arnd; +Cc: jiri, torvalds, linux-kernel, idosch, dan.carpenter, netdev
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 18 Oct 2016 00:16:15 +0200
> 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>
Applied.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 28/28] Kbuild: bring back -Wmaybe-uninitialized warning
2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
` (4 preceding siblings ...)
2016-10-17 22:16 ` [PATCH 27/28] rocker: fix maybe-uninitialized warning Arnd Bergmann
@ 2016-10-17 22:19 ` Arnd Bergmann
2016-10-18 5:08 ` [PATCH 00/28] Reenable maybe-uninitialized warnings Christoph Hellwig
6 siblings, 0 replies; 16+ messages in thread
From: Arnd Bergmann @ 2016-10-17 22:19 UTC (permalink / raw)
To: Linus Torvalds, Michal Marek
Cc: Nicolas Pitre, Greg Kroah-Hartman, Heiko Carstens, dri-devel,
linux-mtd, Ingo Molnar, linux-s390, Herbert Xu, x86,
Christian Borntraeger, Ilya Dryomov, linux-ext4, linux-media,
Kees Cook, Arnd Bergmann, linux-kbuild, Josh Poimboeuf,
ceph-devel, Mauro Carvalho Chehab, linux-snps-arc, netdev,
linux-kernel, linux-f2fs-devel, netfilter-devel, linux-crypto,
Vineet Gupta
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 <schwidefsky@de.ibm.com>
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
diff --git a/Makefile b/Makefile
index 512e47a..43cd3d9 100644
--- a/Makefile
+++ b/Makefile
@@ -370,7 +370,7 @@ LDFLAGS_MODULE =
CFLAGS_KERNEL =
AFLAGS_KERNEL =
LDFLAGS_vmlinux =
-CFLAGS_GCOV = -fprofile-arcs -ftest-coverage -fno-tree-loop-im
+CFLAGS_GCOV = -fprofile-arcs -ftest-coverage -fno-tree-loop-im -Wno-maybe-uninitialized
CFLAGS_KCOV := $(call cc-option,-fsanitize-coverage=trace-pc,)
@@ -620,7 +620,6 @@ ARCH_CFLAGS :=
include arch/$(SRCARCH)/Makefile
KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks,)
-KBUILD_CFLAGS += $(call cc-disable-warning,maybe-uninitialized,)
KBUILD_CFLAGS += $(call cc-disable-warning,frame-address,)
ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
@@ -629,15 +628,18 @@ KBUILD_CFLAGS += $(call cc-option,-fdata-sections,)
endif
ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
-KBUILD_CFLAGS += -Os
+KBUILD_CFLAGS += -Os $(call cc-disable-warning,maybe-uninitialized,)
else
ifdef CONFIG_PROFILE_ALL_BRANCHES
-KBUILD_CFLAGS += -O2
+KBUILD_CFLAGS += -O2 $(call cc-disable-warning,maybe-uninitialized,)
else
KBUILD_CFLAGS += -O2
endif
endif
+KBUILD_CFLAGS += $(call cc-ifversion, -lt, 0409, \
+ $(call cc-disable-warning,maybe-uninitialized,))
+
# Tell gcc to never replace conditional load with a non-conditional one
KBUILD_CFLAGS += $(call cc-option,--param=allow-store-data-races=0)
diff --git a/arch/arc/Makefile b/arch/arc/Makefile
index aa82d13..19cce22 100644
--- a/arch/arc/Makefile
+++ b/arch/arc/Makefile
@@ -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 $(call cc-disable-warning,maybe-uninitialized,)
endif
# small data is default for elf32 tool-chain. If not usable, disable it
diff --git a/scripts/Makefile.ubsan b/scripts/Makefile.ubsan
index dd779c4..3b1b138 100644
--- a/scripts/Makefile.ubsan
+++ b/scripts/Makefile.ubsan
@@ -17,4 +17,8 @@ endif
ifdef CONFIG_UBSAN_NULL
CFLAGS_UBSAN += $(call cc-option, -fsanitize=null)
endif
+
+ # -fsanitize=* options makes GCC less smart than usual and
+ # increase number of 'maybe-uninitialized false-positives
+ CFLAGS_UBSAN += $(call cc-option, -Wno-maybe-uninitialized)
endif
--
2.9.0
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 00/28] Reenable maybe-uninitialized warnings
2016-10-17 22:03 [PATCH 00/28] Reenable maybe-uninitialized warnings Arnd Bergmann
` (5 preceding siblings ...)
2016-10-17 22:19 ` [PATCH 28/28] Kbuild: bring back -Wmaybe-uninitialized warning Arnd Bergmann
@ 2016-10-18 5:08 ` Christoph Hellwig
6 siblings, 0 replies; 16+ messages in thread
From: Christoph Hellwig @ 2016-10-18 5:08 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Linus Torvalds, linux-kernel, x86, linux-media,
Mauro Carvalho Chehab, Martin Schwidefsky, linux-s390,
Ilya Dryomov, dri-devel, linux-mtd, Herbert Xu, linux-crypto,
David S. Miller, netdev, Greg Kroah-Hartman, ceph-devel,
linux-f2fs-devel, linux-ext4, netfilter-devel
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.
^ permalink raw reply [flat|nested] 16+ messages in thread