Thread (8 messages) flat view 8 messages, 5 authors, 2025-12-16

Re: [PATCH v1 1/1] bpf: Disable -Wsuggest-attribute=format

From: Andrii Nakryiko <hidden>
Date: 2025-12-15 18:23:33
Also in: bpf, lkml

On Mon, Dec 15, 2025 at 10:13 AM Alexei Starovoitov
[off-list ref] wrote:
On Mon, Dec 15, 2025 at 9:36 AM Andrii Nakryiko
[off-list ref] wrote:
quoted
On Wed, Dec 10, 2025 at 5:12 AM Andy Shevchenko
[off-list ref] wrote:
quoted
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help