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.