linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	linux-kernel@vger.kernel.org, clang-built-linux@googlegroups.com,
	Nathan Chancellor <natechancellor@gmail.com>,
	Nick Desaulniers <ndesaulniers@google.com>
Subject: Re: [PATCH] waitqueue: fix clang -Wuninitialized warnings
Date: Thu, 11 Jul 2019 17:49:49 -0700	[thread overview]
Message-ID: <20190711174949.dc74310efd1fd3c8bd4ea276@linux-foundation.org> (raw)
In-Reply-To: <20190703081119.209976-1-arnd@arndb.de>

On Wed,  3 Jul 2019 10:10:55 +0200 Arnd Bergmann <arnd@arndb.de> wrote:

> When CONFIG_LOCKDEP is set, every use of DECLARE_WAIT_QUEUE_HEAD_ONSTACK()
> produces an annoying 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; })
>                                        ^~~~

<scratches head>

Surely clang is being extraordinarily dumb here?

DECLARE_WAIT_QUEUE_HEAD_ONSTACK() is effectively doing

	struct wait_queue_head name = ({ __init_waitqueue_head(&name) ; name; })

which is perfectly legitimate!  clang has no business assuming that
__init_waitqueue_head() will do any reads from the pointer which it was
passed, nor can clang assume that __init_waitqueue_head() leaves any of
*name uninitialized.

Does it also warn if code does this?

	struct wait_queue_head name;
	__init_waitqueue_head(&name);
	name = name;

which is equivalent, isn't it?


The proposed solution is, effectively, to open-code
__init_waitqueue_head() at each DECLARE_WAIT_QUEUE_HEAD_ONSTACK()
callsite.  That's pretty unpleasant and calls for an explanatory
comment at the __WAIT_QUEUE_HEAD_INIT_ONSTACK() definition site as well
as a cautionary comment at the __init_waitqueue_head() definition so we
can keep the two versions in sync as code evolves.


Hopefully clang will soon be hit with the cluebat (yes?) and this
change becomes obsolete in the quite short term.  Surely 6-12 months
from now nobody will be using the uncluebatted version of clang on
contemporary kernel sources so we get to remove this nastiness again. 
Which makes me wonder whether we should merge it at all.


  parent reply	other threads:[~2019-07-12  0:49 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-03  8:10 [PATCH] waitqueue: fix clang -Wuninitialized warnings Arnd Bergmann
2019-07-03 17:58 ` Nathan Chancellor
2019-07-09 19:27   ` Arnd Bergmann
2019-07-12  7:28     ` Peter Zijlstra
2019-07-12  0:49 ` Andrew Morton [this message]
2019-07-12  7:45   ` Arnd Bergmann
2019-07-12  7:54     ` Nathan Chancellor
2019-07-12 14:50       ` Arnd Bergmann
2019-07-12 16:48     ` Nick Desaulniers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190711174949.dc74310efd1fd3c8bd4ea276@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=clang-built-linux@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=natechancellor@gmail.com \
    --cc=ndesaulniers@google.com \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).