Re: linux-next: build warning after merge of the net tree

[David Miller]

From: Stephen Rothwell <sfr@xxxxxxxxxxxxxxxx>
Date: Thu, 5 Dec 2019 08:44:40 +1100

> After merging the net tree, today's linux-next build (x86_64 allmodconfig)
> produced this warning:
> 
> drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c: In function 'mlx5e_tc_tun_create_header_ipv6':
> drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c:332:20: warning: 'n' may be used uninitialized in this function [-Wmaybe-uninitialized]
>   332 |  struct neighbour *n;
>       |                    ^
> 
> Introduced by commit
> 
>   6c8991f41546 ("net: ipv6_stub: use ip6_dst_lookup_flow instead of ip6_dst_lookup")
> 
> It looks like a false positive.

I think it is a false positive as well.  It looks like the compiler
has trouble seeing through ptr error guards.  Actually, the top level
logic looks generically like this:


Old code:

int foo(int *byref)
{
	int err, val;

	err = something(&val);
	if (err < 0)
		return err;

	*byref = val;
	return 0;
}

...
	struct whatever *obj;
	
	err = foo(&obj);
	if (err < 0)
		return err;
	a = obj->something;

New code:

int foo(int *byref)
{
	struct otherthing *x;

	x = something(&val);
	if (IS_ERR(x))
		return PTR_ERR(x);
	*byref = bar(x);
	return 0;
}

...
	struct whatever *obj;

	err = foo(&obj);
	if (err < 0)
		return err;
	a = obj->somethng;

In the new code the compiler can only see that the return value in the
error case is non-zero, not necessarily that it is < 0 which is the
guard against uninitialized accesses of 'obj'.

It will satisfy this property, but through the various casts and
implicit demotions, this information is lost on the compiler.

My compiler didn't emit this warning FWIW.

gcc (GCC) 8.3.1 20190223 (Red Hat 8.3.1-2)

I'm unsure how to handle this, setting 'n' to explicitly be NULL
is bogus because the compiler now will think that a NULL deref
happens since the guard isn't guarding the existing assignment.