inclyc created this revision.
inclyc added a reviewer: aaron.ballman.
Herald added a subscriber: emaste.
Herald added a project: All.
inclyc published this revision for review.
inclyc added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
https://reviews.llvm.org/D132266
This patch seems to ignore this scenario:
printf("%hhf", [int])
Where we have length modifier `h`, and the argument type is `int`. Causes clang
to suppress warnings about type mismatches between floats and integers.
================
Comment at: clang/test/FixIt/format.mm:10
const wchar_t wchar_data = L'a';
NSLog(@"%C", wchar_data); // expected-warning{{format specifies type
'unichar' (aka 'unsigned short') but the argument has type 'wchar_t'}}
----------------
The language built-in `wchar_t` will be properly diagnosed.
================
Comment at: clang/test/SemaObjC/format-strings-objc.m:194
const wchar_t wchar_data = L'a';
- NSLog(@"%C", wchar_data); // expected-warning{{format specifies type
'unichar' (aka 'unsigned short') but the argument has type 'wchar_t'}}
}
----------------
This `wchar_t` is defined as `typedef __WCHAR_TYPE__ wchar_t;`. Actually it is
`int` on my machine, I'm not sure if we should still report this warning as
before, because it is just `int`, not a real `"wchar_t"`.
The main focus of this patch is to make ArgType::matchesType check for
possible default parameter promotions when the argType is not a pointer.
If so, no warning will be given for `int`, `unsigned int` types as
corresponding arguments to %hhd and %hd. However, the usage of %hhd
corresponding to short is relatively rare, and it is more likely to be a
misuse. This patch keeps the original behavior of clang like this as
much as possible, while making it more convenient to consider the
default arguments promotion.
Fixes https://github.com/llvm/llvm-project/issues/57102
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D132568
Files:
clang/include/clang/AST/FormatString.h
clang/lib/AST/FormatString.cpp
clang/lib/Sema/SemaChecking.cpp
clang/test/FixIt/format.m
clang/test/FixIt/format.mm
clang/test/Sema/format-strings-freebsd.c
clang/test/Sema/format-strings-scanf.c
clang/test/Sema/format-strings.c
clang/test/SemaObjC/format-strings-objc.m
Index: clang/test/SemaObjC/format-strings-objc.m
===================================================================
--- clang/test/SemaObjC/format-strings-objc.m
+++ clang/test/SemaObjC/format-strings-objc.m
@@ -80,7 +80,7 @@
// <rdar://problem/7068334> - Catch use of long long with int arguments.
void rdar_7068334(void) {
- long long test = 500;
+ long long test = 500;
printf("%i ",test); // expected-warning{{format specifies type 'int' but the argument has type 'long long'}}
NSLog(@"%i ",test); // expected-warning{{format specifies type 'int' but the argument has type 'long long'}}
CFStringCreateWithFormat(CFSTR("%i"),test); // expected-warning{{format specifies type 'int' but the argument has type 'long long'}}
@@ -191,7 +191,7 @@
NSLog(@"%C", data); // no-warning
const wchar_t wchar_data = L'a';
- NSLog(@"%C", wchar_data); // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'wchar_t'}}
+ NSLog(@"%C", wchar_data); // no-warning
}
// Test that %@ works with toll-free bridging (<rdar://problem/10814120>).
@@ -273,7 +273,7 @@
void testUnicode(void) {
NSLog(@"%C", 0x2022); // no-warning
- NSLog(@"%C", 0x202200); // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'int'}}
+ NSLog(@"%C", 0x202200); // no-warning
}
// Test Objective-C modifier flags.
Index: clang/test/Sema/format-strings.c
===================================================================
--- clang/test/Sema/format-strings.c
+++ clang/test/Sema/format-strings.c
@@ -830,3 +830,28 @@
printf_arg2("foo", "%s string %i\n", "aaa", 123);
printf_arg2("%s string\n", "foo", "bar"); // expected-warning{{data argument not used by format string}}
}
+
+void test_promotion(void) {
+ // Default argument promotions for *printf in N2562
+ // https://github.com/llvm/llvm-project/issues/57102
+ // N2562: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2562.pdf
+ int i;
+ signed char sc;
+ unsigned char uc;
+ char c;
+
+ printf("%hhd %hd %d %hhd %hd %d", i, i, i, sc, sc, sc); // no-warning
+ printf("%hhd %hd %d %hhd %hd %d", uc, uc, uc, c, c, c); // no-warning
+
+ printf("%ld", i); // expected-warning{{format specifies type 'long' but the argument has type 'int'}}
+ printf("%lld", i); // expected-warning{{format specifies type 'long long' but the argument has type 'int'}}
+ printf("%ld", sc); // expected-warning{{format specifies type 'long' but the argument has type 'signed char'}}
+ printf("%lld", sc); // expected-warning{{format specifies type 'long long' but the argument has type 'signed char'}}
+ printf("%ld", uc); // expected-warning{{format specifies type 'long' but the argument has type 'unsigned char'}}
+ printf("%lld", uc); // expected-warning{{format specifies type 'long long' but the argument has type 'unsigned char'}}
+ printf("%llx", i); // expected-warning{{format specifies type 'unsigned long long' but the argument has type 'int'}}
+ printf("%hf", // expected-warning{{length modifier 'h' results in undefined behavior or no effect with 'f' conversion specifier}}
+ sc); // expected-warning{{format specifies type 'double' but the argument has type 'signed char'}}
+ printf("%s", i); // expected-warning{{format specifies type 'char *' but the argument has type 'int'}}
+
+}
Index: clang/test/Sema/format-strings-scanf.c
===================================================================
--- clang/test/Sema/format-strings-scanf.c
+++ clang/test/Sema/format-strings-scanf.c
@@ -12,7 +12,7 @@
unsigned int : (int)0, \
unsigned short : (short)0, \
unsigned char : (signed char)0))
-typedef __SSIZE_TYPE__ ssize_t;
+typedef __SSIZE_TYPE__ ssize_t;
typedef __PTRDIFF_TYPE__ ptrdiff_t;
#define __UNSIGNED_PTRDIFF_TYPE__ \
@@ -224,13 +224,13 @@
ptrdiff_t p2 = 0;
scanf("%td", &p2); // No warning.
-
+
double d2 = 0.;
scanf("%td", &d2); // expected-warning-re{{format specifies type 'ptrdiff_t *' (aka '{{.+}}') but the argument has type 'double *'}}
ptrdiff_t p3 = 0;
scanf("%tn", &p3); // No warning.
-
+
double d3 = 0.;
scanf("%tn", &d3); // expected-warning-re{{format specifies type 'ptrdiff_t *' (aka '{{.+}}') but the argument has type 'double *'}}
}
@@ -246,3 +246,30 @@
scanf(i ? "%d" : "%d", i, s); // expected-warning{{data argument not used}}
scanf(i ? "%s" : "%d", s); // expected-warning{{format specifies type 'int *'}}
}
+
+void test_promotion(void) {
+ // No promotions for *scanf pointers clarified in N2562
+ // https://github.com/llvm/llvm-project/issues/57102
+ // N2562: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2562.pdf
+ int i;
+ signed char sc;
+ unsigned char uc;
+ char c;
+
+ scanf("%hhd", &i); // expected-warning{{format specifies type 'char *' but the argument has type 'int *'}}
+ scanf("%hd", &i); // expected-warning{{format specifies type 'short *' but the argument has type 'int *'}}
+ scanf("%d", &i); // no-warning
+
+
+ scanf("%ld", &i); // expected-warning{{format specifies type 'long *' but the argument has type 'int *'}}
+ scanf("%lld", &i); // expected-warning{{format specifies type 'long long *' but the argument has type 'int *'}}
+ scanf("%ld", &sc); // expected-warning{{format specifies type 'long *' but the argument has type 'signed char *'}}
+ scanf("%lld", &sc); // expected-warning{{format specifies type 'long long *' but the argument has type 'signed char *'}}
+ scanf("%ld", &uc); // expected-warning{{format specifies type 'long *' but the argument has type 'unsigned char *'}}
+ scanf("%lld", &uc); // expected-warning{{format specifies type 'long long *' but the argument has type 'unsigned char *'}}
+ scanf("%llx", &i); // expected-warning{{format specifies type 'unsigned long long *' but the argument has type 'int *'}}
+ scanf("%hf", // expected-warning{{length modifier 'h' results in undefined behavior or no effect with 'f' conversion specifier}}
+ &sc); // Is this a clang bug ?
+ scanf("%s", i); // expected-warning{{format specifies type 'char *' but the argument has type 'int'}}
+
+}
Index: clang/test/Sema/format-strings-freebsd.c
===================================================================
--- clang/test/Sema/format-strings-freebsd.c
+++ clang/test/Sema/format-strings-freebsd.c
@@ -35,9 +35,9 @@
freebsd_kernel_printf("%lr", l); // no-warning
// h modifier expects a short
- freebsd_kernel_printf("%hr", i); // expected-warning{{format specifies type 'short' but the argument has type 'int'}}
+ freebsd_kernel_printf("%hr", i); // no-warning
freebsd_kernel_printf("%hr", h); // no-warning
- freebsd_kernel_printf("%hy", i); // expected-warning{{format specifies type 'short' but the argument has type 'int'}}
+ freebsd_kernel_printf("%hy", i); // no-warning
freebsd_kernel_printf("%hy", h); // no-warning
// %y expects an int
Index: clang/test/FixIt/format.mm
===================================================================
--- clang/test/FixIt/format.mm
+++ clang/test/FixIt/format.mm
@@ -11,19 +11,12 @@
NSLog(@"%C", wchar_data); // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'wchar_t'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:16-[[@LINE-1]]:16}:"(unsigned short)"
- NSLog(@"%C", 0x260300); // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'int'}}
- // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%d"
- // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:"(unsigned short)"
-
typedef unsigned short unichar;
NSLog(@"%C", wchar_data); // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'wchar_t'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:16-[[@LINE-1]]:16}:"(unichar)"
-
- NSLog(@"%C", 0x260300); // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'int'}}
- // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%d"
- // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:"(unichar)"
-
+
+
NSLog(@"%C", 0.0); // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'double'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%f"
// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:"(unichar)"
Index: clang/test/FixIt/format.m
===================================================================
--- clang/test/FixIt/format.m
+++ clang/test/FixIt/format.m
@@ -37,7 +37,7 @@
// CHECK: fix-it:"{{.*}}":{34:11-34:13}:"%s"
}
-void test_object_correction (id x) {
+void test_object_correction (id x) {
NSLog(@"%d", x); // expected-warning{{format specifies type 'int' but the argument has type 'id'}}
NSLog(@"%s", x); // expected-warning{{format specifies type 'char *' but the argument has type 'id'}}
NSLog(@"%lf", x); // expected-warning{{format specifies type 'double' but the argument has type 'id'}}
@@ -108,7 +108,7 @@
NSLog(@"%c", c); // no-warning
// CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%c"
-
+
NSLog(@"%s", s); // expected-warning{{format specifies type 'char *' but the argument has type 'signed char'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%c"
@@ -170,11 +170,6 @@
NSLog(@"%@", 'abcd'); // expected-warning{{format specifies type 'id' but the argument has type 'int'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%d"
- NSLog(@"%hhd", 'a'); // expected-warning{{format specifies type 'char' but the argument has type 'int'}}
- // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:15}:"%d"
-
- NSLog(@"%hhu", 'a'); // expected-warning{{format specifies type 'unsigned char' but the argument has type 'int'}}
- // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:15}:"%d"
}
void multichar_constants_false_negative(void) {
@@ -192,20 +187,8 @@
const unsigned short data = 'a';
NSLog(@"%C", data); // no-warning
- NSLog(@"%C", 0x260300); // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'int'}}
- // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%d"
- // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:"(unsigned short)"
typedef unsigned short unichar;
-
- NSLog(@"%C", 0x260300); // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'int'}}
- // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%d"
- // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:"(unichar)"
-
- NSLog(@"%C", data ? 0x2F0000 : 0x260300); // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'int'}}
- // CHECK-NOT: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%d"
- // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:16-[[@LINE-2]]:16}:"(unichar)("
- // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:42-[[@LINE-3]]:42}:")"
NSLog(@"%C", 0.0); // expected-warning{{format specifies type 'unichar' (aka 'unsigned short') but the argument has type 'double'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:13}:"%f"
@@ -239,7 +222,7 @@
printf("%zd", 0.f); // expected-warning-re{{format specifies type 'ssize_t' (aka '{{.+}}') but the argument has type 'float'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:14}:"%f"
-
+
short x;
#if !defined(__ANDROID__) && !defined(__Fuchsia__)
printf("%zn", &x); // expected-warning-re{{format specifies type 'ssize_t *' (aka '{{.+}}') but the argument has type 'short *'}}
@@ -266,7 +249,7 @@
printf("%tu", 0.f); // expected-warning-re{{format specifies type 'unsigned ptrdiff_t' (aka '{{.+}}') but the argument has type 'float'}}
// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:11-[[@LINE-1]]:14}:"%f"
-
+
ptrdiff_t p2 = 0;
printf("%td", p2); // No warning.
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10081,9 +10081,13 @@
return true;
}
- analyze_printf::ArgType::MatchKind Match = AT.matchesType(S.Context, ExprTy);
- if (Match == analyze_printf::ArgType::Match)
+ ArgType::MatchKind ImplicitMatch = ArgType::NoMatch,
+ Match = AT.matchesType(S.Context, ExprTy);
+ if (Match == ArgType::Match)
return true;
+ assert(Match != ArgType::NoMatchPromotionTypeConfusion);
+
+ bool IsCharacterLiteralInt = false;
// Look through argument promotions for our error message's reported type.
// This includes the integral and floating promotions, but excludes array
@@ -10101,13 +10105,9 @@
if (ICE->getType() == S.Context.IntTy ||
ICE->getType() == S.Context.UnsignedIntTy) {
// All further checking is done on the subexpression
- const analyze_printf::ArgType::MatchKind ImplicitMatch =
- AT.matchesType(S.Context, ExprTy);
- if (ImplicitMatch == analyze_printf::ArgType::Match)
+ ImplicitMatch = AT.matchesType(S.Context, ExprTy);
+ if (ImplicitMatch == ArgType::Match)
return true;
- if (ImplicitMatch == ArgType::NoMatchPedantic ||
- ImplicitMatch == ArgType::NoMatchTypeConfusion)
- Match = ImplicitMatch;
}
}
} else if (const CharacterLiteral *CL = dyn_cast<CharacterLiteral>(E)) {
@@ -10118,9 +10118,23 @@
// modifier is provided.
if (ExprTy == S.Context.IntTy &&
FS.getLengthModifier().getKind() != LengthModifier::AsChar)
- if (llvm::isUIntN(S.Context.getCharWidth(), CL->getValue()))
+ if (llvm::isUIntN(S.Context.getCharWidth(), CL->getValue())) {
ExprTy = S.Context.CharTy;
+ IsCharacterLiteralInt = true;
+ }
}
+ if (Match == ArgType::MatchPromotion) {
+ if (ImplicitMatch != ArgType::NoMatchPromotionTypeConfusion &&
+ ImplicitMatch != ArgType::NoMatchTypeConfusion &&
+ !IsCharacterLiteralInt)
+ return true;
+ Match = ArgType::NoMatch;
+ }
+ if (ImplicitMatch == ArgType::NoMatchPedantic ||
+ ImplicitMatch == ArgType::NoMatchTypeConfusion)
+ Match = ImplicitMatch;
+
+ assert(Match != ArgType::MatchPromotion);
// Look through enums to their underlying type.
bool IsEnum = false;
@@ -10194,7 +10208,10 @@
if (IntendedTy == ExprTy && !ShouldNotPrintDirectly) {
unsigned Diag;
switch (Match) {
- case ArgType::Match: llvm_unreachable("expected non-matching");
+ case ArgType::Match:
+ case ArgType::MatchPromotion:
+ case ArgType::NoMatchPromotionTypeConfusion:
+ llvm_unreachable("expected non-matching");
case ArgType::NoMatchPedantic:
Diag = diag::warn_format_conversion_argument_type_mismatch_pedantic;
break;
@@ -10291,7 +10308,10 @@
case Sema::VAK_ValidInCXX11: {
unsigned Diag;
switch (Match) {
- case ArgType::Match: llvm_unreachable("expected non-matching");
+ case ArgType::Match:
+ case ArgType::MatchPromotion:
+ case ArgType::NoMatchPromotionTypeConfusion:
+ llvm_unreachable("expected non-matching");
case ArgType::NoMatchPedantic:
Diag = diag::warn_format_conversion_argument_type_mismatch_pedantic;
break;
Index: clang/lib/AST/FormatString.cpp
===================================================================
--- clang/lib/AST/FormatString.cpp
+++ clang/lib/AST/FormatString.cpp
@@ -356,17 +356,30 @@
argTy = ETy->getDecl()->getIntegerType();
}
- if (const BuiltinType *BT = argTy->getAs<BuiltinType>())
+ if (const BuiltinType *BT = argTy->getAs<BuiltinType>()) {
switch (BT->getKind()) {
+ default:
+ break;
+ case BuiltinType::Char_S:
+ case BuiltinType::SChar:
+ case BuiltinType::UChar:
+ case BuiltinType::Char_U:
+ case BuiltinType::Bool:
+ return Match;
+ }
+ if (!Ptr) {
+ switch (BT->getKind()) {
default:
break;
- case BuiltinType::Char_S:
- case BuiltinType::SChar:
- case BuiltinType::UChar:
- case BuiltinType::Char_U:
- case BuiltinType::Bool:
- return Match;
+ case BuiltinType::Int:
+ case BuiltinType::UInt:
+ return MatchPromotion;
+ case BuiltinType::Short:
+ case BuiltinType::UShort:
+ return NoMatchPromotionTypeConfusion;
+ }
}
+ }
return NoMatch;
}
@@ -384,7 +397,41 @@
if (T == argTy)
return Match;
// Check for "compatible types".
- if (const BuiltinType *BT = argTy->getAs<BuiltinType>())
+ if (const BuiltinType *BT = argTy->getAs<BuiltinType>()) {
+ if (!Ptr) {
+ switch (BT->getKind()) {
+ default:
+ break;
+ case BuiltinType::Int:
+ case BuiltinType::UInt:
+ if (T == C.SignedCharTy || T == C.UnsignedCharTy ||
+ T == C.ShortTy || T == C.UnsignedShortTy) {
+ return MatchPromotion;
+ }
+ break;
+ case BuiltinType::Short:
+ case BuiltinType::UShort:
+ if (T == C.SignedCharTy || T == C.UnsignedCharTy || T == C.IntTy ||
+ T == C.UnsignedIntTy) {
+ return NoMatchPromotionTypeConfusion;
+ }
+ break;
+ case BuiltinType::Char_S:
+ case BuiltinType::SChar:
+ case BuiltinType::Char_U:
+ case BuiltinType::UChar:
+ case BuiltinType::Bool:
+ // Don't warn printf("%hd", [char])
+ // https://reviews.llvm.org/D66186
+ if (T == C.IntTy || T == C.UnsignedIntTy) {
+ return NoMatchPromotionTypeConfusion;
+ }
+ break;
+ case BuiltinType::WChar_U:
+ case BuiltinType::WChar_S:
+ return NoMatchPromotionTypeConfusion;
+ }
+ }
switch (BT->getKind()) {
default:
break;
@@ -414,6 +461,7 @@
case BuiltinType::ULongLong:
return T == C.LongLongTy ? Match : NoMatch;
}
+ }
return NoMatch;
}
Index: clang/include/clang/AST/FormatString.h
===================================================================
--- clang/include/clang/AST/FormatString.h
+++ clang/include/clang/AST/FormatString.h
@@ -263,6 +263,12 @@
/// The conversion specifier and the argument type are compatible. For
/// instance, "%d" and _Bool.
Match = 1,
+ /// The conversion specifier and the argument type are compatible because of
+ /// default argument promotions. For instance, "%hhd" and int.
+ MatchPromotion,
+ /// The conversion specifier and the argument type are compatible but still
+ /// seems likely to be an error. For instanace, "%hhd" and short.
+ NoMatchPromotionTypeConfusion,
/// The conversion specifier and the argument type are disallowed by the C
/// standard, but are in practice harmless. For instance, "%p" and int*.
NoMatchPedantic,
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
