Re: [PATCH 0/3] Clean the new GCC 9 -Wmissing-attributes warnings

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Laura Abbott <labbott@redhat.com>, Arnd Bergmann <arnd@arndb.de>,
	Martin Sebor <msebor@gcc.gnu.org>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Luc Van Oostenryck <luc.vanoostenryck@gmail.com>,
	Andrey Konovalov <andreyknvl@google.com>,
	Kees Cook <keescook@chromium.org>,
	Sean Christopherson <sean.j.christopherson@intel.com>,
	Jessica Yu <jeyu@kernel.org>,
	Masahiro Yamada <yamada.masahiro@socionext.com>,
	James Morris <james.morris@microsoft.com>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Borislav Petkov <bp@suse.de>, Matt Mullins <mmullins@fb.com>,
	Vincent Whitchurch <vincent.whitchurch@axis.com>,
	WANG Chao <chao.wang@ucloud.cn>
Subject: Re: [PATCH 0/3] Clean the new GCC 9 -Wmissing-attributes warnings
Date: Sat, 9 Feb 2019 12:25:42 +0100	[thread overview]
Message-ID: <CAKv+Gu9a=W6Fg+ai_W-HB1gL5oUg=FKcGdLk_15CdbHLWt6gMg@mail.gmail.com> (raw)
In-Reply-To: <CANiq72nf2qyNxJ2Z9nypzqUC99-v5+WC_f3o98P-zoMqy+5sLw@mail.gmail.com>

On Sat, 9 Feb 2019 at 12:19, Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Sat, Feb 9, 2019 at 11:44 AM Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
> >
> > On Sat, 9 Feb 2019 at 01:09, Miguel Ojeda
> > <miguel.ojeda.sandonis@gmail.com> wrote:
> > >
> > > The upcoming GCC 9 release extends the -Wmissing-attributes warnings
> > > (enabled by -Wall) to C and aliases: it warns when particular function
> > > attributes are missing in the aliases but not in their target, e.g.:
> > >
> > >     void __cold f(void) {}
> >
> > Apologies if I missed any discussions on this topic, but I would argue
> > that 'cold' is not an attribute that has to be applied to the function
> > pointer as well as each of its targets, since it basically decides the
> > placement in the binary, and it doesn't affect the nature of the code
> > being referenced.
>
> It also affects the optimizer in two different ways AFAIK:
>
>   * For the function itself, it gets optimized for size instead of speed.
>   * For callers, the paths that lead to the calls are treated as unlikely.
>

That seems reasonable, but that still does not mean it is necessarily
a problem if you apply 'cold' to one but not the other.

So to me, it makes perfect sense to permit 'cold' on the target, but
not on the function pointer itself, in which case you get 1) but not
2)

> So GCC reports it because you would be (likely) missing the
> optimizations you expected if you are using the alias instead of the
> target.
>

I see how that could be reasonable for extern declarations that do not
match the definition, since in that case, it is assumed that there is
only one instance of the function. For function pointers, I don't
think this assumption is valid.

> In our case in patch 3, we do not want the optimization for callers,
> which is why we don't mark the extern declaration as __cold (see the
> commit message).
>

You did not cc me on the whole set, so I don't have the patch. But in
any case, GCC 9 has not been released so we should still have time to
talk sense into the GCC guys.