bpftool icon indicating copy to clipboard operation
bpftool copied to clipboard

Do not use `-Wformat-signedness` when compiling with clang (clang doesn't know this flag)

Open qmonnet opened this issue 9 months ago • 6 comments

Commit 4c56992b53cc125f6939a971f01d4705996f5e0f introduced the use of warning -Wformat-signedness when compiling bpftool, but this warning seems to be unknown to clang, which now produces the following output when compiling bpftool:

[...]
warning: warning: unknown warning option '-Wformat-signedness' [-Wunknown-warning-option]
unknown warning option '-Wformat-signedness' [-Wunknown-warning-option]
warning: unknown warning option '-Wformat-signedness' [-Wunknown-warning-option]
warning: unknown warning option '-Wformat-signedness' [-Wunknown-warning-option]
warning: unknown warning option '-Wformat-signedness' [-Wunknown-warning-option]
warning: unknown warning option '-Wformat-signedness' [-Wunknown-warning-option]
warning: unknown warning option '-Wformat-signedness' [-Wunknown-warning-option]
warning: unknown warning option '-Wformat-signedness' [-Wunknown-warning-option]
warning: unknown warning option '-Wformat-signedness' [-Wunknown-warning-option]
warning: unknown warning option '-Wformat-signedness' [-Wunknown-warning-option]
warning: unknown warning option '-Wformat-signedness' [-Wunknown-warning-option]
warning: unknown warning option '-Wformat-signedness' [-Wunknown-warning-option]
warning: unknown warning option '-Wformat-signedness' [-Wunknown-warning-option]
warning: unknown warning option '-Wformat-signedness' [-Wunknown-warning-option]
warning: unknown warning option '-Wformat-signedness' [-Wunknown-warning-option]
warning: unknown warning option '-Wformat-signedness' [-Wunknown-warning-option]
warning: unknown warning option '-Wformat-signedness' [-Wunknown-warning-option]
[...]

(Example from last PR)

We should not use this warning when compiling with clang. Should be easy enough to fix in the Makefile.

Cc @mrpre, if you feel like looking into it 🙂 I'll fix it at some point, otherwise.

qmonnet avatar May 02 '25 09:05 qmonnet

"-Wformat-signedness" supported by LLVM since 19.1.0

https://github.com/llvm/llvm-project/commit/ea92b1f9d0fc31f1fd97ad04eb0412003a37cb0d
https://clang.llvm.org/docs/DiagnosticsReference.html#wformat-signedness

I think it's okay to remove this Clang compilation option for now, but I believe we should reconsider adding it back in the future, once our CI dependencies are updated to a higher version of LLVM.

Alternatively, just update our LLVM dependency instead.

mrpre avatar May 03 '25 10:05 mrpre

Oh, I didn't know that - thanks for letting me know.

Probably not worth removing the option in that case, the warning is harmless and it will go out soon as distributions transition to newer versions. We can always come back to it if anyone complains.

Yes, I'll need to update the LLVM binaries used in CI at some point.

qmonnet avatar May 04 '25 19:05 qmonnet

Related: https://lore.kernel.org/bpf/[email protected]/t/#u

qmonnet avatar Sep 25 '25 10:09 qmonnet

Hi @qmonnet ,

I have started looking at this, and I could reproduce this when I do a make LLVM=1 after downgrading clang/llvm to 18. I have applied the above patch and it fixes the above warnings, so we will be taking it ?

Thanks, Harshit

harshimogalapalli avatar Dec 05 '25 16:12 harshimogalapalli

Hi Quentin,

> 
> Hi, how annoying is this warning?

Not too bad, it prints out 5~6 times warnings (see the build log below).
Though the warnings are a bit noisy, they does not break the build.

> I'm asking because as far as I
> understand, the option has been introduced in LLVM 19.1.0 [0] - the
> latest being 21.1.0 already - so we won't need this check once distros
> have transitioned, and I'm a bit reluctant to add it.

I agree the warning likely won't affect distros, as package maintainers
will hide it. But it can annoy _paranoid_ developers in day-to-day
builds ;)

As the warning doesn't block anything, so I am fine with not merging
this patch.

Thanks,
Leo

Just followed this discussion from the email thread, so the idea would be to just let distros update to clang19+ so they won't see these warnings ?

Thanks, Harshit

harshimogalapalli avatar Dec 05 '25 16:12 harshimogalapalli

Yep, it looks like the warning is going away as everyone transitions to newer LLVM. It looks like it's not worth a fix at this point. I kept the issue open as a reference for the discussion, but I don't think there's any change needed. Thanks for looking into it, though!

qmonnet avatar Dec 05 '25 17:12 qmonnet