[PATCH] [v2] waitqueue: shut up clang -Wuninitialized warnings

[PATCH] [v2] waitqueue: shut up clang -Wuninitialized warnings

[Arnd Bergmann]

When CONFIG_LOCKDEP is set, every use of DECLARE_WAIT_QUEUE_HEAD_ONSTACK()
produces an bogus warning from clang, which is particularly annoying
for allmodconfig builds:

fs/namei.c:1646:34: error: variable 'wq' is uninitialized when used within its own initialization [-Werror,-Wuninitialized]
        DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
                                        ^~
include/linux/wait.h:74:63: note: expanded from macro 'DECLARE_WAIT_QUEUE_HEAD_ONSTACK'
        struct wait_queue_head name = __WAIT_QUEUE_HEAD_INIT_ONSTACK(name)
                               ~~~~                                  ^~~~
include/linux/wait.h:72:33: note: expanded from macro '__WAIT_QUEUE_HEAD_INIT_ONSTACK'
        ({ init_waitqueue_head(&name); name; })
                                       ^~~~

A patch for clang has already been proposed and should soon be
merged for clang-9, but for now all clang versions produce the
warning in an otherwise (almost) clean allmodconfig build.

Link: https://bugs.llvm.org/show_bug.cgi?id=31829
Link: https://bugs.llvm.org/show_bug.cgi?id=42604
Link: https://lore.kernel.org/lkml/20190703081119.209976-1-arnd@arndb.de/
Link: https://reviews.llvm.org/D64678
Link: https://storage.kernelci.org/next/master/next-20190717/arm64/allmodconfig/clang-8/build-warnings.log
Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: given that kernelci is getting close to reporting a clean build for
    clang, I'm trying again with a less invasive approach after my
    first version was not too popular.
---
 include/linux/wait.h | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/include/linux/wait.h b/include/linux/wait.h
index ddb959641709..276499ae1a3e 100644
--- a/include/linux/wait.h
+++ b/include/linux/wait.h
@@ -70,8 +70,17 @@ extern void __init_waitqueue_head(struct wait_queue_head *wq_head, const char *n
 #ifdef CONFIG_LOCKDEP
 # define __WAIT_QUEUE_HEAD_INIT_ONSTACK(name) \
 	({ init_waitqueue_head(&name); name; })
-# define DECLARE_WAIT_QUEUE_HEAD_ONSTACK(name) \
+# if defined(__clang__) && __clang_major__ <= 9
+/* work around https://bugs.llvm.org/show_bug.cgi?id=42604 */
+#  define DECLARE_WAIT_QUEUE_HEAD_ONSTACK(name)					\
+	_Pragma("clang diagnostic push")					\
+	_Pragma("clang diagnostic ignored \"-Wuninitialized\"")			\
+	struct wait_queue_head name = __WAIT_QUEUE_HEAD_INIT_ONSTACK(name)	\
+	_Pragma("clang diagnostic pop")
+# else
+#  define DECLARE_WAIT_QUEUE_HEAD_ONSTACK(name) \
 	struct wait_queue_head name = __WAIT_QUEUE_HEAD_INIT_ONSTACK(name)
+# endif
 #else
 # define DECLARE_WAIT_QUEUE_HEAD_ONSTACK(name) DECLARE_WAIT_QUEUE_HEAD(name)
 #endif
-- 
2.20.0

Re: [PATCH] [v2] waitqueue: shut up clang -Wuninitialized warnings

[Nathan Chancellor]

On Fri, Jul 19, 2019 at 01:36:00PM +0200, Arnd Bergmann wrote:
> When CONFIG_LOCKDEP is set, every use of DECLARE_WAIT_QUEUE_HEAD_ONSTACK()
> produces an bogus warning from clang, which is particularly annoying
> for allmodconfig builds:
> 
> fs/namei.c:1646:34: error: variable 'wq' is uninitialized when used within its own initialization [-Werror,-Wuninitialized]
>         DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
>                                         ^~
> include/linux/wait.h:74:63: note: expanded from macro 'DECLARE_WAIT_QUEUE_HEAD_ONSTACK'
>         struct wait_queue_head name = __WAIT_QUEUE_HEAD_INIT_ONSTACK(name)
>                                ~~~~                                  ^~~~
> include/linux/wait.h:72:33: note: expanded from macro '__WAIT_QUEUE_HEAD_INIT_ONSTACK'
>         ({ init_waitqueue_head(&name); name; })
>                                        ^~~~
> 
> A patch for clang has already been proposed and should soon be
> merged for clang-9, but for now all clang versions produce the
> warning in an otherwise (almost) clean allmodconfig build.
> 
> Link: https://bugs.llvm.org/show_bug.cgi?id=31829
> Link: https://bugs.llvm.org/show_bug.cgi?id=42604
> Link: https://lore.kernel.org/lkml/20190703081119.209976-1-arnd@arndb.de/
> Link: https://reviews.llvm.org/D64678
> Link: https://storage.kernelci.org/next/master/next-20190717/arm64/allmodconfig/clang-8/build-warnings.log
> Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> v2: given that kernelci is getting close to reporting a clean build for
>     clang, I'm trying again with a less invasive approach after my
>     first version was not too popular.
> ---
>  include/linux/wait.h | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/wait.h b/include/linux/wait.h
> index ddb959641709..276499ae1a3e 100644
> --- a/include/linux/wait.h
> +++ b/include/linux/wait.h
> @@ -70,8 +70,17 @@ extern void __init_waitqueue_head(struct wait_queue_head *wq_head, const char *n
>  #ifdef CONFIG_LOCKDEP
>  # define __WAIT_QUEUE_HEAD_INIT_ONSTACK(name) \
>  	({ init_waitqueue_head(&name); name; })
> -# define DECLARE_WAIT_QUEUE_HEAD_ONSTACK(name) \
> +# if defined(__clang__) && __clang_major__ <= 9

Might look cleaner if we used CONFIG_CC_IS_CLANG and
CONFIG_CLANG_VERSION but I have no strong opinion.

It works as is, I checked clang-9, clang-10, and GCC 9.1.0.

Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>

Re: [PATCH] [v2] waitqueue: shut up clang -Wuninitialized warnings

[Peter Zijlstra]

On Fri, Jul 19, 2019 at 01:36:00PM +0200, Arnd Bergmann wrote:
> When CONFIG_LOCKDEP is set, every use of DECLARE_WAIT_QUEUE_HEAD_ONSTACK()
> produces an bogus warning from clang, which is particularly annoying
> for allmodconfig builds:
> 
> fs/namei.c:1646:34: error: variable 'wq' is uninitialized when used within its own initialization [-Werror,-Wuninitialized]
>         DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
>                                         ^~
> include/linux/wait.h:74:63: note: expanded from macro 'DECLARE_WAIT_QUEUE_HEAD_ONSTACK'
>         struct wait_queue_head name = __WAIT_QUEUE_HEAD_INIT_ONSTACK(name)
>                                ~~~~                                  ^~~~
> include/linux/wait.h:72:33: note: expanded from macro '__WAIT_QUEUE_HEAD_INIT_ONSTACK'
>         ({ init_waitqueue_head(&name); name; })
>                                        ^~~~
> 
> A patch for clang has already been proposed and should soon be
> merged for clang-9, but for now all clang versions produce the
> warning in an otherwise (almost) clean allmodconfig build.
> 
> Link: https://bugs.llvm.org/show_bug.cgi?id=31829
> Link: https://bugs.llvm.org/show_bug.cgi?id=42604
> Link: https://lore.kernel.org/lkml/20190703081119.209976-1-arnd@arndb.de/
> Link: https://reviews.llvm.org/D64678
> Link: https://storage.kernelci.org/next/master/next-20190717/arm64/allmodconfig/clang-8/build-warnings.log
> Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> v2: given that kernelci is getting close to reporting a clean build for
>     clang, I'm trying again with a less invasive approach after my
>     first version was not too popular.
> ---
>  include/linux/wait.h | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/wait.h b/include/linux/wait.h
> index ddb959641709..276499ae1a3e 100644
> --- a/include/linux/wait.h
> +++ b/include/linux/wait.h
> @@ -70,8 +70,17 @@ extern void __init_waitqueue_head(struct wait_queue_head *wq_head, const char *n
>  #ifdef CONFIG_LOCKDEP
>  # define __WAIT_QUEUE_HEAD_INIT_ONSTACK(name) \
>  	({ init_waitqueue_head(&name); name; })
> -# define DECLARE_WAIT_QUEUE_HEAD_ONSTACK(name) \
> +# if defined(__clang__) && __clang_major__ <= 9
> +/* work around https://bugs.llvm.org/show_bug.cgi?id=42604 */
> +#  define DECLARE_WAIT_QUEUE_HEAD_ONSTACK(name)					\
> +	_Pragma("clang diagnostic push")					\
> +	_Pragma("clang diagnostic ignored \"-Wuninitialized\"")			\
> +	struct wait_queue_head name = __WAIT_QUEUE_HEAD_INIT_ONSTACK(name)	\
> +	_Pragma("clang diagnostic pop")
> +# else
> +#  define DECLARE_WAIT_QUEUE_HEAD_ONSTACK(name) \
>  	struct wait_queue_head name = __WAIT_QUEUE_HEAD_INIT_ONSTACK(name)
> +# endif

While this is indeed much better than before; do we really want to do
this? That is, since clang-9 release will not need this, we're basically
doing the above for pre-release compilers only.

Re: [PATCH] [v2] waitqueue: shut up clang -Wuninitialized warnings

[Arnd Bergmann]

On Tue, Jul 23, 2019 at 12:50 PM Peter Zijlstra <peterz@infradead.org> wrote:
> On Fri, Jul 19, 2019 at 01:36:00PM +0200, Arnd Bergmann wrote:
> > --- a/include/linux/wait.h
> > +++ b/include/linux/wait.h
> > @@ -70,8 +70,17 @@ extern void __init_waitqueue_head(struct wait_queue_head *wq_head, const char *n
> >  #ifdef CONFIG_LOCKDEP
> >  # define __WAIT_QUEUE_HEAD_INIT_ONSTACK(name) \
> >       ({ init_waitqueue_head(&name); name; })
> > -# define DECLARE_WAIT_QUEUE_HEAD_ONSTACK(name) \
> > +# if defined(__clang__) && __clang_major__ <= 9
> > +/* work around https://bugs.llvm.org/show_bug.cgi?id=42604 */
> > +#  define DECLARE_WAIT_QUEUE_HEAD_ONSTACK(name)                                      \
> > +     _Pragma("clang diagnostic push")                                        \
> > +     _Pragma("clang diagnostic ignored \"-Wuninitialized\"")                 \
> > +     struct wait_queue_head name = __WAIT_QUEUE_HEAD_INIT_ONSTACK(name)      \
> > +     _Pragma("clang diagnostic pop")
> > +# else
> > +#  define DECLARE_WAIT_QUEUE_HEAD_ONSTACK(name) \
> >       struct wait_queue_head name = __WAIT_QUEUE_HEAD_INIT_ONSTACK(name)
> > +# endif
>
> While this is indeed much better than before; do we really want to do
> this? That is, since clang-9 release will not need this, we're basically
> doing the above for pre-release compilers only.

Kernelci currently builds arch/arm and arch/arm64 kernels with clang-8,
and probably won't change to clang-9 until after that is released,
presumably in September.

Anyone doing x86 builds would use a clang-9 snapshot today
because of the asm-goto support, but so far the fix has not
been merged there either. I think the chances of it getting
fixed before the release are fairly good, but I don't know how
long it will actually take.

       Arnd

Re: [PATCH] [v2] waitqueue: shut up clang -Wuninitialized warnings

[Nathan Chancellor]

On Tue, Jul 23, 2019 at 01:03:05PM +0200, Arnd Bergmann wrote:
> On Tue, Jul 23, 2019 at 12:50 PM Peter Zijlstra <peterz@infradead.org> wrote:
> > On Fri, Jul 19, 2019 at 01:36:00PM +0200, Arnd Bergmann wrote:
> > > --- a/include/linux/wait.h
> > > +++ b/include/linux/wait.h
> > > @@ -70,8 +70,17 @@ extern void __init_waitqueue_head(struct wait_queue_head *wq_head, const char *n
> > >  #ifdef CONFIG_LOCKDEP
> > >  # define __WAIT_QUEUE_HEAD_INIT_ONSTACK(name) \
> > >       ({ init_waitqueue_head(&name); name; })
> > > -# define DECLARE_WAIT_QUEUE_HEAD_ONSTACK(name) \
> > > +# if defined(__clang__) && __clang_major__ <= 9
> > > +/* work around https://bugs.llvm.org/show_bug.cgi?id=42604 */
> > > +#  define DECLARE_WAIT_QUEUE_HEAD_ONSTACK(name)                                      \
> > > +     _Pragma("clang diagnostic push")                                        \
> > > +     _Pragma("clang diagnostic ignored \"-Wuninitialized\"")                 \
> > > +     struct wait_queue_head name = __WAIT_QUEUE_HEAD_INIT_ONSTACK(name)      \
> > > +     _Pragma("clang diagnostic pop")
> > > +# else
> > > +#  define DECLARE_WAIT_QUEUE_HEAD_ONSTACK(name) \
> > >       struct wait_queue_head name = __WAIT_QUEUE_HEAD_INIT_ONSTACK(name)
> > > +# endif
> >
> > While this is indeed much better than before; do we really want to do
> > this? That is, since clang-9 release will not need this, we're basically
> > doing the above for pre-release compilers only.
> 
> Kernelci currently builds arch/arm and arch/arm64 kernels with clang-8,
> and probably won't change to clang-9 until after that is released,
> presumably in September.
> 
> Anyone doing x86 builds would use a clang-9 snapshot today
> because of the asm-goto support, but so far the fix has not
> been merged there either. I think the chances of it getting
> fixed before the release are fairly good, but I don't know how
> long it will actually take.
> 
>        Arnd

Furthermore, while x86 will only be supported by clang-9 and up, there
are other architectures/configurations that work with earlier versions
that will never see that fix. There are a few people that still use
clang-7 for example.

In an ideal world, everyone should be using the latest version of clang
because of all of the fixes and improvements that are going into that
latest version but the same can be said of any piece of software. I am
not sure that it is fair to force someone to upgrade when it works for
them. Not everyone runs Ubuntu/Debian to get access to apt.llvm.org
builds or wants to add random repositories to their list or wants to
build clang from source.

I suppose it comes down to policy: if we don't want to support versions
of LLVM before 9.x then we should just break the build when it is
detected but Nick has spoken out against that and I think that he has a
fair point.

https://lore.kernel.org/lkml/CAKwvOdnzrMOCo4RRsfcR=K5ELWU8obgMqtOGZnx_avLrArjpRQ@mail.gmail.com/

Cheers,
Nathan

Re: [PATCH] [v2] waitqueue: shut up clang -Wuninitialized warnings

[Nick Desaulniers]

On Tue, Jul 23, 2019 at 1:22 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> On Tue, Jul 23, 2019 at 01:03:05PM +0200, Arnd Bergmann wrote:
> > On Tue, Jul 23, 2019 at 12:50 PM Peter Zijlstra <peterz@infradead.org> wrote:
> > > On Fri, Jul 19, 2019 at 01:36:00PM +0200, Arnd Bergmann wrote:
> > > > --- a/include/linux/wait.h
> > > > +++ b/include/linux/wait.h
> > > > @@ -70,8 +70,17 @@ extern void __init_waitqueue_head(struct wait_queue_head *wq_head, const char *n
> > > >  #ifdef CONFIG_LOCKDEP
> > > >  # define __WAIT_QUEUE_HEAD_INIT_ONSTACK(name) \
> > > >       ({ init_waitqueue_head(&name); name; })
> > > > -# define DECLARE_WAIT_QUEUE_HEAD_ONSTACK(name) \
> > > > +# if defined(__clang__) && __clang_major__ <= 9
> > > > +/* work around https://bugs.llvm.org/show_bug.cgi?id=42604 */
> > > > +#  define DECLARE_WAIT_QUEUE_HEAD_ONSTACK(name)                                      \
> > > > +     _Pragma("clang diagnostic push")                                        \
> > > > +     _Pragma("clang diagnostic ignored \"-Wuninitialized\"")                 \
> > > > +     struct wait_queue_head name = __WAIT_QUEUE_HEAD_INIT_ONSTACK(name)      \
> > > > +     _Pragma("clang diagnostic pop")
> > > > +# else
> > > > +#  define DECLARE_WAIT_QUEUE_HEAD_ONSTACK(name) \
> > > >       struct wait_queue_head name = __WAIT_QUEUE_HEAD_INIT_ONSTACK(name)
> > > > +# endif
> > >
> > > While this is indeed much better than before; do we really want to do
> > > this? That is, since clang-9 release will not need this, we're basically
> > > doing the above for pre-release compilers only.
> >
> > Kernelci currently builds arch/arm and arch/arm64 kernels with clang-8,
> > and probably won't change to clang-9 until after that is released,
> > presumably in September.
> >
> > Anyone doing x86 builds would use a clang-9 snapshot today
> > because of the asm-goto support, but so far the fix has not
> > been merged there either. I think the chances of it getting
> > fixed before the release are fairly good, but I don't know how
> > long it will actually take.
> >
> >        Arnd
>
> Furthermore, while x86 will only be supported by clang-9 and up, there
> are other architectures/configurations that work with earlier versions
> that will never see that fix. There are a few people that still use
> clang-7 for example.
>
> In an ideal world, everyone should be using the latest version of clang
> because of all of the fixes and improvements that are going into that
> latest version but the same can be said of any piece of software. I am
> not sure that it is fair to force someone to upgrade when it works for
> them. Not everyone runs Ubuntu/Debian to get access to apt.llvm.org
> builds or wants to add random repositories to their list or wants to
> build clang from source.
>
> I suppose it comes down to policy: if we don't want to support versions
> of LLVM before 9.x then we should just break the build when it is
> detected but Nick has spoken out against that and I think that he has a
> fair point.
>
> https://lore.kernel.org/lkml/CAKwvOdnzrMOCo4RRsfcR=K5ELWU8obgMqtOGZnx_avLrArjpRQ@mail.gmail.com/

Note that pre-clang-9 can be used for LTS x86_64; I don't think
CONFIG_JUMP_LABEL was made mandatory for x86 until 4.20 release, IIRC.
There's only a small window of non LTS kernels and only for x86 where
clang-9+ is really necessary.

Thanks,
~Nick Desaulniers

Re: [PATCH] [v2] waitqueue: shut up clang -Wuninitialized warnings

[Peter Zijlstra]

On Tue, Jul 23, 2019 at 01:54:03PM -0700, Nick Desaulniers wrote:
> Note that pre-clang-9 can be used for LTS x86_64; I don't think
> CONFIG_JUMP_LABEL was made mandatory for x86 until 4.20 release, IIRC.
> There's only a small window of non LTS kernels and only for x86 where
> clang-9+ is really necessary.

Given all the code-gen issues we've been finding, I wouldn't want to
touch a pre-9 clang. Irrespective of wether it builds or not, it's
absolutely unreliable crap.