Adding a lock around one of the assignments prevents gcc from
tracking the state of the local 'fibmatch' variable, so it can no
longer prove that 'dst' is always initialized, leading to a bogus
warning:
net/ipv6/route.c: In function 'inet6_rtm_getroute':
net/ipv6/route.c:3659:2: error: 'dst' may be used uninitialized in this function [-Werror=maybe-uninitialized]
This moves the other assignment into the same lock to shut up the
warning.
Fixes: 121622dba8da ("ipv6: route: make rtm_getroute not assume rtnl is locked")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
net/ipv6/route.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
This kind of warning involving an unlock between variable initialization
and use is relatively frequent for false-positives. I should try to
seek clarification from the gcc developers on whether this can be
improved.
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index dc021ed6dd37..bec12ae3e6b7 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3624,6 +3624,8 @@ static int inet6_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
if (!fibmatch)
dst = ip6_route_input_lookup(net, dev, &fl6, flags);
+ else
+ dst = ip6_route_lookup(net, &fl6, 0);
rcu_read_unlock();
} else {@@ -3631,10 +3633,10 @@ static int inet6_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
if (!fibmatch)
dst = ip6_route_output(net, NULL, &fl6);
+ else
+ dst = ip6_route_lookup(net, &fl6, 0);
}
- if (fibmatch)
- dst = ip6_route_lookup(net, &fl6, 0);
rt = container_of(dst, struct rt6_info, dst);
if (rt->dst.error) {--
2.9.0
From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 18 Aug 2017 13:34:22 +0200
Adding a lock around one of the assignments prevents gcc from
tracking the state of the local 'fibmatch' variable, so it can no
longer prove that 'dst' is always initialized, leading to a bogus
warning:
net/ipv6/route.c: In function 'inet6_rtm_getroute':
net/ipv6/route.c:3659:2: error: 'dst' may be used uninitialized in this function [-Werror=maybe-uninitialized]
This moves the other assignment into the same lock to shut up the
warning.
Fixes: 121622dba8da ("ipv6: route: make rtm_getroute not assume rtnl is locked")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
net/ipv6/route.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
This kind of warning involving an unlock between variable initialization
and use is relatively frequent for false-positives. I should try to
seek clarification from the gcc developers on whether this can be
improved.
This will have to do for now I suppose.
I guess the issue is that if the local variable ever sits on the stack
then the memory barriers in the locks block the full dataflow
analysis.
But this makes no sense from a dataflow perspective. Even if the
local variable has a stack slot, there is no "escapability" of that
memory addres to foreign modifications.
If I had a nickel for every uninitialized variable warning we had to
work around....
On Fri, Aug 18, 2017 at 7:49 PM, David Miller [off-list ref] wrote:
From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 18 Aug 2017 13:34:22 +0200
quoted
This kind of warning involving an unlock between variable initialization
and use is relatively frequent for false-positives. I should try to
seek clarification from the gcc developers on whether this can be
improved.
This will have to do for now I suppose.
I guess the issue is that if the local variable ever sits on the stack
then the memory barriers in the locks block the full dataflow
analysis.
But this makes no sense from a dataflow perspective. Even if the
local variable has a stack slot, there is no "escapability" of that
memory addres to foreign modifications.
If I had a nickel for every uninitialized variable warning we had to
work around....
Since this pattern has come up so often, I spent most of my working
day today on a reduced testcase, and ended up with this surprising
snippet:
int f(void);
static inline void rcu_read_unlock(void)
{
static _Bool __warned;
if (f() && !__warned && !f()) {
__warned = 1;
}
}
int inet6_rtm_getroute(void)
{
int dst;
int fibmatch = f();
if (!fibmatch)
dst = f();
rcu_read_unlock();
if (fibmatch)
dst = 0;
return dst;
}
So at least in this particular case, the culprit is not actually
a memory barrier, but RCU_LOCKDEP_WARN(). A related
problem is __branch_check__()/__trace_if().
While the maybe-uninitialized warnings are unreliable by
definition, I think that case really should be understood by gcc.
I looked through the gcc bug database which has countless
entries but doesn't seem to have this one yet, so I opened
a new bug:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81897
Unfortunately the basic behavior shows up in gcc-4.7 already,
so it has no chance of getting fixed on older compilers.
Arnd