Re: [PATCH v1 1/1] bpf: Disable -Wsuggest-attribute=format
From: Andrii Nakryiko
Date: Mon Dec 15 2025 - 13:23:33 EST
On Mon, Dec 15, 2025 at 10:13 AM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Mon, Dec 15, 2025 at 9:36 AM Andrii Nakryiko
> <andrii.nakryiko@xxxxxxxxx> wrote:
> >
> > On Wed, Dec 10, 2025 at 5:12 AM Andy Shevchenko
> > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> > >
> > > The printing functions in BPF code are using printf() type of format,
> > > and compiler is not happy about them as is:
> > >
> > > kernel/bpf/helpers.c:1069:9: error: function ‘____bpf_snprintf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
> > > 1069 | err = bstr_printf(str, str_size, fmt, data.bin_args);
> > > | ^~~
> > >
> > > kernel/bpf/stream.c:241:9: error: function ‘bpf_stream_vprintk_impl’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
> > > 241 | ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt__str, data.bin_args);
> > > | ^~~
> > >
> > > kernel/trace/bpf_trace.c:377:9: error: function ‘____bpf_trace_printk’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
> > > 377 | ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt, data.bin_args);
> > > | ^~~
> > >
> > > kernel/trace/bpf_trace.c:433:9: error: function ‘____bpf_trace_vprintk’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
> > > 433 | ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt, data.bin_args);
> > > | ^~~
> > >
> > > kernel/trace/bpf_trace.c:475:9: error: function ‘____bpf_seq_printf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
> > > 475 | seq_bprintf(m, fmt, data.bin_args);
> > > | ^~~~~~~~~~~
> > >
> >
> > I just want to point out that the compiler suggestion is wrong here
> > and these functions do not follow printf semantics. Yes, they have
> > printf format string argument, but arguments themselves are passed
> > using a special convention that the compiler won't know how to verify
> > properly. So now, these are not candidates for gnu_printf, and it
> > would be nice to have some way to shut up GCC for individual function
> > instead of blanket -Wno-suggest-attribute for the entire file.
> >
> > Similarly, I see you marked bstr_printf() with __printf() earlier.
> > That also seems wrong, so you might want to fix that mistake as well,
> > while at it.
> >
> > Maybe the pragma push/pop approach would be a bit better and more
> > explicit in the code?
>
> I suggested using makefile and file level disable to avoid polluting
> the code. Even when attr-print applies it doesn't help definitions.
> The attribute is only useful in declaration and in our case it's not
> going to be in vmlinux.h or in bpf_helpers.h
> So having it right or wrong in .c is misleading.
Makefile is fine, even if it's a big hammer, I don't mind or care.
But I think instead of Makefile changes we should fix the root cause
here. And that seems to be just wrong __printf annotations for
seq_bprintf and bstr_printf. They are not printf-like, they should not
be marked as such, and then the compiler won't be wrongly suggesting
bpf_stream_vprintk_impl (and others that make use of either
bstr_printf or seq_bprintf) to be marked with __printf.