Re: [GIT PULL] more printk for 6.15

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

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@linux-foundation.org> wrote:
> > > On Wed, 2 Apr 2025 at 11:39, Andy Shevchenko <andy.shevchenko@gmail.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@linux.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