Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang or clang-tidy] -Wformat awareness for <cinttypes> #41959

Open
LebedevRI opened this issue Jul 13, 2019 · 2 comments
Open

[clang or clang-tidy] -Wformat awareness for <cinttypes> #41959

LebedevRI opened this issue Jul 13, 2019 · 2 comments
Labels
bugzilla Issues migrated from bugzilla clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer

Comments

@LebedevRI
Copy link
Member

Bugzilla Link 42614
Version trunk
OS Linux
CC @AaronBallman,@JonasToth,@gribozavr,@zygoloid,@rjmccall

Extended Description

printf()-style functions have a well-defined format string:
https://en.cppreference.com/w/cpp/io/c/fprintf
clang knows how to verify it (-Wformat),

However there is a huge pitfall hiding in plain sight.
If one defines the variable e.g. as uint64_t: normally it is a 'unsigned long',
so one will just use %lu - that is what clang recommends.
But on different platform uint64_t can be 'unsigned long long',
and -Wformat will complain that '%llu' should be used.

Neither of these is the "correct" solution - PRIu64 should be used instead.
https://en.cppreference.com/w/cpp/header/cinttypes

Now, obviously the current -Wformat behavior isn't wrong - it does produce
the correct results on the current platform - but they aren't great, since
they don't catch (and actively advertise) platform-dependent format string.

While i expect it may be reasonably trivial to distinguish whether the
printf() parameter is int or int32_t (e.g.), i'm honestly not sure how
to deal with format string parsing - the current approach won't work,
as it would need to be done before macro substitution.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
@thesamesam
Copy link
Member

FWIW, the GCC counterpart of this bug is https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78014.

@EugeneZelenko EugeneZelenko added clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer and removed clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 9, 2022
@frederick-vs-ja
Copy link
Contributor

frederick-vs-ja commented Oct 19, 2022

PRIu64 is a macro that expands to a string literal and is generally concatenated later. How can the compiler know whether it has been used? Is it even possible for string literals to carry such additional information?

I think the new length modifiers (in C23) added by WG14-N2680 can be helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer
Projects
None yet
Development

No branches or pull requests

4 participants