[clang] [llvm] [Sema] Implement support for -Wformat-signedness (PR #74440)

Karl-Johan Karlsson via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 8 00:13:07 PST 2023


https://github.com/karka228 updated https://github.com/llvm/llvm-project/pull/74440

>From a80bf9d03f19d48c0aca4af7758dc49516da8825 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/4] [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 5c4ad9baaef60..c267a32be4d6f 100644
--- a/clang/include/clang/AST/FormatString.h
+++ b/clang/include/clang/AST/FormatString.h
@@ -264,6 +264,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 6d0c1b14acc12..a9562e7d8dcb0 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 1d04e4f6e7e6d..04fdf9a7eb92d 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -918,6 +918,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 e0c9e18cfe3a2..670cde017d3ac 100644
--- a/clang/lib/AST/FormatString.cpp
+++ b/clang/lib/AST/FormatString.cpp
@@ -409,7 +409,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;
@@ -419,44 +419,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 f02f7c841b91f..6a6a40293d7d3 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6054,6 +6054,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 07ced5ffc3407..ea8d5a5d6b913 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -11815,6 +11815,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,
@@ -11858,6 +11870,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;
 
@@ -11881,6 +11895,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;
       }
@@ -12001,6 +12017,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:
@@ -12037,8 +12054,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)) {
@@ -12106,6 +12125,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:
@@ -12318,6 +12338,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 0000000000000..02964def10eef
--- /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 1f5fe21d5851e268dfedf909f5ec2832b741a514 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/4] [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 3571a2242d88e4c5b1b6817aa4f5701cf5146c77 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/4] [Sema] Implement support for -Wformat-signedness

Move warning option config to DiagnosticSemaKinds.td
---
 clang/docs/ReleaseNotes.rst                   |  2 +
 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, 144 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 828dd10e3d6db..a9ba5dbfcc512 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -503,6 +503,8 @@ Improvements to Clang's diagnostics
        48 | static_assert(1 << 4 == 15);
           |               ~~~~~~~^~~~~
 
+- 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 c267a32be4d6f..33e9f60fd07b3 100644
--- a/clang/include/clang/AST/FormatString.h
+++ b/clang/include/clang/AST/FormatString.h
@@ -264,8 +264,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,
@@ -275,6 +273,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 ff028bbbf7426..03e6854f32a31 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -968,6 +968,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 a9562e7d8dcb0..6d0c1b14acc12 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 6dfb2d7195203..038efb868b600 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9714,6 +9714,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;
@@ -9724,6 +9727,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 04fdf9a7eb92d..1d04e4f6e7e6d 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -918,9 +918,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 670cde017d3ac..aba205b56fd9a 100644
--- a/clang/lib/AST/FormatString.cpp
+++ b/clang/lib/AST/FormatString.cpp
@@ -422,7 +422,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;
@@ -433,39 +433,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 6a6a40293d7d3..f02f7c841b91f 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6054,8 +6054,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 ea8d5a5d6b913..7354fa8529d09 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -11815,18 +11815,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,
@@ -11870,8 +11858,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;
 
@@ -11895,8 +11881,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;
       }
@@ -11929,6 +11913,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);
@@ -12017,12 +12002,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;
@@ -12054,10 +12041,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)) {
@@ -12093,9 +12078,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,
@@ -12104,8 +12098,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);
@@ -12125,12 +12122,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;
@@ -12338,19 +12337,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 0000000000000..2ce794472b752
--- /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 02964def10eef..6e819ed715be3 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 597adefb434306408250a533e8b2941fac0eb466 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/4] [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 7354fa8529d09..848011ef730a8 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -12080,15 +12080,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(),
@@ -12098,13 +12098,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);
       }
     }
@@ -12345,15 +12345,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) {



More information about the cfe-commits mailing list