[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z-557YrwVr8bONq4@smile.fi.intel.com>
Date: Thu, 3 Apr 2025 15:07:09 +0300
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Nathan Chancellor <nathan@...nel.org>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
Kees Cook <kees@...nel.org>, Petr Mladek <pmladek@...e.com>,
Sergey Senozhatsky <senozhatsky@...omium.org>,
Steven Rostedt <rostedt@...dmis.org>,
John Ogness <john.ogness@...utronix.de>,
Rasmus Villemoes <linux@...musvillemoes.dk>,
Peter Zijlstra <peterz@...radead.org>, linux-kernel@...r.kernel.org
Subject: Re: [GIT PULL] more printk for 6.15
On Wed, Apr 02, 2025 at 01:34:22PM -0700, Nathan Chancellor wrote:
> On Wed, Apr 02, 2025 at 10:25:46PM +0300, Andy Shevchenko wrote:
> > +Cc: Kees and Nathan (I believe this discussion has some material for
> > you, folks, to think of / comment on / etc)
>
> Thanks, I have commented on the part of the message that seem relevant
> for me.
Thank you!
> > On Wed, Apr 2, 2025 at 10:06 PM Linus Torvalds
> > <torvalds@...ux-foundation.org> wrote:
> > > On Wed, 2 Apr 2025 at 11:39, Andy Shevchenko <andy.shevchenko@...il.com> wrote:
> > > >
> > > > Yes. Clang complains on unknown pragma.
> > >
> > > What a crock.
> > >
> > > It says GCC, for chrissake!
> > >
> > > And clang clearly doesn't complain about
> > >
> > > > +#pragma GCC diagnostic push
> > > > +#pragma GCC diagnostic pop
> > >
> > > which are *not* protected by that #ifndef __clang__ thing.
> > >
> > > So this smells like a clang bug to me.
>
> Yes, clang implements support for '#pragma GCC' for compatability with
> existing source code:
>
> https://clang.llvm.org/docs/UsersManual.html#controlling-diagnostics-via-pragmas
>
> Otherwise, the pragma would need to be duplicated if the warning was
> shared between the compilers (as many are nowadays).
>
> It complains specifically about an unknown warning being passed to
> 'diagnostic ignored':
>
> lib/vsprintf.c:1703:32: error: unknown warning group '-Wsuggest-attribute=format', ignored [-Werror,-Wunknown-warning-option]
> 1703 | #pragma GCC diagnostic ignored "-Wsuggest-attribute=format"
> | ^
> 1 error generated.
>
> Which I suppose you could argue is a bug since it is a GCC pragma,
> although warning on an unknown option to the ignored diagnostic pragma
> is what GCC does as well (it just ignores '#pragma clang' altogether):
>
> $ echo '#pragma GCC diagnostic ignored "-Wfoo"' | gcc -fsyntax-only -x c -
> <stdin>:1:32: warning: unknown option after ‘#pragma GCC diagnostic’ kind [-Wpragmas]
>
> I can look into filing a report upstream about this, however...
>
> > > Can we please use wrapper defines instead so that we don't have that
> > > #ifndef in the middle of code? And since those don't work with
> > > '#pragma', they need to use the _Pragma() operator instead.
> > >
> > > Something like
> > >
> > > #define GCC_PRAGMA(x) _Pragma(#x)
> > >
> > > in compiler-gcc.h, and then add a
> > >
> > > #ifndef GCC_PRAGMA
> > > #define GCC_PRAGMA(x) /* Nothing */
> > > #endif
> > >
> > > and then you can just do
> > >
> > > GCC_PRAGMA(Wsuggest-attribute=format)
> > >
> > > in places like this?
> > >
> > > (Entirely untested: I *despise* pragma in general).
>
> We have the __diag() infrastructure for this already. I think this issue
> would be as simple as the following diff, which makes clang and GCC
> happy without any obvious ifdeffery.
FWIW, I have tested this in my case for both compilers and they are happy with it.
Tested-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> diff --git a/include/linux/compiler-igcc.h b/include/linux/compiler-gcc.h
> index 32048052c64a..5d07c469b571 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -127,6 +127,8 @@
> #define __diag_GCC_8(s)
> #endif
>
> +#define __diag_GCC_all(s) __diag(s)
> +
> #define __diag_ignore_all(option, comment) \
> __diag(__diag_GCC_ignore option)
>
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 01699852f30c..6ff4d85e144e 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -1699,10 +1699,8 @@ char *escaped_string(char *buf, char *end, u8 *addr, struct printf_spec spec,
> return buf;
> }
>
> -#pragma GCC diagnostic push
> -#ifndef __clang__
> -#pragma GCC diagnostic ignored "-Wsuggest-attribute=format"
> -#endif
> +__diag_push();
> +__diag_ignore(GCC, all, "-Wsuggest-attribute=format", "<reason>");
> static char *va_format(char *buf, char *end, struct va_format *va_fmt,
> struct printf_spec spec)
> {
> @@ -1717,7 +1715,7 @@ static char *va_format(char *buf, char *end, struct va_format *va_fmt,
>
> return buf;
> }
> -#pragma GCC diagnostic pop
> +__diag_pop();
>
> static noinline_for_stack
> char *uuid_string(char *buf, char *end, const u8 *addr,
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists