| Message ID | 20190719113638.4189771-1-arnd@arndb.de |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] waitqueue: shut up clang -Wuninitialized warnings | expand |
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>
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.
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
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
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
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.
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
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(-) -- 2.20.0