[clang] [llvm] [Sema] Implement support for -Wformat-signedness (PR #74440)
Karl-Johan Karlsson via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 26 01:16:36 PDT 2024
https://github.com/karka228 updated https://github.com/llvm/llvm-project/pull/74440
>From aa9d6cd10ff32fdcdd3d1b2ef334aa70f56cccb1 Mon Sep 17 00:00:00 2001
From: Karl-Johan Karlsson <karl-johan.karlsson at ericsson.com>
Date: Tue, 5 Dec 2023 10:03:00 +0100
Subject: [PATCH 1/9] [Sema] Implement support for -Wformat-signedness
In gcc there exist a modifier option -Wformat-signedness that turns on
additional signedness warnings in the already existing -Wformat warning.
This patch implements that support in clang.
---
clang/include/clang/AST/FormatString.h | 2 +
.../include/clang/Basic/DiagnosticOptions.def | 1 +
clang/include/clang/Driver/Options.td | 3 +
clang/lib/AST/FormatString.cpp | 29 +++--
clang/lib/Driver/ToolChains/Clang.cpp | 2 +
clang/lib/Sema/SemaChecking.cpp | 26 ++++-
format-strings-signedness.c | 107 ++++++++++++++++++
7 files changed, 158 insertions(+), 12 deletions(-)
create mode 100644 format-strings-signedness.c
diff --git a/clang/include/clang/AST/FormatString.h b/clang/include/clang/AST/FormatString.h
index e2232fb4a47153..41cd0443ca2ac7 100644
--- a/clang/include/clang/AST/FormatString.h
+++ b/clang/include/clang/AST/FormatString.h
@@ -275,6 +275,8 @@ class ArgType {
/// The conversion specifier and the argument type are compatible. For
/// instance, "%d" and int.
Match = 1,
+ /// The conversion specifier and the argument type have different sign
+ MatchSignedness,
/// The conversion specifier and the argument type are compatible because of
/// default argument promotions. For instance, "%hhd" and int.
MatchPromotion,
diff --git a/clang/include/clang/Basic/DiagnosticOptions.def b/clang/include/clang/Basic/DiagnosticOptions.def
index 6d0c1b14acc120..a9562e7d8dcb0d 100644
--- a/clang/include/clang/Basic/DiagnosticOptions.def
+++ b/clang/include/clang/Basic/DiagnosticOptions.def
@@ -44,6 +44,7 @@ DIAGOPT(Name, Bits, Default)
#endif
SEMANTIC_DIAGOPT(IgnoreWarnings, 1, 0) /// -w
+DIAGOPT(FormatSignedness, 1, 0) /// -Wformat-signedness
DIAGOPT(NoRewriteMacros, 1, 0) /// -Wno-rewrite-macros
DIAGOPT(Pedantic, 1, 0) /// -pedantic
DIAGOPT(PedanticErrors, 1, 0) /// -pedantic-errors
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 4a954258ce40b6..1698c6c89e6e98 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -954,6 +954,9 @@ def Wdeprecated : Flag<["-"], "Wdeprecated">, Group<W_Group>,
HelpText<"Enable warnings for deprecated constructs and define __DEPRECATED">;
def Wno_deprecated : Flag<["-"], "Wno-deprecated">, Group<W_Group>,
Visibility<[ClangOption, CC1Option]>;
+def Wformat_signedness : Flag<["-"], "Wformat-signedness">,
+ Flags<[HelpHidden]>, Visibility<[ClangOption, CC1Option]>,
+ MarshallingInfoFlag<DiagnosticOpts<"FormatSignedness">>;
def Wl_COMMA : CommaJoined<["-"], "Wl,">, Visibility<[ClangOption, FlangOption]>,
Flags<[LinkerInput, RenderAsInput]>,
HelpText<"Pass the comma separated arguments in <arg> to the linker">,
diff --git a/clang/lib/AST/FormatString.cpp b/clang/lib/AST/FormatString.cpp
index 0c80ad109ccbb2..bdd2f046113e7e 100644
--- a/clang/lib/AST/FormatString.cpp
+++ b/clang/lib/AST/FormatString.cpp
@@ -413,7 +413,7 @@ ArgType::matchesType(ASTContext &C, QualType argTy) const {
return Match;
if (const auto *BT = argTy->getAs<BuiltinType>()) {
// Check if the only difference between them is signed vs unsigned
- // if true, we consider they are compatible.
+ // if true, return match signedness.
switch (BT->getKind()) {
default:
break;
@@ -423,44 +423,53 @@ ArgType::matchesType(ASTContext &C, QualType argTy) const {
[[fallthrough]];
case BuiltinType::Char_S:
case BuiltinType::SChar:
+ if (T == C.UnsignedShortTy || T == C.ShortTy)
+ return NoMatchTypeConfusion;
+ if (T == C.UnsignedCharTy)
+ return MatchSignedness;
+ if (T == C.SignedCharTy)
+ return Match;
+ break;
case BuiltinType::Char_U:
case BuiltinType::UChar:
if (T == C.UnsignedShortTy || T == C.ShortTy)
return NoMatchTypeConfusion;
- if (T == C.UnsignedCharTy || T == C.SignedCharTy)
+ if (T == C.UnsignedCharTy)
return Match;
+ if (T == C.SignedCharTy)
+ return MatchSignedness;
break;
case BuiltinType::Short:
if (T == C.UnsignedShortTy)
- return Match;
+ return MatchSignedness;
break;
case BuiltinType::UShort:
if (T == C.ShortTy)
- return Match;
+ return MatchSignedness;
break;
case BuiltinType::Int:
if (T == C.UnsignedIntTy)
- return Match;
+ return MatchSignedness;
break;
case BuiltinType::UInt:
if (T == C.IntTy)
- return Match;
+ return MatchSignedness;
break;
case BuiltinType::Long:
if (T == C.UnsignedLongTy)
- return Match;
+ return MatchSignedness;
break;
case BuiltinType::ULong:
if (T == C.LongTy)
- return Match;
+ return MatchSignedness;
break;
case BuiltinType::LongLong:
if (T == C.UnsignedLongLongTy)
- return Match;
+ return MatchSignedness;
break;
case BuiltinType::ULongLong:
if (T == C.LongLongTy)
- return Match;
+ return MatchSignedness;
break;
}
// "Partially matched" because of promotions?
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 86a287db72a4eb..561ff84b665c51 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6303,6 +6303,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
CmdArgs.push_back("-fdeprecated-macro");
}
+ Args.AddLastArg(CmdArgs, options::OPT_Wformat_signedness);
+
// Translate GCC's misnamer '-fasm' arguments to '-fgnu-keywords'.
if (Arg *Asm = Args.getLastArg(options::OPT_fasm, options::OPT_fno_asm)) {
if (Asm->getOption().matches(options::OPT_fasm))
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 246e3577809a79..df519bfa66afd5 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -12465,6 +12465,18 @@ isArithmeticArgumentPromotion(Sema &S, const ImplicitCastExpr *ICE) {
S.Context.getFloatingTypeOrder(From, To) < 0;
}
+static clang::analyze_format_string::ArgType::MatchKind
+handleFormatSignedness(clang::analyze_format_string::ArgType::MatchKind Match,
+ DiagnosticOptions &DiagOpts) {
+ if (Match == clang::analyze_format_string::ArgType::MatchSignedness) {
+ if (DiagOpts.FormatSignedness)
+ Match = clang::analyze_format_string::ArgType::NoMatch;
+ else
+ Match = clang::analyze_format_string::ArgType::Match;
+ }
+ return Match;
+}
+
bool
CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
const char *StartSpecifier,
@@ -12508,6 +12520,8 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
ArgType::MatchKind ImplicitMatch = ArgType::NoMatch;
ArgType::MatchKind Match = AT.matchesType(S.Context, ExprTy);
+ Match =
+ handleFormatSignedness(Match, S.getDiagnostics().getDiagnosticOptions());
if (Match == ArgType::Match)
return true;
@@ -12531,6 +12545,8 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
ICE->getType() == S.Context.UnsignedIntTy) {
// All further checking is done on the subexpression
ImplicitMatch = AT.matchesType(S.Context, ExprTy);
+ ImplicitMatch = handleFormatSignedness(
+ ImplicitMatch, S.getDiagnostics().getDiagnosticOptions());
if (ImplicitMatch == ArgType::Match)
return true;
}
@@ -12651,6 +12667,7 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
switch (Match) {
case ArgType::Match:
case ArgType::MatchPromotion:
+ case ArgType::MatchSignedness:
case ArgType::NoMatchPromotionTypeConfusion:
llvm_unreachable("expected non-matching");
case ArgType::NoMatchPedantic:
@@ -12687,8 +12704,10 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
CastFix << (S.LangOpts.CPlusPlus ? ">" : ")");
SmallVector<FixItHint,4> Hints;
- if (AT.matchesType(S.Context, IntendedTy) != ArgType::Match ||
- ShouldNotPrintDirectly)
+ ArgType::MatchKind IntendedMatch = AT.matchesType(S.Context, IntendedTy);
+ IntendedMatch = handleFormatSignedness(
+ IntendedMatch, S.getDiagnostics().getDiagnosticOptions());
+ if ((IntendedMatch != ArgType::Match) || ShouldNotPrintDirectly)
Hints.push_back(FixItHint::CreateReplacement(SpecRange, os.str()));
if (const CStyleCastExpr *CCast = dyn_cast<CStyleCastExpr>(E)) {
@@ -12756,6 +12775,7 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
switch (Match) {
case ArgType::Match:
case ArgType::MatchPromotion:
+ case ArgType::MatchSignedness:
case ArgType::NoMatchPromotionTypeConfusion:
llvm_unreachable("expected non-matching");
case ArgType::NoMatchPedantic:
@@ -12968,6 +12988,8 @@ bool CheckScanfHandler::HandleScanfSpecifier(
analyze_format_string::ArgType::MatchKind Match =
AT.matchesType(S.Context, Ex->getType());
+ Match =
+ handleFormatSignedness(Match, S.getDiagnostics().getDiagnosticOptions());
bool Pedantic = Match == analyze_format_string::ArgType::NoMatchPedantic;
if (Match == analyze_format_string::ArgType::Match)
return true;
diff --git a/format-strings-signedness.c b/format-strings-signedness.c
new file mode 100644
index 00000000000000..02964def10eef8
--- /dev/null
+++ b/format-strings-signedness.c
@@ -0,0 +1,107 @@
+// RUN: %clang_cc1 -std=c11 -fsyntax-only -verify -Wformat -Wformat-signedness %s
+
+int printf(const char *restrict format, ...);
+int scanf(const char * restrict, ...);
+
+void test_printf_bool(_Bool x)
+{
+ printf("%d", x); // no-warning
+ printf("%u", x); // expected-warning{{format specifies type 'unsigned int' but the argument has type '_Bool'}}
+ printf("%x", x); // expected-warning{{format specifies type 'unsigned int' but the argument has type '_Bool'}}
+}
+
+void test_printf_char(char x)
+{
+ printf("%c", x); // no-warning
+}
+
+void test_printf_unsigned_char(unsigned char x)
+{
+ printf("%c", x); // no-warning
+}
+
+void test_printf_int(int x)
+{
+ printf("%d", x); // no-warning
+ printf("%u", x); // expected-warning{{format specifies type 'unsigned int' but the argument has type 'int'}}
+ printf("%x", x); // expected-warning{{format specifies type 'unsigned int' but the argument has type 'int'}}
+}
+
+void test_printf_unsigned(unsigned x)
+{
+ printf("%d", x); // expected-warning{{format specifies type 'int' but the argument has type 'unsigned int'}}
+ printf("%u", x); // no-warning
+ printf("%x", x); // no-warning
+}
+
+void test_printf_long(long x)
+{
+ printf("%ld", x); // no-warning
+ printf("%lu", x); // expected-warning{{format specifies type 'unsigned long' but the argument has type 'long'}}
+ printf("%lx", x); // expected-warning{{format specifies type 'unsigned long' but the argument has type 'long'}}
+}
+
+void test_printf_unsigned_long(unsigned long x)
+{
+ printf("%ld", x); // expected-warning{{format specifies type 'long' but the argument has type 'unsigned long'}}
+ printf("%lu", x); // no-warning
+ printf("%lx", x); // no-warning
+}
+
+void test_printf_long_long(long long x)
+{
+ printf("%lld", x); // no-warning
+ printf("%llu", x); // expected-warning{{format specifies type 'unsigned long long' but the argument has type 'long long'}}
+ printf("%llx", x); // expected-warning{{format specifies type 'unsigned long long' but the argument has type 'long long'}}
+}
+
+void test_printf_unsigned_long_long(unsigned long long x)
+{
+ printf("%lld", x); // expected-warning{{format specifies type 'long long' but the argument has type 'unsigned long long'}}
+ printf("%llu", x); // no-warning
+ printf("%llx", x); // no-warning
+}
+
+void test_scanf_char(char *y) {
+ scanf("%c", y); // no-warning
+}
+
+void test_scanf_unsigned_char(unsigned char *y) {
+ scanf("%c", y); // no-warning
+}
+
+void test_scanf_int(int *x) {
+ scanf("%d", x); // no-warning
+ scanf("%u", x); // expected-warning{{format specifies type 'unsigned int *' but the argument has type 'int *'}}
+ scanf("%x", x); // expected-warning{{format specifies type 'unsigned int *' but the argument has type 'int *'}}
+}
+
+void test_scanf_unsigned(unsigned *x) {
+ scanf("%d", x); // expected-warning{{format specifies type 'int *' but the argument has type 'unsigned int *'}}
+ scanf("%u", x); // no-warning
+ scanf("%x", x); // no-warning
+}
+
+void test_scanf_long(long *x) {
+ scanf("%ld", x); // no-warning
+ scanf("%lu", x); // expected-warning{{format specifies type 'unsigned long *' but the argument has type 'long *'}}
+ scanf("%lx", x); // expected-warning{{format specifies type 'unsigned long *' but the argument has type 'long *'}}
+}
+
+void test_scanf_unsigned_long(unsigned long *x) {
+ scanf("%ld", x); // expected-warning{{format specifies type 'long *' but the argument has type 'unsigned long *'}}
+ scanf("%lu", x); // no-warning
+ scanf("%lx", x); // no-warning
+}
+
+void test_scanf_longlong(long long *x) {
+ scanf("%lld", x); // no-warning
+ scanf("%llu", x); // expected-warning{{format specifies type 'unsigned long long *' but the argument has type 'long long *'}}
+ scanf("%llx", x); // expected-warning{{format specifies type 'unsigned long long *' but the argument has type 'long long *'}}
+}
+
+void test_scanf_unsigned_longlong(unsigned long long *x) {
+ scanf("%lld", x); // expected-warning{{format specifies type 'long long *' but the argument has type 'unsigned long long *'}}
+ scanf("%llu", x); // no-warning
+ scanf("%llx", x); // no-warning
+}
>From 1862886bd24211e8f62831fe61ba176d0aa0af5c Mon Sep 17 00:00:00 2001
From: Karl-Johan Karlsson <karl-johan.karlsson at ericsson.com>
Date: Tue, 5 Dec 2023 11:17:08 +0100
Subject: [PATCH 2/9] [Sema] Implement support for -Wformat-signedness
Move the testcase to its correct dir.
---
.../test/Sema/format-strings-signedness.c | 0
1 file changed, 0 insertions(+), 0 deletions(-)
rename format-strings-signedness.c => clang/test/Sema/format-strings-signedness.c (100%)
diff --git a/format-strings-signedness.c b/clang/test/Sema/format-strings-signedness.c
similarity index 100%
rename from format-strings-signedness.c
rename to clang/test/Sema/format-strings-signedness.c
>From 59ed89dc73482b8fb027c0eb7fb3006220c75558 Mon Sep 17 00:00:00 2001
From: Karl-Johan Karlsson <karl-johan.karlsson at ericsson.com>
Date: Fri, 8 Dec 2023 07:58:58 +0100
Subject: [PATCH 3/9] [Sema] Implement support for -Wformat-signedness
Move warning option config to DiagnosticSemaKinds.td
---
clang/docs/ReleaseNotes.rst | 3 +
clang/include/clang/AST/FormatString.h | 4 +-
clang/include/clang/Basic/DiagnosticGroups.td | 1 +
.../include/clang/Basic/DiagnosticOptions.def | 1 -
.../clang/Basic/DiagnosticSemaKinds.td | 6 ++
clang/include/clang/Driver/Options.td | 3 -
clang/lib/AST/FormatString.cpp | 20 +++---
clang/lib/Driver/ToolChains/Clang.cpp | 2 -
clang/lib/Sema/SemaChecking.cpp | 70 ++++++++++---------
.../Sema/format-strings-signedness-fixit.c | 65 +++++++++++++++++
clang/test/Sema/format-strings-signedness.c | 21 ++++++
11 files changed, 145 insertions(+), 51 deletions(-)
create mode 100644 clang/test/Sema/format-strings-signedness-fixit.c
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index fd12bb41be47a3..336df3e5e7fc6f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -289,6 +289,9 @@ Improvements to Clang's diagnostics
- Clang now correctly diagnoses no arguments to a variadic macro parameter as a C23/C++20 extension.
Fixes #GH84495.
+- New ``-Wformat-signedness`` diagnostic that warn if the format string requires an
+ unsigned argument and the argument is signed and vice versa.
+
Improvements to Clang's time-trace
----------------------------------
diff --git a/clang/include/clang/AST/FormatString.h b/clang/include/clang/AST/FormatString.h
index 41cd0443ca2ac7..a074dd23e2ad4c 100644
--- a/clang/include/clang/AST/FormatString.h
+++ b/clang/include/clang/AST/FormatString.h
@@ -275,8 +275,6 @@ class ArgType {
/// The conversion specifier and the argument type are compatible. For
/// instance, "%d" and int.
Match = 1,
- /// The conversion specifier and the argument type have different sign
- MatchSignedness,
/// The conversion specifier and the argument type are compatible because of
/// default argument promotions. For instance, "%hhd" and int.
MatchPromotion,
@@ -286,6 +284,8 @@ class ArgType {
/// The conversion specifier and the argument type are disallowed by the C
/// standard, but are in practice harmless. For instance, "%p" and int*.
NoMatchPedantic,
+ /// The conversion specifier and the argument type have different sign.
+ NoMatchSignedness,
/// The conversion specifier and the argument type are compatible, but still
/// seems likely to be an error. For instance, "%hd" and _Bool.
NoMatchTypeConfusion,
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 44035e2fd16f2e..520168f01fd846 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -985,6 +985,7 @@ def FormatSecurity : DiagGroup<"format-security">;
def FormatNonStandard : DiagGroup<"format-non-iso">;
def FormatY2K : DiagGroup<"format-y2k">;
def FormatPedantic : DiagGroup<"format-pedantic">;
+def FormatSignedness : DiagGroup<"format-signedness">;
def FormatTypeConfusion : DiagGroup<"format-type-confusion">;
def FormatOverflowNonKprintf: DiagGroup<"format-overflow-non-kprintf">;
diff --git a/clang/include/clang/Basic/DiagnosticOptions.def b/clang/include/clang/Basic/DiagnosticOptions.def
index a9562e7d8dcb0d..6d0c1b14acc120 100644
--- a/clang/include/clang/Basic/DiagnosticOptions.def
+++ b/clang/include/clang/Basic/DiagnosticOptions.def
@@ -44,7 +44,6 @@ DIAGOPT(Name, Bits, Default)
#endif
SEMANTIC_DIAGOPT(IgnoreWarnings, 1, 0) /// -w
-DIAGOPT(FormatSignedness, 1, 0) /// -Wformat-signedness
DIAGOPT(NoRewriteMacros, 1, 0) /// -Wno-rewrite-macros
DIAGOPT(Pedantic, 1, 0) /// -pedantic
DIAGOPT(PedanticErrors, 1, 0) /// -pedantic-errors
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index fc727cef9cd835..ec7097d16294e7 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9826,6 +9826,9 @@ def warn_format_conversion_argument_type_mismatch : Warning<
def warn_format_conversion_argument_type_mismatch_pedantic : Extension<
warn_format_conversion_argument_type_mismatch.Summary>,
InGroup<FormatPedantic>;
+def warn_format_conversion_argument_type_mismatch_signedness : Warning<
+ warn_format_conversion_argument_type_mismatch.Summary>,
+ InGroup<FormatSignedness>, DefaultIgnore;
def warn_format_conversion_argument_type_mismatch_confusion : Warning<
warn_format_conversion_argument_type_mismatch.Summary>,
InGroup<FormatTypeConfusion>, DefaultIgnore;
@@ -9836,6 +9839,9 @@ def warn_format_argument_needs_cast : Warning<
def warn_format_argument_needs_cast_pedantic : Warning<
warn_format_argument_needs_cast.Summary>,
InGroup<FormatPedantic>, DefaultIgnore;
+def warn_format_argument_needs_cast_signedness : Warning<
+ warn_format_argument_needs_cast.Summary>,
+ InGroup<FormatSignedness>, DefaultIgnore;
def warn_printf_positional_arg_exceeds_data_args : Warning <
"data argument position '%0' exceeds the number of data arguments (%1)">,
InGroup<Format>;
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 1698c6c89e6e98..4a954258ce40b6 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -954,9 +954,6 @@ def Wdeprecated : Flag<["-"], "Wdeprecated">, Group<W_Group>,
HelpText<"Enable warnings for deprecated constructs and define __DEPRECATED">;
def Wno_deprecated : Flag<["-"], "Wno-deprecated">, Group<W_Group>,
Visibility<[ClangOption, CC1Option]>;
-def Wformat_signedness : Flag<["-"], "Wformat-signedness">,
- Flags<[HelpHidden]>, Visibility<[ClangOption, CC1Option]>,
- MarshallingInfoFlag<DiagnosticOpts<"FormatSignedness">>;
def Wl_COMMA : CommaJoined<["-"], "Wl,">, Visibility<[ClangOption, FlangOption]>,
Flags<[LinkerInput, RenderAsInput]>,
HelpText<"Pass the comma separated arguments in <arg> to the linker">,
diff --git a/clang/lib/AST/FormatString.cpp b/clang/lib/AST/FormatString.cpp
index bdd2f046113e7e..da8164bad518ec 100644
--- a/clang/lib/AST/FormatString.cpp
+++ b/clang/lib/AST/FormatString.cpp
@@ -426,7 +426,7 @@ ArgType::matchesType(ASTContext &C, QualType argTy) const {
if (T == C.UnsignedShortTy || T == C.ShortTy)
return NoMatchTypeConfusion;
if (T == C.UnsignedCharTy)
- return MatchSignedness;
+ return NoMatchSignedness;
if (T == C.SignedCharTy)
return Match;
break;
@@ -437,39 +437,39 @@ ArgType::matchesType(ASTContext &C, QualType argTy) const {
if (T == C.UnsignedCharTy)
return Match;
if (T == C.SignedCharTy)
- return MatchSignedness;
+ return NoMatchSignedness;
break;
case BuiltinType::Short:
if (T == C.UnsignedShortTy)
- return MatchSignedness;
+ return NoMatchSignedness;
break;
case BuiltinType::UShort:
if (T == C.ShortTy)
- return MatchSignedness;
+ return NoMatchSignedness;
break;
case BuiltinType::Int:
if (T == C.UnsignedIntTy)
- return MatchSignedness;
+ return NoMatchSignedness;
break;
case BuiltinType::UInt:
if (T == C.IntTy)
- return MatchSignedness;
+ return NoMatchSignedness;
break;
case BuiltinType::Long:
if (T == C.UnsignedLongTy)
- return MatchSignedness;
+ return NoMatchSignedness;
break;
case BuiltinType::ULong:
if (T == C.LongTy)
- return MatchSignedness;
+ return NoMatchSignedness;
break;
case BuiltinType::LongLong:
if (T == C.UnsignedLongLongTy)
- return MatchSignedness;
+ return NoMatchSignedness;
break;
case BuiltinType::ULongLong:
if (T == C.LongLongTy)
- return MatchSignedness;
+ return NoMatchSignedness;
break;
}
// "Partially matched" because of promotions?
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 561ff84b665c51..86a287db72a4eb 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6303,8 +6303,6 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
CmdArgs.push_back("-fdeprecated-macro");
}
- Args.AddLastArg(CmdArgs, options::OPT_Wformat_signedness);
-
// Translate GCC's misnamer '-fasm' arguments to '-fgnu-keywords'.
if (Arg *Asm = Args.getLastArg(options::OPT_fasm, options::OPT_fno_asm)) {
if (Asm->getOption().matches(options::OPT_fasm))
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index df519bfa66afd5..5dca458ade2a59 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -12465,18 +12465,6 @@ isArithmeticArgumentPromotion(Sema &S, const ImplicitCastExpr *ICE) {
S.Context.getFloatingTypeOrder(From, To) < 0;
}
-static clang::analyze_format_string::ArgType::MatchKind
-handleFormatSignedness(clang::analyze_format_string::ArgType::MatchKind Match,
- DiagnosticOptions &DiagOpts) {
- if (Match == clang::analyze_format_string::ArgType::MatchSignedness) {
- if (DiagOpts.FormatSignedness)
- Match = clang::analyze_format_string::ArgType::NoMatch;
- else
- Match = clang::analyze_format_string::ArgType::Match;
- }
- return Match;
-}
-
bool
CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
const char *StartSpecifier,
@@ -12520,8 +12508,6 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
ArgType::MatchKind ImplicitMatch = ArgType::NoMatch;
ArgType::MatchKind Match = AT.matchesType(S.Context, ExprTy);
- Match =
- handleFormatSignedness(Match, S.getDiagnostics().getDiagnosticOptions());
if (Match == ArgType::Match)
return true;
@@ -12545,8 +12531,6 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
ICE->getType() == S.Context.UnsignedIntTy) {
// All further checking is done on the subexpression
ImplicitMatch = AT.matchesType(S.Context, ExprTy);
- ImplicitMatch = handleFormatSignedness(
- ImplicitMatch, S.getDiagnostics().getDiagnosticOptions());
if (ImplicitMatch == ArgType::Match)
return true;
}
@@ -12579,6 +12563,7 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
Match = ArgType::NoMatch;
}
if (ImplicitMatch == ArgType::NoMatchPedantic ||
+ ImplicitMatch == ArgType::NoMatchSignedness ||
ImplicitMatch == ArgType::NoMatchTypeConfusion)
Match = ImplicitMatch;
assert(Match != ArgType::MatchPromotion);
@@ -12667,12 +12652,14 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
switch (Match) {
case ArgType::Match:
case ArgType::MatchPromotion:
- case ArgType::MatchSignedness:
case ArgType::NoMatchPromotionTypeConfusion:
llvm_unreachable("expected non-matching");
case ArgType::NoMatchPedantic:
Diag = diag::warn_format_conversion_argument_type_mismatch_pedantic;
break;
+ case ArgType::NoMatchSignedness:
+ Diag = diag::warn_format_conversion_argument_type_mismatch_signedness;
+ break;
case ArgType::NoMatchTypeConfusion:
Diag = diag::warn_format_conversion_argument_type_mismatch_confusion;
break;
@@ -12704,10 +12691,8 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
CastFix << (S.LangOpts.CPlusPlus ? ">" : ")");
SmallVector<FixItHint,4> Hints;
- ArgType::MatchKind IntendedMatch = AT.matchesType(S.Context, IntendedTy);
- IntendedMatch = handleFormatSignedness(
- IntendedMatch, S.getDiagnostics().getDiagnosticOptions());
- if ((IntendedMatch != ArgType::Match) || ShouldNotPrintDirectly)
+ if (AT.matchesType(S.Context, IntendedTy) != ArgType::Match ||
+ ShouldNotPrintDirectly)
Hints.push_back(FixItHint::CreateReplacement(SpecRange, os.str()));
if (const CStyleCastExpr *CCast = dyn_cast<CStyleCastExpr>(E)) {
@@ -12743,9 +12728,18 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
Name = TypedefTy->getDecl()->getName();
else
Name = CastTyName;
- unsigned Diag = Match == ArgType::NoMatchPedantic
- ? diag::warn_format_argument_needs_cast_pedantic
- : diag::warn_format_argument_needs_cast;
+ unsigned Diag;
+ switch (Match) {
+ default:
+ Diag = diag::warn_format_argument_needs_cast;
+ break;
+ case analyze_format_string::ArgType::NoMatchPedantic:
+ Diag = diag::warn_format_argument_needs_cast_pedantic;
+ break;
+ case analyze_format_string::ArgType::NoMatchSignedness:
+ Diag = diag::warn_format_argument_needs_cast_signedness;
+ break;
+ }
EmitFormatDiagnostic(S.PDiag(Diag) << Name << IntendedTy << IsEnum
<< E->getSourceRange(),
E->getBeginLoc(), /*IsStringLocation=*/false,
@@ -12754,8 +12748,11 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
// In this case, the expression could be printed using a different
// specifier, but we've decided that the specifier is probably correct
// and we should cast instead. Just use the normal warning message.
+ unsigned Diag = Match == ArgType::NoMatchSignedness
+ ? diag::warn_format_conversion_argument_type_mismatch_signedness
+ : diag::warn_format_conversion_argument_type_mismatch;
EmitFormatDiagnostic(
- S.PDiag(diag::warn_format_conversion_argument_type_mismatch)
+ S.PDiag(Diag)
<< AT.getRepresentativeTypeName(S.Context) << ExprTy << IsEnum
<< E->getSourceRange(),
E->getBeginLoc(), /*IsStringLocation*/ false, SpecRange, Hints);
@@ -12775,12 +12772,14 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
switch (Match) {
case ArgType::Match:
case ArgType::MatchPromotion:
- case ArgType::MatchSignedness:
case ArgType::NoMatchPromotionTypeConfusion:
llvm_unreachable("expected non-matching");
case ArgType::NoMatchPedantic:
Diag = diag::warn_format_conversion_argument_type_mismatch_pedantic;
break;
+ case ArgType::NoMatchSignedness:
+ Diag = diag::warn_format_conversion_argument_type_mismatch_signedness;
+ break;
case ArgType::NoMatchTypeConfusion:
Diag = diag::warn_format_conversion_argument_type_mismatch_confusion;
break;
@@ -12988,19 +12987,24 @@ bool CheckScanfHandler::HandleScanfSpecifier(
analyze_format_string::ArgType::MatchKind Match =
AT.matchesType(S.Context, Ex->getType());
- Match =
- handleFormatSignedness(Match, S.getDiagnostics().getDiagnosticOptions());
- bool Pedantic = Match == analyze_format_string::ArgType::NoMatchPedantic;
if (Match == analyze_format_string::ArgType::Match)
return true;
ScanfSpecifier fixedFS = FS;
bool Success = fixedFS.fixType(Ex->getType(), Ex->IgnoreImpCasts()->getType(),
S.getLangOpts(), S.Context);
-
- unsigned Diag =
- Pedantic ? diag::warn_format_conversion_argument_type_mismatch_pedantic
- : diag::warn_format_conversion_argument_type_mismatch;
+ unsigned Diag;
+ switch (Match) {
+ default:
+ Diag = diag::warn_format_conversion_argument_type_mismatch;
+ break;
+ case analyze_format_string::ArgType::NoMatchPedantic:
+ Diag = diag::warn_format_conversion_argument_type_mismatch_pedantic;
+ break;
+ case analyze_format_string::ArgType::NoMatchSignedness:
+ Diag = diag::warn_format_conversion_argument_type_mismatch_signedness;
+ break;
+ }
if (Success) {
// Get the fix string from the fixed format specifier.
diff --git a/clang/test/Sema/format-strings-signedness-fixit.c b/clang/test/Sema/format-strings-signedness-fixit.c
new file mode 100644
index 00000000000000..2ce794472b7527
--- /dev/null
+++ b/clang/test/Sema/format-strings-signedness-fixit.c
@@ -0,0 +1,65 @@
+// RUN: cp %s %t
+// RUN: %clang_cc1 -Wformat -Wformat-signedness -fixit %t
+// RUN: %clang_cc1 -fsyntax-only -Wformat -Wformat-signedness -Werror %t
+// RUN: %clang_cc1 -E -o - %t | FileCheck %s
+
+#include <limits.h>
+
+int printf(const char *restrict format, ...);
+
+enum foo {
+ minus_one = -1,
+ int_val = INT_MAX,
+ unsigned_val = (unsigned)INT_MIN
+};
+
+void test_printf_int(int x)
+{
+ printf("%u", x);
+}
+
+void test_printf_unsigned(unsigned x)
+{
+ printf("%d", x);
+}
+
+void test_printf_long(long x)
+{
+ printf("%lu", x);
+}
+
+void test_printf_unsigned_long(unsigned long x)
+{
+ printf("%ld", x);
+}
+
+void test_printf_long_long(long long x)
+{
+ printf("%llu", x);
+}
+
+void test_printf_unsigned_long_long(unsigned long long x)
+{
+ printf("%lld", x);
+}
+
+void test_printf_enum(enum foo x)
+{
+ printf("%lu", x);
+}
+
+// Validate the fixes.
+// CHECK: void test_printf_int(int x)
+// CHECK: printf("%d", x);
+// CHECK: void test_printf_unsigned(unsigned x)
+// CHECK: printf("%u", x);
+// CHECK: void test_printf_long(long x)
+// CHECK: printf("%ld", x);
+// CHECK: void test_printf_unsigned_long(unsigned long x)
+// CHECK: printf("%lu", x);
+// CHECK: void test_printf_long_long(long long x)
+// CHECK: printf("%lld", x);
+// CHECK: void test_printf_unsigned_long_long(unsigned long long x)
+// CHECK: printf("%llu", x);
+// CHECK: void test_printf_enum(enum foo x)
+// CHECK: printf("%ld", x);
diff --git a/clang/test/Sema/format-strings-signedness.c b/clang/test/Sema/format-strings-signedness.c
index 02964def10eef8..6e819ed715be3a 100644
--- a/clang/test/Sema/format-strings-signedness.c
+++ b/clang/test/Sema/format-strings-signedness.c
@@ -1,8 +1,16 @@
// RUN: %clang_cc1 -std=c11 -fsyntax-only -verify -Wformat -Wformat-signedness %s
+#include <limits.h>
+
int printf(const char *restrict format, ...);
int scanf(const char * restrict, ...);
+enum foo {
+ minus_one = -1,
+ int_val = INT_MAX,
+ unsigned_val = (unsigned)INT_MIN
+};
+
void test_printf_bool(_Bool x)
{
printf("%d", x); // no-warning
@@ -62,6 +70,13 @@ void test_printf_unsigned_long_long(unsigned long long x)
printf("%llx", x); // no-warning
}
+void test_printf_enum(enum foo x)
+{
+ printf("%ld", x); // no-warning
+ printf("%lu", x); // expected-warning{{format specifies type 'unsigned long' but the argument has underlying type 'long'}}
+ printf("%lx", x); // expected-warning{{format specifies type 'unsigned long' but the argument has underlying type 'long'}}
+}
+
void test_scanf_char(char *y) {
scanf("%c", y); // no-warning
}
@@ -105,3 +120,9 @@ void test_scanf_unsigned_longlong(unsigned long long *x) {
scanf("%llu", x); // no-warning
scanf("%llx", x); // no-warning
}
+
+void test_scanf_enum(enum foo *x) {
+ scanf("%ld", x); // no-warning
+ scanf("%lu", x); // expected-warning{{format specifies type 'unsigned long *' but the argument has type 'enum foo *'}}
+ scanf("%lx", x); // expected-warning{{format specifies type 'unsigned long *' but the argument has type 'enum foo *'}}
+}
>From facb9e6dbc97d40270dfc8c44e0b309eec51377d Mon Sep 17 00:00:00 2001
From: Karl-Johan Karlsson <karl-johan.karlsson at ericsson.com>
Date: Fri, 8 Dec 2023 09:07:48 +0100
Subject: [PATCH 4/9] [Sema] Implement support for -Wformat-signedness
Reformat code with clang-format.
---
clang/lib/Sema/SemaChecking.cpp | 48 ++++++++++++++++-----------------
1 file changed, 24 insertions(+), 24 deletions(-)
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 5dca458ade2a59..76509601850816 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -12730,15 +12730,15 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
Name = CastTyName;
unsigned Diag;
switch (Match) {
- default:
- Diag = diag::warn_format_argument_needs_cast;
- break;
- case analyze_format_string::ArgType::NoMatchPedantic:
- Diag = diag::warn_format_argument_needs_cast_pedantic;
- break;
- case analyze_format_string::ArgType::NoMatchSignedness:
- Diag = diag::warn_format_argument_needs_cast_signedness;
- break;
+ default:
+ Diag = diag::warn_format_argument_needs_cast;
+ break;
+ case analyze_format_string::ArgType::NoMatchPedantic:
+ Diag = diag::warn_format_argument_needs_cast_pedantic;
+ break;
+ case analyze_format_string::ArgType::NoMatchSignedness:
+ Diag = diag::warn_format_argument_needs_cast_signedness;
+ break;
}
EmitFormatDiagnostic(S.PDiag(Diag) << Name << IntendedTy << IsEnum
<< E->getSourceRange(),
@@ -12748,13 +12748,13 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
// In this case, the expression could be printed using a different
// specifier, but we've decided that the specifier is probably correct
// and we should cast instead. Just use the normal warning message.
- unsigned Diag = Match == ArgType::NoMatchSignedness
- ? diag::warn_format_conversion_argument_type_mismatch_signedness
- : diag::warn_format_conversion_argument_type_mismatch;
+ unsigned Diag =
+ Match == ArgType::NoMatchSignedness
+ ? diag::warn_format_conversion_argument_type_mismatch_signedness
+ : diag::warn_format_conversion_argument_type_mismatch;
EmitFormatDiagnostic(
- S.PDiag(Diag)
- << AT.getRepresentativeTypeName(S.Context) << ExprTy << IsEnum
- << E->getSourceRange(),
+ S.PDiag(Diag) << AT.getRepresentativeTypeName(S.Context) << ExprTy
+ << IsEnum << E->getSourceRange(),
E->getBeginLoc(), /*IsStringLocation*/ false, SpecRange, Hints);
}
}
@@ -12995,15 +12995,15 @@ bool CheckScanfHandler::HandleScanfSpecifier(
S.getLangOpts(), S.Context);
unsigned Diag;
switch (Match) {
- default:
- Diag = diag::warn_format_conversion_argument_type_mismatch;
- break;
- case analyze_format_string::ArgType::NoMatchPedantic:
- Diag = diag::warn_format_conversion_argument_type_mismatch_pedantic;
- break;
- case analyze_format_string::ArgType::NoMatchSignedness:
- Diag = diag::warn_format_conversion_argument_type_mismatch_signedness;
- break;
+ default:
+ Diag = diag::warn_format_conversion_argument_type_mismatch;
+ break;
+ case analyze_format_string::ArgType::NoMatchPedantic:
+ Diag = diag::warn_format_conversion_argument_type_mismatch_pedantic;
+ break;
+ case analyze_format_string::ArgType::NoMatchSignedness:
+ Diag = diag::warn_format_conversion_argument_type_mismatch_signedness;
+ break;
}
if (Success) {
>From 932ae5736ee96a79ffd5a558ffe714284553a29a Mon Sep 17 00:00:00 2001
From: Karl-Johan Karlsson <karl-johan.karlsson at ericsson.com>
Date: Fri, 8 Dec 2023 09:57:00 +0100
Subject: [PATCH 5/9] [Sema] Implement support for -Wformat-signedness
Fixed and extended the enum tests in format-strings-signedness.c and
format-strings-signedness-fixit.c
---
.../Sema/format-strings-signedness-fixit.c | 55 ++++++++++---
clang/test/Sema/format-strings-signedness.c | 78 ++++++++++++++++---
2 files changed, 111 insertions(+), 22 deletions(-)
diff --git a/clang/test/Sema/format-strings-signedness-fixit.c b/clang/test/Sema/format-strings-signedness-fixit.c
index 2ce794472b7527..b4e6e975657aae 100644
--- a/clang/test/Sema/format-strings-signedness-fixit.c
+++ b/clang/test/Sema/format-strings-signedness-fixit.c
@@ -1,18 +1,12 @@
// RUN: cp %s %t
-// RUN: %clang_cc1 -Wformat -Wformat-signedness -fixit %t
-// RUN: %clang_cc1 -fsyntax-only -Wformat -Wformat-signedness -Werror %t
-// RUN: %clang_cc1 -E -o - %t | FileCheck %s
+// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -Wformat -Wformat-signedness -fixit %t
+// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -Wformat -Wformat-signedness -Werror %t
+// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -E -o - %t | FileCheck %s
#include <limits.h>
int printf(const char *restrict format, ...);
-enum foo {
- minus_one = -1,
- int_val = INT_MAX,
- unsigned_val = (unsigned)INT_MIN
-};
-
void test_printf_int(int x)
{
printf("%u", x);
@@ -43,11 +37,44 @@ void test_printf_unsigned_long_long(unsigned long long x)
printf("%lld", x);
}
-void test_printf_enum(enum foo x)
+enum enum_int {
+ minus_1 = -1
+};
+
+void test_printf_enum_int(enum enum_int x)
+{
+ printf("%u", x);
+}
+
+enum enum_unsigned {
+ zero = 0
+};
+
+void test_printf_enum_unsigned(enum enum_unsigned x)
+{
+ printf("%d", x);
+}
+
+enum enum_long {
+ minus_one = -1,
+ int_val = INT_MAX,
+ unsigned_val = (unsigned)INT_MIN
+};
+
+void test_printf_enum_long(enum enum_long x)
{
printf("%lu", x);
}
+enum enum_unsigned_long {
+ uint_max_plus = (unsigned long)UINT_MAX+1,
+};
+
+void test_printf_enum_unsigned_long(enum enum_unsigned_long x)
+{
+ printf("%ld", x);
+}
+
// Validate the fixes.
// CHECK: void test_printf_int(int x)
// CHECK: printf("%d", x);
@@ -61,5 +88,11 @@ void test_printf_enum(enum foo x)
// CHECK: printf("%lld", x);
// CHECK: void test_printf_unsigned_long_long(unsigned long long x)
// CHECK: printf("%llu", x);
-// CHECK: void test_printf_enum(enum foo x)
+// CHECK: void test_printf_enum_int(enum enum_int x)
+// CHECK: printf("%d", x);
+// CHECK: void test_printf_enum_unsigned(enum enum_unsigned x)
+// CHECK: printf("%u", x);
+// CHECK: void test_printf_enum_long(enum enum_long x)
// CHECK: printf("%ld", x);
+// CHECK: void test_printf_enum_unsigned_long(enum enum_unsigned_long x)
+// CHECK: printf("%lu", x);
diff --git a/clang/test/Sema/format-strings-signedness.c b/clang/test/Sema/format-strings-signedness.c
index 6e819ed715be3a..fe994fa55f4e61 100644
--- a/clang/test/Sema/format-strings-signedness.c
+++ b/clang/test/Sema/format-strings-signedness.c
@@ -1,16 +1,11 @@
-// RUN: %clang_cc1 -std=c11 -fsyntax-only -verify -Wformat -Wformat-signedness %s
+// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -std=c11 -fsyntax-only -verify -Wformat -Wformat-signedness %s
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -std=c11 -fsyntax-only -verify -Wformat -Wformat-signedness %s
#include <limits.h>
int printf(const char *restrict format, ...);
int scanf(const char * restrict, ...);
-enum foo {
- minus_one = -1,
- int_val = INT_MAX,
- unsigned_val = (unsigned)INT_MIN
-};
-
void test_printf_bool(_Bool x)
{
printf("%d", x); // no-warning
@@ -70,13 +65,54 @@ void test_printf_unsigned_long_long(unsigned long long x)
printf("%llx", x); // no-warning
}
-void test_printf_enum(enum foo x)
+enum enum_int {
+ minus_1 = -1
+};
+
+void test_printf_enum_int(enum enum_int x)
+{
+ printf("%d", x); // no-warning
+ printf("%u", x); // expected-warning{{format specifies type 'unsigned int' but the argument has underlying type 'int'}}
+ printf("%x", x); // expected-warning{{format specifies type 'unsigned int' but the argument has underlying type 'int'}}
+}
+
+#ifndef _WIN32 // Disabled due to enums have different underlying type on _WIN32
+enum enum_unsigned {
+ zero = 0
+};
+
+void test_printf_enum_unsigned(enum enum_unsigned x)
+{
+ printf("%d", x); // expected-warning{{format specifies type 'int' but the argument has underlying type 'unsigned int'}}
+ printf("%u", x); // no-warning
+ printf("%x", x); // no-warning
+}
+
+enum enum_long {
+ minus_one = -1,
+ int_val = INT_MAX,
+ unsigned_val = (unsigned)INT_MIN
+};
+
+void test_printf_enum_long(enum enum_long x)
{
printf("%ld", x); // no-warning
printf("%lu", x); // expected-warning{{format specifies type 'unsigned long' but the argument has underlying type 'long'}}
printf("%lx", x); // expected-warning{{format specifies type 'unsigned long' but the argument has underlying type 'long'}}
}
+enum enum_unsigned_long {
+ uint_max_plus = (unsigned long)UINT_MAX+1,
+};
+
+void test_printf_enum_unsigned_long(enum enum_unsigned_long x)
+{
+ printf("%ld", x); // expected-warning{{format specifies type 'long' but the argument has underlying type 'unsigned long'}}
+ printf("%lu", x); // no-warning
+ printf("%lx", x); // no-warning
+}
+#endif
+
void test_scanf_char(char *y) {
scanf("%c", y); // no-warning
}
@@ -121,8 +157,28 @@ void test_scanf_unsigned_longlong(unsigned long long *x) {
scanf("%llx", x); // no-warning
}
-void test_scanf_enum(enum foo *x) {
+void test_scanf_enum_int(enum enum_int *x) {
+ scanf("%d", x); // no-warning
+ scanf("%u", x); // expected-warning{{format specifies type 'unsigned int *' but the argument has type 'enum enum_int *'}}
+ scanf("%x", x); // expected-warning{{format specifies type 'unsigned int *' but the argument has type 'enum enum_int *'}}
+}
+
+#ifndef _WIN32 // Disabled due to enums have different underlying type on _WIN32
+void test_scanf_enum_unsigned(enum enum_unsigned *x) {
+ scanf("%d", x); // expected-warning{{format specifies type 'int *' but the argument has type 'enum enum_unsigned *'}}
+ scanf("%u", x); // no-warning
+ scanf("%x", x); // no-warning
+}
+
+void test_scanf_enum_long(enum enum_long *x) {
scanf("%ld", x); // no-warning
- scanf("%lu", x); // expected-warning{{format specifies type 'unsigned long *' but the argument has type 'enum foo *'}}
- scanf("%lx", x); // expected-warning{{format specifies type 'unsigned long *' but the argument has type 'enum foo *'}}
+ scanf("%lu", x); // expected-warning{{format specifies type 'unsigned long *' but the argument has type 'enum enum_long *'}}
+ scanf("%lx", x); // expected-warning{{format specifies type 'unsigned long *' but the argument has type 'enum enum_long *'}}
+}
+
+void test_scanf_enum_unsigned_long(enum enum_unsigned_long *x) {
+ scanf("%ld", x); // expected-warning{{format specifies type 'long *' but the argument has type 'enum enum_unsigned_long *'}}
+ scanf("%lu", x); // no-warning
+ scanf("%lx", x); // no-warning
}
+#endif
>From 7df121d34f7dc54fb9f2e72ec2edf797a9ef9538 Mon Sep 17 00:00:00 2001
From: Karl-Johan Karlsson <karl-johan.karlsson at ericsson.com>
Date: Tue, 12 Dec 2023 08:09:43 +0100
Subject: [PATCH 6/9] [Sema] Implement support for -Wformat-signedness
Add more GCC compatibility.
---
.../clang/Basic/DiagnosticSemaKinds.td | 3 -
clang/lib/Sema/SemaChecking.cpp | 74 +++++++++----------
clang/test/Sema/format-strings-signedness.c | 12 +++
3 files changed, 47 insertions(+), 42 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index ec7097d16294e7..afa469a98e1f2d 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9839,9 +9839,6 @@ def warn_format_argument_needs_cast : Warning<
def warn_format_argument_needs_cast_pedantic : Warning<
warn_format_argument_needs_cast.Summary>,
InGroup<FormatPedantic>, DefaultIgnore;
-def warn_format_argument_needs_cast_signedness : Warning<
- warn_format_argument_needs_cast.Summary>,
- InGroup<FormatSignedness>, DefaultIgnore;
def warn_printf_positional_arg_exceeds_data_args : Warning <
"data argument position '%0' exceeds the number of data arguments (%1)">,
InGroup<Format>;
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 76509601850816..36c01c50b118ec 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -12465,6 +12465,20 @@ isArithmeticArgumentPromotion(Sema &S, const ImplicitCastExpr *ICE) {
S.Context.getFloatingTypeOrder(From, To) < 0;
}
+static analyze_format_string::ArgType::MatchKind
+handleFormatSignedness(analyze_format_string::ArgType::MatchKind Match,
+ DiagnosticsEngine &Diags, SourceLocation Loc) {
+ if (Match == analyze_format_string::ArgType::NoMatchSignedness) {
+ if (!Diags.isIgnored(
+ diag::warn_format_conversion_argument_type_mismatch_signedness,
+ Loc))
+ Match = analyze_format_string::ArgType::NoMatch;
+ else
+ Match = analyze_format_string::ArgType::Match;
+ }
+ return Match;
+}
+
bool
CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
const char *StartSpecifier,
@@ -12508,6 +12522,7 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
ArgType::MatchKind ImplicitMatch = ArgType::NoMatch;
ArgType::MatchKind Match = AT.matchesType(S.Context, ExprTy);
+ Match = handleFormatSignedness(Match, S.getDiagnostics(), E->getExprLoc());
if (Match == ArgType::Match)
return true;
@@ -12531,6 +12546,8 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
ICE->getType() == S.Context.UnsignedIntTy) {
// All further checking is done on the subexpression
ImplicitMatch = AT.matchesType(S.Context, ExprTy);
+ ImplicitMatch = handleFormatSignedness(
+ ImplicitMatch, S.getDiagnostics(), E->getExprLoc());
if (ImplicitMatch == ArgType::Match)
return true;
}
@@ -12563,7 +12580,6 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
Match = ArgType::NoMatch;
}
if (ImplicitMatch == ArgType::NoMatchPedantic ||
- ImplicitMatch == ArgType::NoMatchSignedness ||
ImplicitMatch == ArgType::NoMatchTypeConfusion)
Match = ImplicitMatch;
assert(Match != ArgType::MatchPromotion);
@@ -12653,13 +12669,11 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
case ArgType::Match:
case ArgType::MatchPromotion:
case ArgType::NoMatchPromotionTypeConfusion:
+ case ArgType::NoMatchSignedness:
llvm_unreachable("expected non-matching");
case ArgType::NoMatchPedantic:
Diag = diag::warn_format_conversion_argument_type_mismatch_pedantic;
break;
- case ArgType::NoMatchSignedness:
- Diag = diag::warn_format_conversion_argument_type_mismatch_signedness;
- break;
case ArgType::NoMatchTypeConfusion:
Diag = diag::warn_format_conversion_argument_type_mismatch_confusion;
break;
@@ -12691,8 +12705,10 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
CastFix << (S.LangOpts.CPlusPlus ? ">" : ")");
SmallVector<FixItHint,4> Hints;
- if (AT.matchesType(S.Context, IntendedTy) != ArgType::Match ||
- ShouldNotPrintDirectly)
+ ArgType::MatchKind IntendedMatch = AT.matchesType(S.Context, IntendedTy);
+ IntendedMatch = handleFormatSignedness(IntendedMatch, S.getDiagnostics(),
+ E->getExprLoc());
+ if ((IntendedMatch != ArgType::Match) || ShouldNotPrintDirectly)
Hints.push_back(FixItHint::CreateReplacement(SpecRange, os.str()));
if (const CStyleCastExpr *CCast = dyn_cast<CStyleCastExpr>(E)) {
@@ -12728,18 +12744,9 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
Name = TypedefTy->getDecl()->getName();
else
Name = CastTyName;
- unsigned Diag;
- switch (Match) {
- default:
- Diag = diag::warn_format_argument_needs_cast;
- break;
- case analyze_format_string::ArgType::NoMatchPedantic:
- Diag = diag::warn_format_argument_needs_cast_pedantic;
- break;
- case analyze_format_string::ArgType::NoMatchSignedness:
- Diag = diag::warn_format_argument_needs_cast_signedness;
- break;
- }
+ unsigned Diag = Match == ArgType::NoMatchPedantic
+ ? diag::warn_format_argument_needs_cast_pedantic
+ : diag::warn_format_argument_needs_cast;
EmitFormatDiagnostic(S.PDiag(Diag) << Name << IntendedTy << IsEnum
<< E->getSourceRange(),
E->getBeginLoc(), /*IsStringLocation=*/false,
@@ -12748,13 +12755,10 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
// In this case, the expression could be printed using a different
// specifier, but we've decided that the specifier is probably correct
// and we should cast instead. Just use the normal warning message.
- unsigned Diag =
- Match == ArgType::NoMatchSignedness
- ? diag::warn_format_conversion_argument_type_mismatch_signedness
- : diag::warn_format_conversion_argument_type_mismatch;
EmitFormatDiagnostic(
- S.PDiag(Diag) << AT.getRepresentativeTypeName(S.Context) << ExprTy
- << IsEnum << E->getSourceRange(),
+ S.PDiag(diag::warn_format_conversion_argument_type_mismatch)
+ << AT.getRepresentativeTypeName(S.Context) << ExprTy << IsEnum
+ << E->getSourceRange(),
E->getBeginLoc(), /*IsStringLocation*/ false, SpecRange, Hints);
}
}
@@ -12773,13 +12777,11 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
case ArgType::Match:
case ArgType::MatchPromotion:
case ArgType::NoMatchPromotionTypeConfusion:
+ case ArgType::NoMatchSignedness:
llvm_unreachable("expected non-matching");
case ArgType::NoMatchPedantic:
Diag = diag::warn_format_conversion_argument_type_mismatch_pedantic;
break;
- case ArgType::NoMatchSignedness:
- Diag = diag::warn_format_conversion_argument_type_mismatch_signedness;
- break;
case ArgType::NoMatchTypeConfusion:
Diag = diag::warn_format_conversion_argument_type_mismatch_confusion;
break;
@@ -12987,24 +12989,18 @@ bool CheckScanfHandler::HandleScanfSpecifier(
analyze_format_string::ArgType::MatchKind Match =
AT.matchesType(S.Context, Ex->getType());
+ Match = handleFormatSignedness(Match, S.getDiagnostics(), Ex->getExprLoc());
+ bool Pedantic = Match == analyze_format_string::ArgType::NoMatchPedantic;
if (Match == analyze_format_string::ArgType::Match)
return true;
ScanfSpecifier fixedFS = FS;
bool Success = fixedFS.fixType(Ex->getType(), Ex->IgnoreImpCasts()->getType(),
S.getLangOpts(), S.Context);
- unsigned Diag;
- switch (Match) {
- default:
- Diag = diag::warn_format_conversion_argument_type_mismatch;
- break;
- case analyze_format_string::ArgType::NoMatchPedantic:
- Diag = diag::warn_format_conversion_argument_type_mismatch_pedantic;
- break;
- case analyze_format_string::ArgType::NoMatchSignedness:
- Diag = diag::warn_format_conversion_argument_type_mismatch_signedness;
- break;
- }
+
+ unsigned Diag =
+ Pedantic ? diag::warn_format_conversion_argument_type_mismatch_pedantic
+ : diag::warn_format_conversion_argument_type_mismatch;
if (Success) {
// Get the fix string from the fixed format specifier.
diff --git a/clang/test/Sema/format-strings-signedness.c b/clang/test/Sema/format-strings-signedness.c
index fe994fa55f4e61..1646dee5a08fe8 100644
--- a/clang/test/Sema/format-strings-signedness.c
+++ b/clang/test/Sema/format-strings-signedness.c
@@ -1,6 +1,10 @@
// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -std=c11 -fsyntax-only -verify -Wformat -Wformat-signedness %s
// RUN: %clang_cc1 -triple=x86_64-pc-win32 -std=c11 -fsyntax-only -verify -Wformat -Wformat-signedness %s
+// Verify that -Wformat-signedness warnings are not reported with only -Wformat
+// (gcc compat).
+// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -std=c11 -fsyntax-only -Wformat -Werror %s
+
#include <limits.h>
int printf(const char *restrict format, ...);
@@ -182,3 +186,11 @@ void test_scanf_enum_unsigned_long(enum enum_unsigned_long *x) {
scanf("%lx", x); // no-warning
}
#endif
+
+// Verify that we can suppress a -Wformat-signedness warning by ignoring
+// -Wformat (gcc compat).
+void test_suppress(int x)
+{
+#pragma GCC diagnostic ignored "-Wformat"
+ printf("%u", x);
+}
>From e3f0f4afb6b0fb72f54b9c80eb3f147a292b1253 Mon Sep 17 00:00:00 2001
From: Karl-Johan Karlsson <karl-johan.karlsson at ericsson.com>
Date: Mon, 18 Dec 2023 20:59:36 +0100
Subject: [PATCH 7/9] [Sema] Implement support for -Wformat-signedness
Fixed bug and extended the testcase to cover this.
---
clang/lib/Sema/SemaChecking.cpp | 8 ++++++++
clang/test/Sema/format-strings-signedness.c | 21 +++++++++++++++++++--
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 36c01c50b118ec..d7b303d24c92c3 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -12522,6 +12522,8 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
ArgType::MatchKind ImplicitMatch = ArgType::NoMatch;
ArgType::MatchKind Match = AT.matchesType(S.Context, ExprTy);
+ ArgType::MatchKind OrigMatch = Match;
+
Match = handleFormatSignedness(Match, S.getDiagnostics(), E->getExprLoc());
if (Match == ArgType::Match)
return true;
@@ -12546,6 +12548,12 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
ICE->getType() == S.Context.UnsignedIntTy) {
// All further checking is done on the subexpression
ImplicitMatch = AT.matchesType(S.Context, ExprTy);
+ if (OrigMatch == ArgType::NoMatchSignedness &&
+ ImplicitMatch != ArgType::NoMatchSignedness)
+ // If the original match was a signedness match this match on the
+ // implicit cast type also need to be signedness match otherwise we
+ // might introduce new unexpected warnings from -Wformat-signedness.
+ return true;
ImplicitMatch = handleFormatSignedness(
ImplicitMatch, S.getDiagnostics(), E->getExprLoc());
if (ImplicitMatch == ArgType::Match)
diff --git a/clang/test/Sema/format-strings-signedness.c b/clang/test/Sema/format-strings-signedness.c
index 1646dee5a08fe8..47e5acbc919556 100644
--- a/clang/test/Sema/format-strings-signedness.c
+++ b/clang/test/Sema/format-strings-signedness.c
@@ -13,8 +13,8 @@ int scanf(const char * restrict, ...);
void test_printf_bool(_Bool x)
{
printf("%d", x); // no-warning
- printf("%u", x); // expected-warning{{format specifies type 'unsigned int' but the argument has type '_Bool'}}
- printf("%x", x); // expected-warning{{format specifies type 'unsigned int' but the argument has type '_Bool'}}
+ printf("%u", x); // no-warning
+ printf("%x", x); // no-warning
}
void test_printf_char(char x)
@@ -187,6 +187,23 @@ void test_scanf_enum_unsigned_long(enum enum_unsigned_long *x) {
}
#endif
+// Verify that we get no warnings from <inttypes.h>
+
+typedef short int int16_t;
+typedef unsigned short int uint16_t;
+
+void test_printf_priX16(int16_t x) {
+ printf("PRId16: %" "d" /*PRId16*/ "\n", x); // no-warning
+ printf("PRIi16: %" "i" /*PRIi16*/ "\n", x); // no-warning
+}
+
+void test_printf_unsigned_priX16(uint16_t x) {
+ printf("PRIo16: %" "o" /*PRIo16*/ "\n", x); // no-warning
+ printf("PRIu16: %" "u" /*PRIu16*/ "\n", x); // no-warning
+ printf("PRIx16: %" "x" /*PRIx16*/ "\n", x); // no-warning
+ printf("PRIX16: %" "X" /*PRIX16*/ "\n", x); // no-warning
+}
+
// Verify that we can suppress a -Wformat-signedness warning by ignoring
// -Wformat (gcc compat).
void test_suppress(int x)
>From 99c18f3751fe175320b5805b8e788c30581e3765 Mon Sep 17 00:00:00 2001
From: Karl-Johan Karlsson <karl-johan.karlsson at ericsson.com>
Date: Fri, 22 Mar 2024 15:16:40 +0100
Subject: [PATCH 8/9] [Sema] Implement support for -Wformat-signedness
Misc fixes.
---
clang/lib/Sema/SemaChecking.cpp | 11 +++++------
clang/test/Sema/format-strings-signedness.c | 4 ++++
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index d7b303d24c92c3..6836b562ae4d72 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -12469,12 +12469,11 @@ static analyze_format_string::ArgType::MatchKind
handleFormatSignedness(analyze_format_string::ArgType::MatchKind Match,
DiagnosticsEngine &Diags, SourceLocation Loc) {
if (Match == analyze_format_string::ArgType::NoMatchSignedness) {
- if (!Diags.isIgnored(
- diag::warn_format_conversion_argument_type_mismatch_signedness,
- Loc))
- Match = analyze_format_string::ArgType::NoMatch;
- else
- Match = analyze_format_string::ArgType::Match;
+ Match =
+ Diags.isIgnored(
+ diag::warn_format_conversion_argument_type_mismatch_signedness, Loc)
+ ? analyze_format_string::ArgType::Match
+ : analyze_format_string::ArgType::NoMatch;
}
return Match;
}
diff --git a/clang/test/Sema/format-strings-signedness.c b/clang/test/Sema/format-strings-signedness.c
index 47e5acbc919556..d766caa3cb97d8 100644
--- a/clang/test/Sema/format-strings-signedness.c
+++ b/clang/test/Sema/format-strings-signedness.c
@@ -1,6 +1,10 @@
// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -std=c11 -fsyntax-only -verify -Wformat -Wformat-signedness %s
// RUN: %clang_cc1 -triple=x86_64-pc-win32 -std=c11 -fsyntax-only -verify -Wformat -Wformat-signedness %s
+// Verify that -Wformat-signedness alone (without -Wformat) trigger the warnings. Note this is not compatible behavior with gcc.
+// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -std=c11 -fsyntax-only -verify -Wformat-signedness %s
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -std=c11 -fsyntax-only -verify -Wformat-signedness %s
+
// Verify that -Wformat-signedness warnings are not reported with only -Wformat
// (gcc compat).
// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -std=c11 -fsyntax-only -Wformat -Werror %s
>From fb2848c56e3f2b71883ca5f7db6e17834ad55ac2 Mon Sep 17 00:00:00 2001
From: Karl-Johan Karlsson <karl-johan.karlsson at ericsson.com>
Date: Tue, 26 Mar 2024 09:08:39 +0100
Subject: [PATCH 9/9] [Sema] Implement support for -Wformat-signedness
Updated testcase.
---
clang/test/Sema/format-strings-signedness.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/clang/test/Sema/format-strings-signedness.c b/clang/test/Sema/format-strings-signedness.c
index d766caa3cb97d8..b5c9bbc507eff6 100644
--- a/clang/test/Sema/format-strings-signedness.c
+++ b/clang/test/Sema/format-strings-signedness.c
@@ -1,7 +1,9 @@
// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -std=c11 -fsyntax-only -verify -Wformat -Wformat-signedness %s
// RUN: %clang_cc1 -triple=x86_64-pc-win32 -std=c11 -fsyntax-only -verify -Wformat -Wformat-signedness %s
-// Verify that -Wformat-signedness alone (without -Wformat) trigger the warnings. Note this is not compatible behavior with gcc.
+// Verify that -Wformat-signedness alone (without -Wformat) trigger the
+// warnings. Note in gcc this will not trigger the signedness warnings as
+// -Wformat is default off in gcc.
// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -std=c11 -fsyntax-only -verify -Wformat-signedness %s
// RUN: %clang_cc1 -triple=x86_64-pc-win32 -std=c11 -fsyntax-only -verify -Wformat-signedness %s
@@ -9,6 +11,10 @@
// (gcc compat).
// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -std=c11 -fsyntax-only -Wformat -Werror %s
+// Verify that -Wformat-signedness with -Wno-format are not reported (gcc compat).
+// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -std=c11 -fsyntax-only -Wformat-signedness -Wno-format -Werror %s
+// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -std=c11 -fsyntax-only -Wno-format -Wformat-signedness -Werror %s
+
#include <limits.h>
int printf(const char *restrict format, ...);
More information about the llvm-commits
mailing list