Skip to content

Commit 04e6178

Browse files
[Sema] tolerate more promotion matches in format string checking
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 #59824.
1 parent 41818ce commit 04e6178

File tree

5 files changed

+107
-8
lines changed

5 files changed

+107
-8
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,13 @@ Removed Compiler Flags
128128
Attribute Changes in Clang
129129
--------------------------
130130

131+
- When a non-variadic function is decorated with the ``format`` attribute,
132+
Clang now checks that the format string would match the function's parameters'
133+
types after default argument promotion. As a result, it's no longer an
134+
automatic diagnostic to use parameters of types that the format style
135+
supports but that are never the result of default argument promotion, such as
136+
``float``. (`#59824: <https://github.com/llvm/llvm-project/issues/59824>`_)
137+
131138
Improvements to Clang's diagnostics
132139
-----------------------------------
133140
- Clang constexpr evaluator now prints template arguments when displaying

clang/lib/AST/FormatString.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,13 +458,35 @@ ArgType::matchesType(ASTContext &C, QualType argTy) const {
458458
switch (BT->getKind()) {
459459
default:
460460
break;
461+
case BuiltinType::Bool:
462+
if (T == C.IntTy || T == C.UnsignedIntTy)
463+
return MatchPromotion;
464+
break;
461465
case BuiltinType::Int:
462466
case BuiltinType::UInt:
463467
if (T == C.SignedCharTy || T == C.UnsignedCharTy ||
464468
T == C.ShortTy || T == C.UnsignedShortTy || T == C.WCharTy ||
465469
T == C.WideCharTy)
466470
return MatchPromotion;
467471
break;
472+
case BuiltinType::Char_U:
473+
if (T == C.UnsignedIntTy)
474+
return MatchPromotion;
475+
if (T == C.UnsignedShortTy)
476+
return NoMatchPromotionTypeConfusion;
477+
break;
478+
case BuiltinType::Char_S:
479+
if (T == C.IntTy)
480+
return MatchPromotion;
481+
if (T == C.ShortTy)
482+
return NoMatchPromotionTypeConfusion;
483+
break;
484+
case BuiltinType::Half:
485+
case BuiltinType::Float16:
486+
case BuiltinType::Float:
487+
if (T == C.DoubleTy)
488+
return MatchPromotion;
489+
break;
468490
case BuiltinType::Short:
469491
case BuiltinType::UShort:
470492
if (T == C.SignedCharTy || T == C.UnsignedCharTy)

clang/test/Sema/attr-format.c

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -94,13 +94,9 @@ void call_nonvariadic(void) {
9494
d3("%s", 123); // expected-warning{{format specifies type 'char *' but the argument has type 'int'}}
9595
}
9696

97-
__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}}
98-
forward_fixed(fmt, i);
99-
a(fmt, i);
100-
}
101-
102-
__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}}
103-
forward_fixed_2(fmt, i, j);
104-
a(fmt, i);
97+
__attribute__((format(printf, 1, 2)))
98+
void forward_fixed(const char *fmt, _Bool b, char i, short j, int k, float l, double m) { // expected-warning{{GCC requires a function with the 'format' attribute to be variadic}}
99+
forward_fixed(fmt, b, i, j, k, l, m);
100+
a(fmt, b, i, j, k, l, m);
105101
}
106102

clang/test/SemaCXX/attr-format.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,12 +72,20 @@ void do_format() {
7272
int x = 123;
7373
int &y = x;
7474
const char *s = "world";
75+
bool b = false;
7576
format("bare string");
7677
format("%s", 123); // expected-warning{{format specifies type 'char *' but the argument has type 'int'}}
7778
format("%s %s %u %d %i %p\n", "hello", s, 10u, x, y, &do_format);
7879
format("%s %s %u %d %i %p\n", "hello", s, 10u, x, y, do_format);
7980
format("bad format %s"); // expected-warning{{more '%' conversions than data arguments}}
8081

82+
format("%c %c %hhd %hd %d\n", (char)'a', 'a', 'a', (short)123, (int)123);
83+
format("%f %f %f\n", (__fp16)123.f, 123.f, 123.);
84+
format("%Lf", (__fp16)123.f); // expected-warning{{format specifies type 'long double' but the argument has type '__fp16'}}
85+
format("%Lf", 123.f); // expected-warning{{format specifies type 'long double' but the argument has type 'float'}}
86+
format("%hhi %hhu %hi %hu %i %u", b, b, b, b, b, b);
87+
format("%li", b); // expected-warning{{format specifies type 'long' but the argument has type 'bool'}}
88+
8189
struct foo f;
8290
format_invalid_nonpod("hello %i", f); // expected-warning{{format specifies type 'int' but the argument has type 'struct foo'}}
8391

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
// RUN: %clang_cc1 -fsyntax-only -verify -Wformat-nonliteral -Wformat-non-iso -fblocks -std=c++11 %s
2+
3+
__attribute__((format(scanf, 1, 2)))
4+
int scanf(const char *, ...);
5+
6+
template<typename... Args>
7+
__attribute__((format(scanf, 1, 2)))
8+
int scan(const char *fmt, Args &&...args) { // expected-warning{{GCC requires a function with the 'format' attribute to be variadic}}
9+
return scanf(fmt, args...);
10+
}
11+
12+
union bag {
13+
bool b;
14+
unsigned char uc;
15+
signed char sc;
16+
unsigned short us;
17+
signed short ss;
18+
unsigned int ui;
19+
signed int si;
20+
unsigned long ul;
21+
signed long sl;
22+
unsigned long long ull;
23+
signed long long sll;
24+
__fp16 f16;
25+
float ff;
26+
double fd;
27+
long double fl;
28+
};
29+
30+
void test(void) {
31+
bag b;
32+
scan("%hhi %hhu %hhi %hhu", &b.sc, &b.uc, &b.b, &b.b);
33+
scan("%hi %hu", &b.ss, &b.us);
34+
scan("%i %u", &b.si, &b.ui);
35+
scan("%li %lu", &b.sl, &b.ul);
36+
scan("%lli %llu", &b.sll, &b.ull);
37+
scan("%f", &b.ff);
38+
scan("%lf", &b.fd);
39+
scan("%Lf", &b.fl);
40+
41+
// expected-warning@+4{{format specifies type 'short *' but the argument has type 'signed char *'}}
42+
// expected-warning@+3{{format specifies type 'unsigned short *' but the argument has type 'unsigned char *'}}
43+
// expected-warning@+2{{format specifies type 'short *' but the argument has type 'bool *'}}
44+
// expected-warning@+1{{format specifies type 'unsigned short *' but the argument has type 'bool *'}}
45+
scan("%hi %hu %hi %hu", &b.sc, &b.uc, &b.b, &b.b);
46+
47+
// expected-warning@+3{{format specifies type 'long *' but the argument has type 'short *'}}
48+
// expected-warning@+2{{format specifies type 'char *' but the argument has type 'short *'}}
49+
// expected-warning@+1{{format specifies type 'int *' but the argument has type 'short *'}}
50+
scan("%hhi %i %li", &b.ss, &b.ss, &b.ss);
51+
52+
// expected-warning@+3{{format specifies type 'float *' but the argument has type '__fp16 *'}}
53+
// expected-warning@+2{{format specifies type 'float *' but the argument has type 'double *'}}
54+
// expected-warning@+1{{format specifies type 'float *' but the argument has type 'long double *'}}
55+
scan("%f %f %f", &b.f16, &b.fd, &b.fl);
56+
57+
// expected-warning@+3{{format specifies type 'double *' but the argument has type '__fp16 *'}}
58+
// expected-warning@+2{{format specifies type 'double *' but the argument has type 'float *'}}
59+
// expected-warning@+1{{format specifies type 'double *' but the argument has type 'long double *'}}
60+
scan("%lf %lf %lf", &b.f16, &b.ff, &b.fl);
61+
62+
// expected-warning@+3{{format specifies type 'long double *' but the argument has type '__fp16 *'}}
63+
// expected-warning@+2{{format specifies type 'long double *' but the argument has type 'float *'}}
64+
// expected-warning@+1{{format specifies type 'long double *' but the argument has type 'double *'}}
65+
scan("%Lf %Lf %Lf", &b.f16, &b.ff, &b.fd);
66+
}

0 commit comments

Comments
 (0)