fcloutier created this revision.
fcloutier added a reviewer: aaron.ballman.
Herald added a project: All.
fcloutier requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
It's been reported that when using __attribute__((format)) on non-variadic
functions, certain values that normally get promoted when passed as variadic
arguments now unconditionally emit a diagnostic:
c
void foo(const char *fmt, float f) __attribute__((format(printf, 1, 2)));
void bar(void) {
foo("%g", 123.f);
// ^ format specifies type 'double' but the argument has type 'float'
}
This is normally not an issue because float values get promoted to doubles when
passed as variadic arguments, but needless to say, variadic argument promotion
does not apply to non-variadic arguments.
While this can be fixed by adjusting the prototype of `foo`, this is sometimes
undesirable in C (for instance, if `foo` is ABI). In C++, using variadic
templates, this might instead require call-site fixing, which is tedious and
arguably needless work:
c++
template<typename... Args>
void foo(const char *fmt, Args &&...args) __attribute__((format(printf, 1,
2)));
void bar(void) {
foo("%g", 123.f);
// ^ format specifies type 'double' but the argument has type 'float'
}
To address this issue, we teach FormatString about a few promotions that have
always been around but that have never been exercised in the direction that
FormatString checks for:
- `char`, `unsigned char` -> `int`, `unsigned`
- `half`, `float16`, `float` -> `double`
This addresses issue https://github.com/llvm/llvm-project/issues/59824.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D158318
Files:
clang/lib/AST/FormatString.cpp
clang/test/Sema/attr-format.c
clang/test/SemaCXX/attr-format.cpp
Index: clang/test/SemaCXX/attr-format.cpp
===================================================================
--- clang/test/SemaCXX/attr-format.cpp
+++ clang/test/SemaCXX/attr-format.cpp
@@ -78,6 +78,9 @@
format("%s %s %u %d %i %p\n", "hello", s, 10u, x, y, do_format);
format("bad format %s"); // expected-warning{{more '%' conversions than data
arguments}}
+ format("%c %c %hhd %hd %d\n", (char)'a', 'a', 'a', (short)123, (int)123);
+ format("%f %f %f\n", (__fp16)123.f, 123.f, 123.);
+
struct foo f;
format_invalid_nonpod("hello %i", f); // expected-warning{{format specifies
type 'int' but the argument has type 'struct foo'}}
Index: clang/test/Sema/attr-format.c
===================================================================
--- clang/test/Sema/attr-format.c
+++ clang/test/Sema/attr-format.c
@@ -94,13 +94,9 @@
d3("%s", 123); // expected-warning{{format specifies type 'char *' but the
argument has type 'int'}}
}
-__attribute__((format(printf, 1, 2))) void forward_fixed(const char *fmt, int
i) { // expected-warning{{GCC requires a function with the 'format' attribute
to be variadic}}
- forward_fixed(fmt, i);
- a(fmt, i);
-}
-
-__attribute__((format(printf, 1, 2))) void forward_fixed_2(const char *fmt,
int i, int j) { // expected-warning{{GCC requires a function with the 'format'
attribute to be variadic}}
- forward_fixed_2(fmt, i, j);
- a(fmt, i);
+__attribute__((format(printf, 1, 2)))
+void forward_fixed(const char *fmt, char i, short j, int k, float l, double m)
{ // expected-warning{{GCC requires a function with the 'format' attribute to
be variadic}}
+ forward_fixed(fmt, i, j, k, l, m);
+ a(fmt, i, j, k, l, m);
}
Index: clang/lib/AST/FormatString.cpp
===================================================================
--- clang/lib/AST/FormatString.cpp
+++ clang/lib/AST/FormatString.cpp
@@ -465,6 +465,24 @@
T == C.WideCharTy)
return MatchPromotion;
break;
+ case BuiltinType::Char_U:
+ if (T == C.UnsignedIntTy)
+ return MatchPromotion;
+ if (T == C.UnsignedShortTy)
+ return NoMatchPromotionTypeConfusion;
+ break;
+ case BuiltinType::Char_S:
+ if (T == C.IntTy)
+ return MatchPromotion;
+ if (T == C.ShortTy)
+ return NoMatchPromotionTypeConfusion;
+ break;
+ case BuiltinType::Half:
+ case BuiltinType::Float16:
+ case BuiltinType::Float:
+ if (T == C.DoubleTy)
+ return MatchPromotion;
+ break;
case BuiltinType::Short:
case BuiltinType::UShort:
if (T == C.SignedCharTy || T == C.UnsignedCharTy)
Index: clang/test/SemaCXX/attr-format.cpp
===================================================================
--- clang/test/SemaCXX/attr-format.cpp
+++ clang/test/SemaCXX/attr-format.cpp
@@ -78,6 +78,9 @@
format("%s %s %u %d %i %p\n", "hello", s, 10u, x, y, do_format);
format("bad format %s"); // expected-warning{{more '%' conversions than data arguments}}
+ format("%c %c %hhd %hd %d\n", (char)'a', 'a', 'a', (short)123, (int)123);
+ format("%f %f %f\n", (__fp16)123.f, 123.f, 123.);
+
struct foo f;
format_invalid_nonpod("hello %i", f); // expected-warning{{format specifies type 'int' but the argument has type 'struct foo'}}
Index: clang/test/Sema/attr-format.c
===================================================================
--- clang/test/Sema/attr-format.c
+++ clang/test/Sema/attr-format.c
@@ -94,13 +94,9 @@
d3("%s", 123); // expected-warning{{format specifies type 'char *' but the argument has type 'int'}}
}
-__attribute__((format(printf, 1, 2))) void forward_fixed(const char *fmt, int i) { // expected-warning{{GCC requires a function with the 'format' attribute to be variadic}}
- forward_fixed(fmt, i);
- a(fmt, i);
-}
-
-__attribute__((format(printf, 1, 2))) void forward_fixed_2(const char *fmt, int i, int j) { // expected-warning{{GCC requires a function with the 'format' attribute to be variadic}}
- forward_fixed_2(fmt, i, j);
- a(fmt, i);
+__attribute__((format(printf, 1, 2)))
+void forward_fixed(const char *fmt, char i, short j, int k, float l, double m) { // expected-warning{{GCC requires a function with the 'format' attribute to be variadic}}
+ forward_fixed(fmt, i, j, k, l, m);
+ a(fmt, i, j, k, l, m);
}
Index: clang/lib/AST/FormatString.cpp
===================================================================
--- clang/lib/AST/FormatString.cpp
+++ clang/lib/AST/FormatString.cpp
@@ -465,6 +465,24 @@
T == C.WideCharTy)
return MatchPromotion;
break;
+ case BuiltinType::Char_U:
+ if (T == C.UnsignedIntTy)
+ return MatchPromotion;
+ if (T == C.UnsignedShortTy)
+ return NoMatchPromotionTypeConfusion;
+ break;
+ case BuiltinType::Char_S:
+ if (T == C.IntTy)
+ return MatchPromotion;
+ if (T == C.ShortTy)
+ return NoMatchPromotionTypeConfusion;
+ break;
+ case BuiltinType::Half:
+ case BuiltinType::Float16:
+ case BuiltinType::Float:
+ if (T == C.DoubleTy)
+ return MatchPromotion;
+ break;
case BuiltinType::Short:
case BuiltinType::UShort:
if (T == C.SignedCharTy || T == C.UnsignedCharTy)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
