Skip to content

Commit 4cdc338

Browse files
DeanSturtevant1DeanSturtevant1
authored
Fix Clang bug that -Wformat-signedness is not reported properly. (#150962)
The goal is to correctly identify diagnostics that are emitted by virtue of -Wformat-signedness. Before this change, diagnostic messages triggered by -Wformat-signedness might look like: format specifies type 'unsigned int' but the argument has type 'int' [-Wformat] signedness of format specifier 'u' is incompatible with 'c' [-Wformat] With this change: format specifies type 'unsigned int' but the argument has type 'int', which differs in signedness [-Wformat-signedness] signedness of format specifier 'u' is incompatible with 'c' [-Wformat-signedness] Fix: - handleFormatSignedness can now return NoMatchSignedness. Callers handle this. - warn_format_conversion_argument_type extends the message it used to emit by a string that mentions "signedness". - warn_format_cmp_specifier_sign_mismatch is now correctly categorized as a diagnostic controlled by -Wformat-signedness.
1 parent 119ff40 commit 4cdc338

File tree

4 files changed

+72
-53
lines changed

4 files changed

+72
-53
lines changed

‎clang/docs/ReleaseNotes.rst

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,17 @@ Improvements to Clang's diagnostics
155155
- Fixed fix-it hint for fold expressions. Clang now correctly places the suggested right
156156
parenthesis when diagnosing malformed fold expressions. (#GH151787)
157157

158+
- Fixed an issue where emitted format-signedness diagnostics were not associated with an appropriate
159+
diagnostic id. Besides being incorrect from an API standpoint, this was user visible, e.g.:
160+
"format specifies type 'unsigned int' but the argument has type 'int' [-Wformat]"
161+
"signedness of format specifier 'u' is incompatible with 'c' [-Wformat]"
162+
This was misleading, because even though -Wformat is required in order to emit the diagnostics,
163+
the warning flag the user needs to concerned with here is -Wformat-signedness, which is also
164+
required and is not enabled by default. With the change you'll now see:
165+
"format specifies type 'unsigned int' but the argument has type 'int', which differs in signedness [-Wformat-signedness]"
166+
"signedness of format specifier 'u' is incompatible with 'c' [-Wformat-signedness]"
167+
and the API-visible diagnostic id will be appropriate.
168+
158169
Improvements to Clang's time-trace
159170
----------------------------------
160171

‎clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10423,9 +10423,10 @@ def warn_format_conversion_argument_type_mismatch : Warning<
1042310423
def warn_format_conversion_argument_type_mismatch_pedantic : Extension<
1042410424
warn_format_conversion_argument_type_mismatch.Summary>,
1042510425
InGroup<FormatPedantic>;
10426-
def warn_format_conversion_argument_type_mismatch_signedness : Warning<
10427-
warn_format_conversion_argument_type_mismatch.Summary>,
10428-
InGroup<FormatSignedness>, DefaultIgnore;
10426+
def warn_format_conversion_argument_type_mismatch_signedness: Warning<
10427+
"format specifies type %0 but the argument has %select{type|underlying "
10428+
"type}2 %1, which differs in signedness" >
10429+
, InGroup<FormatSignedness>, DefaultIgnore;
1042910430
def warn_format_conversion_argument_type_mismatch_confusion : Warning<
1043010431
warn_format_conversion_argument_type_mismatch.Summary>,
1043110432
InGroup<FormatTypeConfusion>, DefaultIgnore;
@@ -10537,8 +10538,10 @@ def warn_format_cmp_sensitivity_mismatch : Warning<
1053710538
"it should be %select{unspecified|private|public|sensitive}1">, InGroup<Format>;
1053810539
def warn_format_cmp_specifier_mismatch : Warning<
1053910540
"format specifier '%0' is incompatible with '%1'">, InGroup<Format>;
10540-
def warn_format_cmp_specifier_sign_mismatch : Warning<
10541-
"signedness of format specifier '%0' is incompatible with '%1'">, InGroup<Format>;
10541+
def warn_format_cmp_specifier_sign_mismatch
10542+
: Warning<"signedness of format specifier '%0' is incompatible with '%1'">,
10543+
InGroup<FormatSignedness>,
10544+
DefaultIgnore;
1054210545
def warn_format_cmp_specifier_mismatch_pedantic : Extension<
1054310546
warn_format_cmp_specifier_sign_mismatch.Summary>, InGroup<FormatPedantic>;
1054410547
def note_format_cmp_with : Note<

‎clang/lib/Sema/SemaChecking.cpp

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
#include "clang/AST/UnresolvedSet.h"
4242
#include "clang/Basic/AddressSpaces.h"
4343
#include "clang/Basic/Diagnostic.h"
44+
#include "clang/Basic/DiagnosticSema.h"
4445
#include "clang/Basic/IdentifierTable.h"
4546
#include "clang/Basic/LLVM.h"
4647
#include "clang/Basic/LangOptions.h"
@@ -7656,17 +7657,13 @@ bool EquatableFormatArgument::VerifyCompatible(
76567657
break;
76577658

76587659
case MK::NoMatchSignedness:
7659-
if (!S.getDiagnostics().isIgnored(
7660-
diag::warn_format_conversion_argument_type_mismatch_signedness,
7661-
ElementLoc)) {
7662-
EmitDiagnostic(S,
7663-
S.PDiag(diag::warn_format_cmp_specifier_sign_mismatch)
7664-
<< buildFormatSpecifier()
7665-
<< Other.buildFormatSpecifier(),
7666-
FmtExpr, InFunctionCall);
7667-
HadError = S.Diag(Other.ElementLoc, diag::note_format_cmp_with)
7668-
<< 0 << Other.Range;
7669-
}
7660+
EmitDiagnostic(S,
7661+
S.PDiag(diag::warn_format_cmp_specifier_sign_mismatch)
7662+
<< buildFormatSpecifier()
7663+
<< Other.buildFormatSpecifier(),
7664+
FmtExpr, InFunctionCall);
7665+
HadError = S.Diag(Other.ElementLoc, diag::note_format_cmp_with)
7666+
<< 0 << Other.Range;
76707667
break;
76717668
}
76727669
return !HadError;
@@ -8203,11 +8200,14 @@ static analyze_format_string::ArgType::MatchKind
82038200
handleFormatSignedness(analyze_format_string::ArgType::MatchKind Match,
82048201
DiagnosticsEngine &Diags, SourceLocation Loc) {
82058202
if (Match == analyze_format_string::ArgType::NoMatchSignedness) {
8206-
Match =
8203+
if (Diags.isIgnored(
8204+
diag::warn_format_conversion_argument_type_mismatch_signedness,
8205+
Loc) ||
82078206
Diags.isIgnored(
8208-
diag::warn_format_conversion_argument_type_mismatch_signedness, Loc)
8209-
? analyze_format_string::ArgType::Match
8210-
: analyze_format_string::ArgType::NoMatch;
8207+
// Arbitrary -Wformat diagnostic to detect -Wno-format:
8208+
diag::warn_format_conversion_argument_type_mismatch, Loc)) {
8209+
return analyze_format_string::ArgType::Match;
8210+
}
82118211
}
82128212
return Match;
82138213
}
@@ -8424,8 +8424,10 @@ CheckPrintfHandler::checkFormatExpr(const analyze_printf::PrintfSpecifier &FS,
84248424
case ArgType::Match:
84258425
case ArgType::MatchPromotion:
84268426
case ArgType::NoMatchPromotionTypeConfusion:
8427-
case ArgType::NoMatchSignedness:
84288427
llvm_unreachable("expected non-matching");
8428+
case ArgType::NoMatchSignedness:
8429+
Diag = diag::warn_format_conversion_argument_type_mismatch_signedness;
8430+
break;
84298431
case ArgType::NoMatchPedantic:
84308432
Diag = diag::warn_format_conversion_argument_type_mismatch_pedantic;
84318433
break;
@@ -8750,17 +8752,20 @@ bool CheckScanfHandler::HandleScanfSpecifier(
87508752
analyze_format_string::ArgType::MatchKind Match =
87518753
AT.matchesType(S.Context, Ex->getType());
87528754
Match = handleFormatSignedness(Match, S.getDiagnostics(), Ex->getExprLoc());
8753-
bool Pedantic = Match == analyze_format_string::ArgType::NoMatchPedantic;
87548755
if (Match == analyze_format_string::ArgType::Match)
87558756
return true;
8757+
bool Pedantic = Match == analyze_format_string::ArgType::NoMatchPedantic;
8758+
bool Signedness = Match == analyze_format_string::ArgType::NoMatchSignedness;
87568759

87578760
ScanfSpecifier fixedFS = FS;
87588761
bool Success = fixedFS.fixType(Ex->getType(), Ex->IgnoreImpCasts()->getType(),
87598762
S.getLangOpts(), S.Context);
87608763

87618764
unsigned Diag =
87628765
Pedantic ? diag::warn_format_conversion_argument_type_mismatch_pedantic
8763-
: diag::warn_format_conversion_argument_type_mismatch;
8766+
: Signedness
8767+
? diag::warn_format_conversion_argument_type_mismatch_signedness
8768+
: diag::warn_format_conversion_argument_type_mismatch;
87648769

87658770
if (Success) {
87668771
// Get the fix string from the fixed format specifier.

‎clang/test/Sema/format-strings-signedness.c

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -39,41 +39,41 @@ void test_printf_unsigned_char(unsigned char x)
3939
void test_printf_int(int x)
4040
{
4141
printf("%d", x); // no-warning
42-
printf("%u", x); // expected-warning{{format specifies type 'unsigned int' but the argument has type 'int'}}
43-
printf("%x", x); // expected-warning{{format specifies type 'unsigned int' but the argument has type 'int'}}
42+
printf("%u", x); // expected-warning{{format specifies type 'unsigned int' but the argument has type 'int', which differs in signedness}}
43+
printf("%x", x); // expected-warning{{format specifies type 'unsigned int' but the argument has type 'int', which differs in signedness}}
4444
}
4545

4646
void test_printf_unsigned(unsigned x)
4747
{
48-
printf("%d", x); // expected-warning{{format specifies type 'int' but the argument has type 'unsigned int'}}
48+
printf("%d", x); // expected-warning{{format specifies type 'int' but the argument has type 'unsigned int', which differs in signedness}}
4949
printf("%u", x); // no-warning
5050
printf("%x", x); // no-warning
5151
}
5252

5353
void test_printf_long(long x)
5454
{
5555
printf("%ld", x); // no-warning
56-
printf("%lu", x); // expected-warning{{format specifies type 'unsigned long' but the argument has type 'long'}}
57-
printf("%lx", x); // expected-warning{{format specifies type 'unsigned long' but the argument has type 'long'}}
56+
printf("%lu", x); // expected-warning{{format specifies type 'unsigned long' but the argument has type 'long', which differs in signedness}}
57+
printf("%lx", x); // expected-warning{{format specifies type 'unsigned long' but the argument has type 'long', which differs in signedness}}
5858
}
5959

6060
void test_printf_unsigned_long(unsigned long x)
6161
{
62-
printf("%ld", x); // expected-warning{{format specifies type 'long' but the argument has type 'unsigned long'}}
62+
printf("%ld", x); // expected-warning{{format specifies type 'long' but the argument has type 'unsigned long', which differs in signedness}}
6363
printf("%lu", x); // no-warning
6464
printf("%lx", x); // no-warning
6565
}
6666

6767
void test_printf_long_long(long long x)
6868
{
6969
printf("%lld", x); // no-warning
70-
printf("%llu", x); // expected-warning{{format specifies type 'unsigned long long' but the argument has type 'long long'}}
71-
printf("%llx", x); // expected-warning{{format specifies type 'unsigned long long' but the argument has type 'long long'}}
70+
printf("%llu", x); // expected-warning{{format specifies type 'unsigned long long' but the argument has type 'long long', which differs in signedness}}
71+
printf("%llx", x); // expected-warning{{format specifies type 'unsigned long long' but the argument has type 'long long', which differs in signedness}}
7272
}
7373

7474
void test_printf_unsigned_long_long(unsigned long long x)
7575
{
76-
printf("%lld", x); // expected-warning{{format specifies type 'long long' but the argument has type 'unsigned long long'}}
76+
printf("%lld", x); // expected-warning{{format specifies type 'long long' but the argument has type 'unsigned long long', which differs in signedness}}
7777
printf("%llu", x); // no-warning
7878
printf("%llx", x); // no-warning
7979
}
@@ -85,8 +85,8 @@ enum enum_int {
8585
void test_printf_enum_int(enum enum_int x)
8686
{
8787
printf("%d", x); // no-warning
88-
printf("%u", x); // expected-warning{{format specifies type 'unsigned int' but the argument has underlying type 'int'}}
89-
printf("%x", x); // expected-warning{{format specifies type 'unsigned int' but the argument has underlying type 'int'}}
88+
printf("%u", x); // expected-warning{{format specifies type 'unsigned int' but the argument has underlying type 'int', which differs in signedness}}
89+
printf("%x", x); // expected-warning{{format specifies type 'unsigned int' but the argument has underlying type 'int', which differs in signedness}}
9090
}
9191

9292
#ifndef _WIN32 // Disabled due to enums have different underlying type on _WIN32
@@ -96,7 +96,7 @@ enum enum_unsigned {
9696

9797
void test_printf_enum_unsigned(enum enum_unsigned x)
9898
{
99-
printf("%d", x); // expected-warning{{format specifies type 'int' but the argument has underlying type 'unsigned int'}}
99+
printf("%d", x); // expected-warning{{format specifies type 'int' but the argument has underlying type 'unsigned int', which differs in signedness}}
100100
printf("%u", x); // no-warning
101101
printf("%x", x); // no-warning
102102
}
@@ -110,8 +110,8 @@ enum enum_long {
110110
void test_printf_enum_long(enum enum_long x)
111111
{
112112
printf("%ld", x); // no-warning
113-
printf("%lu", x); // expected-warning{{format specifies type 'unsigned long' but the argument has underlying type 'long'}}
114-
printf("%lx", x); // expected-warning{{format specifies type 'unsigned long' but the argument has underlying type 'long'}}
113+
printf("%lu", x); // expected-warning{{format specifies type 'unsigned long' but the argument has underlying type 'long', which differs in signedness}}
114+
printf("%lx", x); // expected-warning{{format specifies type 'unsigned long' but the argument has underlying type 'long', which differs in signedness}}
115115
}
116116

117117
enum enum_unsigned_long {
@@ -120,7 +120,7 @@ enum enum_unsigned_long {
120120

121121
void test_printf_enum_unsigned_long(enum enum_unsigned_long x)
122122
{
123-
printf("%ld", x); // expected-warning{{format specifies type 'long' but the argument has underlying type 'unsigned long'}}
123+
printf("%ld", x); // expected-warning{{format specifies type 'long' but the argument has underlying type 'unsigned long', which differs in signedness}}
124124
printf("%lu", x); // no-warning
125125
printf("%lx", x); // no-warning
126126
}
@@ -136,61 +136,61 @@ void test_scanf_unsigned_char(unsigned char *y) {
136136

137137
void test_scanf_int(int *x) {
138138
scanf("%d", x); // no-warning
139-
scanf("%u", x); // expected-warning{{format specifies type 'unsigned int *' but the argument has type 'int *'}}
140-
scanf("%x", x); // expected-warning{{format specifies type 'unsigned int *' but the argument has type 'int *'}}
139+
scanf("%u", x); // expected-warning{{format specifies type 'unsigned int *' but the argument has type 'int *', which differs in signedness}}
140+
scanf("%x", x); // expected-warning{{format specifies type 'unsigned int *' but the argument has type 'int *', which differs in signedness}}
141141
}
142142

143143
void test_scanf_unsigned(unsigned *x) {
144-
scanf("%d", x); // expected-warning{{format specifies type 'int *' but the argument has type 'unsigned int *'}}
144+
scanf("%d", x); // expected-warning{{format specifies type 'int *' but the argument has type 'unsigned int *', which differs in signedness}}
145145
scanf("%u", x); // no-warning
146146
scanf("%x", x); // no-warning
147147
}
148148

149149
void test_scanf_long(long *x) {
150150
scanf("%ld", x); // no-warning
151-
scanf("%lu", x); // expected-warning{{format specifies type 'unsigned long *' but the argument has type 'long *'}}
152-
scanf("%lx", x); // expected-warning{{format specifies type 'unsigned long *' but the argument has type 'long *'}}
151+
scanf("%lu", x); // expected-warning{{format specifies type 'unsigned long *' but the argument has type 'long *', which differs in signedness}}
152+
scanf("%lx", x); // expected-warning{{format specifies type 'unsigned long *' but the argument has type 'long *', which differs in signedness}}
153153
}
154154

155155
void test_scanf_unsigned_long(unsigned long *x) {
156-
scanf("%ld", x); // expected-warning{{format specifies type 'long *' but the argument has type 'unsigned long *'}}
156+
scanf("%ld", x); // expected-warning{{format specifies type 'long *' but the argument has type 'unsigned long *', which differs in signedness}}
157157
scanf("%lu", x); // no-warning
158158
scanf("%lx", x); // no-warning
159159
}
160160

161161
void test_scanf_longlong(long long *x) {
162162
scanf("%lld", x); // no-warning
163-
scanf("%llu", x); // expected-warning{{format specifies type 'unsigned long long *' but the argument has type 'long long *'}}
164-
scanf("%llx", x); // expected-warning{{format specifies type 'unsigned long long *' but the argument has type 'long long *'}}
163+
scanf("%llu", x); // expected-warning{{format specifies type 'unsigned long long *' but the argument has type 'long long *', which differs in signedness}}
164+
scanf("%llx", x); // expected-warning{{format specifies type 'unsigned long long *' but the argument has type 'long long *', which differs in signedness}}
165165
}
166166

167167
void test_scanf_unsigned_longlong(unsigned long long *x) {
168-
scanf("%lld", x); // expected-warning{{format specifies type 'long long *' but the argument has type 'unsigned long long *'}}
168+
scanf("%lld", x); // expected-warning{{format specifies type 'long long *' but the argument has type 'unsigned long long *', which differs in signedness}}
169169
scanf("%llu", x); // no-warning
170170
scanf("%llx", x); // no-warning
171171
}
172172

173173
void test_scanf_enum_int(enum enum_int *x) {
174174
scanf("%d", x); // no-warning
175-
scanf("%u", x); // expected-warning{{format specifies type 'unsigned int *' but the argument has type 'enum enum_int *'}}
176-
scanf("%x", x); // expected-warning{{format specifies type 'unsigned int *' but the argument has type 'enum enum_int *'}}
175+
scanf("%u", x); // expected-warning{{format specifies type 'unsigned int *' but the argument has type 'enum enum_int *', which differs in signedness}}
176+
scanf("%x", x); // expected-warning{{format specifies type 'unsigned int *' but the argument has type 'enum enum_int *', which differs in signedness}}
177177
}
178178

179179
#ifndef _WIN32 // Disabled due to enums have different underlying type on _WIN32
180180
void test_scanf_enum_unsigned(enum enum_unsigned *x) {
181-
scanf("%d", x); // expected-warning{{format specifies type 'int *' but the argument has type 'enum enum_unsigned *'}}
181+
scanf("%d", x); // expected-warning{{format specifies type 'int *' but the argument has type 'enum enum_unsigned *', which differs in signedness}}
182182
scanf("%u", x); // no-warning
183183
scanf("%x", x); // no-warning
184184
}
185185

186186
void test_scanf_enum_long(enum enum_long *x) {
187187
scanf("%ld", x); // no-warning
188-
scanf("%lu", x); // expected-warning{{format specifies type 'unsigned long *' but the argument has type 'enum enum_long *'}}
189-
scanf("%lx", x); // expected-warning{{format specifies type 'unsigned long *' but the argument has type 'enum enum_long *'}}
188+
scanf("%lu", x); // expected-warning{{format specifies type 'unsigned long *' but the argument has type 'enum enum_long *', which differs in signedness}}
189+
scanf("%lx", x); // expected-warning{{format specifies type 'unsigned long *' but the argument has type 'enum enum_long *', which differs in signedness}}
190190
}
191191

192192
void test_scanf_enum_unsigned_long(enum enum_unsigned_long *x) {
193-
scanf("%ld", x); // expected-warning{{format specifies type 'long *' but the argument has type 'enum enum_unsigned_long *'}}
193+
scanf("%ld", x); // expected-warning{{format specifies type 'long *' but the argument has type 'enum enum_unsigned_long *', which differs in signedness}}
194194
scanf("%lu", x); // no-warning
195195
scanf("%lx", x); // no-warning
196196
}

0 commit comments

Comments
 (0)