Repository navigation
enable -Wformat #378
Description
A previous attempt at re-enabling -Wformat showed that there were LOTS of instances of this warning throughout the codebase. Simply re-applying that patch and doing an arm64 defconfig, I experienced 236 unique instances of this warning. Similar to @kees ' work enabling -Wvla, I'd like to find and fix all of these, then re-send the patch re-enabling the warning.
Assuming these aren't false positives, I think the only tricky cases we may encounter may come from config dependent types. ex.
#ifdef CONFIG_FOO
int bar = 0;
#else
float bar = 0;
#endif
printk("bar: %d\n"); // warning: format specifies type 'int' but the argument has type 'float' [-Wformat]which can be fixed via:
+prink("bar: "
+#ifdef CONFIG_FOO
+ "%d"
+#else
+ "%f"
+#endif
+ "\n");
-printk("bar: %d\n");(or whatever maintainers prefer). I don't expect many of those cases, but maybe I'm mistaken.
Also (meta), usually we file individual issues, but maybe it's better to use this one lone issue to track all the instances? (not sure; open to thoughts).
Also, let's find a way to split the work? And keep each other cc'ed when sending patches? (we should probably set up a mailing list to always cc for patches related to Clang Built Linux)
Activity
nathanchance commentedon Feb 25, 2019
Given that some warnings might be easier fixed/received than others, it is still probably better to file individual issues and use the
-Wformatlabel to link them all. We can either file issues as we deal with them or just take the list that I made above and file issues for all of them. I'd find the former easier to manage.I've been thinking this for a bit. I usually CC you but I am sure there are others who would like to be kept in the loop as well.
nickdesaulniers commentedon Feb 25, 2019
https://groups.google.com/forum/#!forum/clang-built-linux
nathanchance commentedon Feb 25, 2019
So the email we'd CC is
clang-built-linux@googlegroups.com?nickdesaulniers commentedon Feb 25, 2019
it would be nice to get something on http://vger.kernel.org/vger-lists.html. I bet @arndb knows who to talk to about that.
nathanchance commentedon Feb 25, 2019
Maybe emailing postmaster@vger.kernel.org?
arndb commentedon Feb 26, 2019
kragniz commentedon Feb 26, 2019
I submitted a patch, tell me if I did something wrong: https://lore.kernel.org/r/20190226234853.20441-1-louis@kragniz.eu/
jon-flatley commentedon Feb 27, 2019
It looks good to me. In the future could you CC Nick and I on the patches? That will help us stay organized and will let us add a quick Reviewed-by if a patch needs some traction.
184 remaining items
nhukc commentedon Jun 8, 2020
The warnings leftover look valid to me. They all truncate before printing which seems like unintended behavior. From here https://lore.kernel.org/r/CAHk-=wgoxnmsj8GEVFJSvTwdnWm8wVJthefNk2n6+4TC=20e0Q@mail.gmail.com/ it looks like Linus thinks that using "hh" specifiers on an int is bad style.
I filed https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95588
nickdesaulniers commentedon Nov 10, 2020
Discussion on the list is that we should do a treewide removal of
%hand%hhformat modifiers for print like functions.https://lore.kernel.org/lkml/CAKwvOdku3o0nHhPppPOJzFXa3j1j_4r5ix3kbkduxY3YSpj9wg@mail.gmail.com/T/#m9795462050a04e8401a0ff4176760d189067316e
See also commit cbacb5a ("docs: printk-formats: Stop encouraging use of unnecessary %h[xudi] and %hh[xudi]").
nickdesaulniers commentedon Dec 2, 2020
Documentation in the kernel has been updated to reflect
%hand%hhis frowned upon: https://lore.kernel.org/lkml/a68114afb134b8633905f5a25ae7c4e6799ce8f1.camel@perches.com/Next up is separate efforts to add a warning to checkpatch.pl, and possibly leverage clang-tidy to help automate the removal of these format flags.
nickdesaulniers commentedon Apr 22, 2021
If I remove
KBUILD_CFLAGS += -Wno-formatfrom scripts/Makefile.extrawarn, I can repro, though for x86_64 defconfig, there's only 2 cases (net/ipv6/ah6.c:710, net/ipv6/esp6.c:1153)! But there are a lot more for ARCH=arm64 defconfig.nickdesaulniers commentedon Jun 17, 2022
https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git/commit/?h=mm-nonmm-unstable&id=d30dfd490f7dc4cb6a7c11a647bd1ff7a22139e7 (accepted) will help with a bunch of calls to
ntohsbeing passed toprintk-like functions.nathanchance commentedon Aug 10, 2022
Congratulations everyone!
https://git.kernel.org/linus/258fafcd0683d9ccfa524129d489948ab3ddc24c
nickdesaulniers commentedon Aug 11, 2022
Reverted by Linus: https://git.kernel.org/linus/21f9c8a13bb2a0c24d9c6b86bc0896542a28c197
Update: Shorten link (dileks)
nickdesaulniers commentedon Aug 19, 2022
llvm/llvm-project#57102 is the upstream report.
I've also posted https://reviews.llvm.org/D132266 to discuss feasibility of moving these to
-Wformat-pedantic.Should that be accepted, I would first send https://lore.kernel.org/linux-kbuild/20220716084532.2324050-1-youngmin.nam@samsung.com/ in a series that then wrapped that in an
if clang version >= 16check re-enabling-Wformatelse at least enable-Wformat-insufficient-args.nathanchance commentedon Aug 19, 2022
Ah, that is a decent workaround. It might be interesting to see if we could enable all the other sub warnings of
-Wformat.nickdesaulniers commentedon Aug 24, 2022
https://reviews.llvm.org/D132568 is probably a better fix.
@nathanchance had question regarding which flags were in the
-Wformatgroup.https://github.com/llvm/llvm-project/blob/cff5bef948c91e4919de8a5fb9765e0edc13f3de/clang/include/clang/Basic/DiagnosticGroups.td#L934-L938 is the definition of the group. Also, https://clang.llvm.org/docs/DiagnosticsReference.html#wformat which is generated from that source mentions:
Also controls
-Wformat-extra-args,-Wformat-insufficient-args,-Wformat-invalid-specifier,-Wformat-security,-Wformat-y2k,-Wformat-zero-length,-Wnonnull.The kernel already explicitly disables
-Wformat-security, so that shouldn't be re-enabled. https://clang.llvm.org/docs/DiagnosticsReference.html#wformat-y2k says that-Wformat-y2kdoes nothing in clang.inclyc commentedon Sep 1, 2022
Thanks @nickdesaulniers ! I've landed https://reviews.llvm.org/D132568 in llvm/llvm-project@e3bd67e :)
nickdesaulniers commentedon Sep 1, 2022
Thanks @inclyc !
I'm working on a kernel patch for this. Looks like
-Wformat-insufficient-argsis clang-12+...nickdesaulniers commentedon Sep 1, 2022
Once more unto the breach! https://lore.kernel.org/lkml/20220901175913.2183047-1-ndesaulniers@google.com/
nathanchance commentedon Sep 4, 2022
https://git.kernel.org/linus/b0839b281c427e844143dba3893e25c83cdd6c17
Congrats everyone, round 2 ;)
bwendling commentedon Oct 11, 2022