Re: [PATCH] x86: suppress sparse warning in copy_to_user()

[Johannes Berg <johannes@sipsolutions.net>]

On Tue, 2016-10-04 at 01:51 -0600, Jan Beulich wrote:
> > 
> > > 
> > > > 
> > > > On 04.10.16 at 09:33, <johannes@sipsolutions.net> wrote:
> > From: Johannes Berg <johannes.berg@intel.com>
> > 
> > __compiletime_object_size() is simply defined to
> > __builtin_object_size()
> > which gcc declares with (void *, int type) prototype.
> 
> If that was the case, everyone should have seen such warnings from
> the day the original patch got introduced. 

Only if they run sparse. Clearly people don't, or we wouldn't have a
history of a ton of such problems, e.g.

112dc0c8069e ("locking/barriers: Suppress sparse warnings in lockless_dereference()")
c15c0ab12fd6 ("ipv6: suppress sparse warnings in IP6_ECN_set_ce()")
1ea049b2de5d ("bvec: avoid variable shadowing warning")

(just to give a few of the examples I fixed recently). These are of
course double-plus annoying in header files, since they show up in
completely unrelated code when the header file is including, making the
tools effectively useless.

> And the compiler warnings
> I get when testing with all four combinations of const and volatile
> also supports this by saying "expected 'const void *' but ..." 

It's not a compiler warning though that I'm getting.

What tool are you using to get such a warning?

On gcc 6.1.1, I'm getting no warning (from the compiler) either way,
even with W=2, and the gcc documentation notes the fact that it treats
it as passing void *:

https://gcc.gnu.org/onlinedocs/gcc/Object-Size-Checking.html

> (arguably the compiler should accept volatile here too). To be
> honest, for code in other trees where I'm maintainer, I'd reject such
> casting away of constness, and demand the utility to get fixed
> instead.

That could be done, but arguably "the tool" (I suppose you also never
run sparse) is actually behaving correctly here - the "function" *is*
defined to pass void *, so it's a correct warning.

Regardless though, it's fairly pointless to worry about it here since
it's a builtin that's evaluated at compile time, so nothing can really
happen.

johannes