* [PATCH] [v2] waitqueue: shut up clang -Wuninitialized warnings
@ 2019-07-19 11:36 Arnd Bergmann
2019-07-19 19:00 ` Nathan Chancellor
2019-07-23 10:50 ` Peter Zijlstra
0 siblings, 2 replies; 7+ messages in thread
From: Arnd Bergmann @ 2019-07-19 11:36 UTC (permalink / raw)
To: Ingo Molnar, Peter Zijlstra
Cc: Arnd Bergmann, Nathan Chancellor, Andrew Morton, linux-kernel,
clang-built-linux
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] [v2] waitqueue: shut up clang -Wuninitialized warnings
2019-07-19 11:36 [PATCH] [v2] waitqueue: shut up clang -Wuninitialized warnings Arnd Bergmann
@ 2019-07-19 19:00 ` Nathan Chancellor
2019-07-23 10:50 ` Peter Zijlstra
1 sibling, 0 replies; 7+ messages in thread
From: Nathan Chancellor @ 2019-07-19 19:00 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Ingo Molnar, Peter Zijlstra, Andrew Morton, linux-kernel,
clang-built-linux
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>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] [v2] waitqueue: shut up clang -Wuninitialized warnings
2019-07-19 11:36 [PATCH] [v2] waitqueue: shut up clang -Wuninitialized warnings Arnd Bergmann
2019-07-19 19:00 ` Nathan Chancellor
@ 2019-07-23 10:50 ` Peter Zijlstra
2019-07-23 11:03 ` Arnd Bergmann
1 sibling, 1 reply; 7+ messages in thread
From: Peter Zijlstra @ 2019-07-23 10:50 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Ingo Molnar, Nathan Chancellor, Andrew Morton, linux-kernel,
clang-built-linux
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] [v2] waitqueue: shut up clang -Wuninitialized warnings
2019-07-23 10:50 ` Peter Zijlstra
@ 2019-07-23 11:03 ` Arnd Bergmann
2019-07-23 20:21 ` Nathan Chancellor
0 siblings, 1 reply; 7+ messages in thread
From: Arnd Bergmann @ 2019-07-23 11:03 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Ingo Molnar, Nathan Chancellor, Andrew Morton,
Linux Kernel Mailing List, clang-built-linux
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] [v2] waitqueue: shut up clang -Wuninitialized warnings
2019-07-23 11:03 ` Arnd Bergmann
@ 2019-07-23 20:21 ` Nathan Chancellor
2019-07-23 20:54 ` Nick Desaulniers
0 siblings, 1 reply; 7+ messages in thread
From: Nathan Chancellor @ 2019-07-23 20:21 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Peter Zijlstra, Ingo Molnar, Andrew Morton,
Linux Kernel Mailing List, clang-built-linux
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] [v2] waitqueue: shut up clang -Wuninitialized warnings
2019-07-23 20:21 ` Nathan Chancellor
@ 2019-07-23 20:54 ` Nick Desaulniers
2019-07-24 7:31 ` Peter Zijlstra
0 siblings, 1 reply; 7+ messages in thread
From: Nick Desaulniers @ 2019-07-23 20:54 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Arnd Bergmann, Peter Zijlstra, Ingo Molnar, Andrew Morton,
Linux Kernel Mailing List, clang-built-linux
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] [v2] waitqueue: shut up clang -Wuninitialized warnings
2019-07-23 20:54 ` Nick Desaulniers
@ 2019-07-24 7:31 ` Peter Zijlstra
0 siblings, 0 replies; 7+ messages in thread
From: Peter Zijlstra @ 2019-07-24 7:31 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Nathan Chancellor, Arnd Bergmann, Ingo Molnar, Andrew Morton,
Linux Kernel Mailing List, clang-built-linux
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-07-24 7:31 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-19 11:36 [PATCH] [v2] waitqueue: shut up clang -Wuninitialized warnings Arnd Bergmann
2019-07-19 19:00 ` Nathan Chancellor
2019-07-23 10:50 ` Peter Zijlstra
2019-07-23 11:03 ` Arnd Bergmann
2019-07-23 20:21 ` Nathan Chancellor
2019-07-23 20:54 ` Nick Desaulniers
2019-07-24 7:31 ` Peter Zijlstra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).