fcloutier updated this revision to Diff 435302.
fcloutier added a comment.
Herald added a project: All.
Apologies the long delay: things happened and I was pulled away. I have some
time to finish this change now. I recommend re-reading the discussion up to now
since it's not _that_ long and it provides a lot of very useful context.
The new change addresses requests from the previous round. The most substantial
changes are around how Clang detects that a format string is being forwarded to
another format function. This is now expressed in terms of transitions from
format argument passing styles, such that given the following 3 function
archetypes:
c
void fixed(const char *, int) __attribute__((format(printf, 1, 2)));
void variadic(const char *, ...) __attribute__((format(printf, 1, 2)));
void valist(const char *, va_list) __attribute__((format(printf, 1, 0)));
there are no warnings for:
- a `variadic` function forwarding its format to a `valist` function
- a `valist` function forwarding its format to another `valist` function
- a `fixed` function forwarding its format to another `fixed` function (new)
- a `fixed` function forwarding its format to a `variadic` function (new)
In other words, for instance, `fixed` can call `variadic` in its implementation
without a warning. Anything else, like forwarding the format of a `valist`
function to a `fixed` function, is a diagnostic.
`fixed` to `fixed`/`variadic` transitions don't check that arguments have
compatible types, but it conceivably could. This is a limitation of the current
implementation. However, at this point, we don't think that this is a very
worthwhile effort; this could change in the future if adoption of the `format`
attribute on functions with a fixed signature ramps up.
I also added a number of tests to make sure that we still have reasonable
warnings. One interesting edge case when using `__attribute__((format))` on
functions with fixed parameters is that it's possible to come up with
combinations that are impossible, for instance:
c
struct nontrivial { nontrivial(); ~nontrivial(); };
void foo(const char *, nontrivial) __attribute__((format(printf, 1, 2)));
It's not a diagnostic to declare this function, however it is always a
diagnostic to call it because no printf format specifier can format a
`nontrivial` object. Ideally there would be a diagnostic on the declaration,
but I think that it's sufficient as it is.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112579/new/
https://reviews.llvm.org/D112579
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/AST/FormatString.cpp
clang/lib/Sema/SemaChecking.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/Sema/attr-format.c
clang/test/Sema/format-strings.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
@@ -1,7 +1,11 @@
// RUN: %clang_cc1 -fsyntax-only -Wformat-nonliteral -verify %s
+#include <stdarg.h>
+
+int printf(const char *fmt, ...) __attribute__((format(printf, 1, 2)));
+
struct S {
- static void f(const char*, ...) __attribute__((format(printf, 1, 2)));
- static const char* f2(const char*) __attribute__((format_arg(1)));
+ static void f(const char *, ...) __attribute__((format(printf, 1, 2)));
+ static const char *f2(const char *) __attribute__((format_arg(1)));
// GCC has a hidden 'this' argument in member functions which is why
// the format argument is argument 2 here.
@@ -38,6 +42,47 @@
// Make sure we interpret member operator calls as having an implicit
// this argument.
-void test_operator_call(S s, const char* str) {
+void test_operator_call(S s, const char *str) {
s("%s", str);
}
+
+template <typename... Args>
+void format(const char *fmt, Args &&...args) // expected-warning{{GCC requires functions with the 'format' attribute to be variadic}}
+ __attribute__((format(printf, 1, 2)));
+
+template <typename Arg>
+Arg &expand(Arg &a) { return a; }
+
+struct foo {
+ int big[10];
+ foo();
+ ~foo();
+
+ template <typename... Args>
+ void format(const char *const fmt, Args &&...args) // expected-warning{{GCC requires functions with the 'format' attribute to be variadic}}
+ __attribute__((format(printf, 2, 3))) {
+ printf(fmt, expand(args)...);
+ }
+};
+
+void format_invalid_nonpod(const char *fmt, struct foo f) // expected-warning{{GCC requires functions with the 'format' attribute to be variadic}}
+ __attribute__((format(printf, 1, 2)));
+
+void do_format() {
+ int x = 123;
+ int &y = x;
+ const char *s = "world";
+ format("bare string");
+ format("%s", 123); // expected-warning{{format specifies type 'char *' but the argument has type 'int'}}
+ format("%s %s %u %d %i %p\n", "hello", s, 10u, x, y, &do_format);
+ 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}}
+
+ struct foo f;
+ format_invalid_nonpod("hello %i", f); // expected-warning{{format specifies type 'int' but the argument has type 'struct foo'}}
+
+ f.format("%s", 123); // expected-warning{{format specifies type 'char *' but the argument has type 'int'}}
+ f.format("%s %s %u %d %i %p\n", "hello", s, 10u, x, y, &do_format);
+ f.format("%s %s %u %d %i %p\n", "hello", s, 10u, x, y, do_format);
+ f.format("bad format %s"); // expected-warning{{more '%' conversions than data arguments}}
+}
Index: clang/test/Sema/format-strings.c
===================================================================
--- clang/test/Sema/format-strings.c
+++ clang/test/Sema/format-strings.c
@@ -816,6 +816,7 @@
__attribute__((__format__(__printf__, 2, 3))) {
va_list ap;
va_start(ap, fmt);
+ vprintf(fmt, ap);
vprintf(not_fmt, ap); // expected-warning{{format string is not a string literal}}
va_end(ap);
};
Index: clang/test/Sema/attr-format.c
===================================================================
--- clang/test/Sema/attr-format.c
+++ clang/test/Sema/attr-format.c
@@ -2,15 +2,15 @@
#include <stdarg.h>
-void a(const char *a, ...) __attribute__((format(printf, 1,2))); // no-error
-void b(const char *a, ...) __attribute__((format(printf, 1,1))); // expected-error {{'format' attribute parameter 3 is out of bounds}}
-void c(const char *a, ...) __attribute__((format(printf, 0,2))); // expected-error {{'format' attribute parameter 2 is out of bounds}}
-void d(const char *a, int c) __attribute__((format(printf, 1,2))); // expected-error {{format attribute requires variadic function}}
-void e(char *str, int c, ...) __attribute__((format(printf, 2,3))); // expected-error {{format argument not a string type}}
+void a(const char *a, ...) __attribute__((format(printf, 1, 2))); // no-error
+void b(const char *a, ...) __attribute__((format(printf, 1, 1))); // expected-error {{'format' attribute parameter 3 is out of bounds}}
+void c(const char *a, ...) __attribute__((format(printf, 0, 2))); // expected-error {{'format' attribute parameter 2 is out of bounds}}
+void d(const char *a, int c) __attribute__((format(printf, 1, 2))); // expected-warning {{GCC requires functions with the 'format' attribute to be variadic}}
+void e(char *str, int c, ...) __attribute__((format(printf, 2, 3))); // expected-error {{format argument not a string type}}
-typedef const char* xpto;
+typedef const char *xpto;
void f(xpto c, va_list list) __attribute__((format(printf, 1, 0))); // no-error
-void g(xpto c) __attribute__((format(printf, 1, 0))); // no-error
+void g(xpto c) __attribute__((format(printf, 1, 0))); // no-error
void y(char *str) __attribute__((format(strftime, 1,0))); // no-error
void z(char *str, int c, ...) __attribute__((format(strftime, 1,2))); // expected-error {{strftime format attribute requires 3rd parameter to be 0}}
@@ -33,17 +33,15 @@
rdar6623513(0, "hello", "%.*s", len, s);
}
-
-
// same as format(printf(...))...
-void a2(const char *a, ...) __attribute__((format(printf0, 1,2))); // no-error
-void b2(const char *a, ...) __attribute__((format(printf0, 1,1))); // expected-error {{'format' attribute parameter 3 is out of bounds}}
-void c2(const char *a, ...) __attribute__((format(printf0, 0,2))); // expected-error {{'format' attribute parameter 2 is out of bounds}}
-void d2(const char *a, int c) __attribute__((format(printf0, 1,2))); // expected-error {{format attribute requires variadic function}}
-void e2(char *str, int c, ...) __attribute__((format(printf0, 2,3))); // expected-error {{format argument not a string type}}
+void a2(const char *a, ...) __attribute__((format(printf0, 1, 2))); // no-error
+void b2(const char *a, ...) __attribute__((format(printf0, 1, 1))); // expected-error {{'format' attribute parameter 3 is out of bounds}}
+void c2(const char *a, ...) __attribute__((format(printf0, 0, 2))); // expected-error {{'format' attribute parameter 2 is out of bounds}}
+void d2(const char *a, int c) __attribute__((format(printf0, 1, 2))); // expected-warning {{GCC requires functions with the 'format' attribute to be variadic}}
+void e2(char *str, int c, ...) __attribute__((format(printf0, 2, 3))); // expected-error {{format argument not a string type}}
// FreeBSD usage
-#define __printf0like(fmt,va) __attribute__((__format__(__printf0__,fmt,va)))
+#define __printf0like(fmt, va) __attribute__((__format__(__printf0__, fmt, va)))
void null(int i, const char *a, ...) __printf0like(2,0); // no-error
void null(int i, const char *a, ...) { // expected-note{{passing argument to parameter 'a' here}}
if (a)
@@ -58,13 +56,11 @@
}
// FreeBSD kernel extensions
-void a3(const char *a, ...) __attribute__((format(freebsd_kprintf, 1,2))); // no-error
-void b3(const char *a, ...) __attribute__((format(freebsd_kprintf, 1,1))); // expected-error {{'format' attribute parameter 3 is out of bounds}}
-void c3(const char *a, ...) __attribute__((format(freebsd_kprintf, 0,2))); // expected-error {{'format' attribute parameter 2 is out of bounds}}
-void d3(const char *a, int c) __attribute__((format(freebsd_kprintf, 1,2))); // expected-error {{format attribute requires variadic function}}
-void e3(char *str, int c, ...) __attribute__((format(freebsd_kprintf, 2,3))); // expected-error {{format argument not a string type}}
-
-
+void a3(const char *a, ...) __attribute__((format(freebsd_kprintf, 1, 2))); // no-error
+void b3(const char *a, ...) __attribute__((format(freebsd_kprintf, 1, 1))); // expected-error {{'format' attribute parameter 3 is out of bounds}}
+void c3(const char *a, ...) __attribute__((format(freebsd_kprintf, 0, 2))); // expected-error {{'format' attribute parameter 2 is out of bounds}}
+void d3(const char *a, int c) __attribute__((format(freebsd_kprintf, 1, 2))); // expected-warning {{GCC requires functions with the 'format' attribute to be variadic}}
+void e3(char *str, int c, ...) __attribute__((format(freebsd_kprintf, 2, 3))); // expected-error {{format argument not a string type}}
// PR4470
int xx_vprintf(const char *, va_list);
@@ -79,11 +75,22 @@
// PR6542
extern void gcc_format (const char *, ...)
__attribute__ ((__format__(__gcc_diag__, 1, 2)));
-extern void gcc_cformat (const char *, ...)
- __attribute__ ((__format__(__gcc_cdiag__, 1, 2)));
-extern void gcc_cxxformat (const char *, ...)
- __attribute__ ((__format__(__gcc_cxxdiag__, 1, 2)));
-extern void gcc_tformat (const char *, ...)
- __attribute__ ((__format__(__gcc_tdiag__, 1, 2)));
-
-const char *foo3(const char *format) __attribute__((format_arg("foo"))); // expected-error{{'format_arg' attribute requires parameter 1 to be an integer constant}}
+extern void gcc_cformat(const char *, ...)
+ __attribute__((__format__(__gcc_cdiag__, 1, 2)));
+extern void gcc_cxxformat(const char *, ...)
+ __attribute__((__format__(__gcc_cxxdiag__, 1, 2)));
+extern void gcc_tformat(const char *, ...)
+ __attribute__((__format__(__gcc_tdiag__, 1, 2)));
+
+const char *foo3(const char *format) __attribute__((format_arg("foo"))); // expected-error{{'format_arg' attribute requires parameter 1 to be an integer constant}}
+
+void call_nonvariadic(void) {
+ d3("%i", 123);
+ d3("%d", 123);
+ 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 functions with the 'format' attribute to be variadic}}
+ forward_fixed(fmt, i);
+ a(fmt, i);
+}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -3881,8 +3881,7 @@
if (isFunctionOrMethodVariadic(D)) {
++NumArgs; // +1 for ...
} else {
- S.Diag(D->getLocation(), diag::err_format_attribute_requires_variadic);
- return;
+ S.Diag(D->getLocation(), diag::warn_gcc_requires_variadic_function) << AL;
}
}
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -109,6 +109,11 @@
Context.getTargetInfo());
}
+static constexpr unsigned short combineFAPK(Sema::FormatArgumentPassingKind A,
+ Sema::FormatArgumentPassingKind B) {
+ return (A << 8) | B;
+}
+
/// Checks that a call expression's argument count is at least the desired
/// number. This is useful when doing custom type-checking on a variadic
/// function. Returns true on error.
@@ -5425,10 +5430,17 @@
/// Returns true when the format fits the function and the FormatStringInfo has
/// been populated.
bool Sema::getFormatStringInfo(const FormatAttr *Format, bool IsCXXMember,
- FormatStringInfo *FSI) {
- FSI->HasVAListArg = Format->getFirstArg() == 0;
+ bool IsVariadic, FormatStringInfo *FSI) {
+ if (Format->getFirstArg() == 0) {
+ FSI->ArgPassingKind = FAPK_VAList;
+ } else if (IsVariadic) {
+ FSI->ArgPassingKind = FAPK_Variadic;
+ } else {
+ FSI->ArgPassingKind = FAPK_Fixed;
+ }
FSI->FormatIdx = Format->getFormatIdx() - 1;
- FSI->FirstDataArg = FSI->HasVAListArg ? 0 : Format->getFirstArg() - 1;
+ FSI->FirstDataArg =
+ FSI->ArgPassingKind == FAPK_VAList ? 0 : Format->getFirstArg() - 1;
// The way the format attribute works in GCC, the implicit this argument
// of member functions is counted. However, it doesn't appear in our own
@@ -5483,7 +5495,7 @@
bool Sema::GetFormatNSStringIdx(const FormatAttr *Format, unsigned &Idx) {
FormatStringInfo FSI;
if ((GetFormatStringType(Format) == FST_NSString) &&
- getFormatStringInfo(Format, false, &FSI)) {
+ getFormatStringInfo(Format, false, true, &FSI)) {
Idx = FSI.FormatIdx;
return true;
}
@@ -7717,7 +7729,7 @@
llvm::SmallBitVector CheckedVarArgs(NumArgs, false);
ArrayRef<const Expr *> Args(TheCall->getArgs(), TheCall->getNumArgs());
bool Success = CheckFormatArguments(
- Args, /*HasVAListArg*/ false, FormatIdx, FirstDataArg, FST_OSLog,
+ Args, FAPK_Variadic, FormatIdx, FirstDataArg, FST_OSLog,
VariadicFunction, TheCall->getBeginLoc(), SourceRange(),
CheckedVarArgs);
if (!Success)
@@ -8436,17 +8448,13 @@
} // namespace
-static void CheckFormatString(Sema &S, const FormatStringLiteral *FExpr,
- const Expr *OrigFormatExpr,
- ArrayRef<const Expr *> Args,
- bool HasVAListArg, unsigned format_idx,
- unsigned firstDataArg,
- Sema::FormatStringType Type,
- bool inFunctionCall,
- Sema::VariadicCallType CallType,
- llvm::SmallBitVector &CheckedVarArgs,
- UncoveredArgHandler &UncoveredArg,
- bool IgnoreStringsWithoutSpecifiers);
+static void CheckFormatString(
+ Sema &S, const FormatStringLiteral *FExpr, const Expr *OrigFormatExpr,
+ ArrayRef<const Expr *> Args, Sema::FormatArgumentPassingKind APK,
+ unsigned format_idx, unsigned firstDataArg, Sema::FormatStringType Type,
+ bool inFunctionCall, Sema::VariadicCallType CallType,
+ llvm::SmallBitVector &CheckedVarArgs, UncoveredArgHandler &UncoveredArg,
+ bool IgnoreStringsWithoutSpecifiers);
// Determine if an expression is a string literal or constant string.
// If this function returns false on the arguments to a function expecting a
@@ -8454,12 +8462,11 @@
// True string literals are then checked by CheckFormatString.
static StringLiteralCheckType
checkFormatStringExpr(Sema &S, const Expr *E, ArrayRef<const Expr *> Args,
- bool HasVAListArg, unsigned format_idx,
+ Sema::FormatArgumentPassingKind APK, unsigned format_idx,
unsigned firstDataArg, Sema::FormatStringType Type,
Sema::VariadicCallType CallType, bool InFunctionCall,
llvm::SmallBitVector &CheckedVarArgs,
- UncoveredArgHandler &UncoveredArg,
- llvm::APSInt Offset,
+ UncoveredArgHandler &UncoveredArg, llvm::APSInt Offset,
bool IgnoreStringsWithoutSpecifiers = false) {
if (S.isConstantEvaluated())
return SLCT_NotALiteral;
@@ -8508,9 +8515,8 @@
if (!CheckLeft)
Left = SLCT_UncheckedLiteral;
else {
- Left = checkFormatStringExpr(S, C->getTrueExpr(), Args,
- HasVAListArg, format_idx, firstDataArg,
- Type, CallType, InFunctionCall,
+ Left = checkFormatStringExpr(S, C->getTrueExpr(), Args, APK, format_idx,
+ firstDataArg, Type, CallType, InFunctionCall,
CheckedVarArgs, UncoveredArg, Offset,
IgnoreStringsWithoutSpecifiers);
if (Left == SLCT_NotALiteral || !CheckRight) {
@@ -8519,8 +8525,8 @@
}
StringLiteralCheckType Right = checkFormatStringExpr(
- S, C->getFalseExpr(), Args, HasVAListArg, format_idx, firstDataArg,
- Type, CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset,
+ S, C->getFalseExpr(), Args, APK, format_idx, firstDataArg, Type,
+ CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset,
IgnoreStringsWithoutSpecifiers);
return (CheckLeft && Left < Right) ? Left : Right;
@@ -8570,42 +8576,85 @@
if (InitList->isStringLiteralInit())
Init = InitList->getInit(0)->IgnoreParenImpCasts();
}
- return checkFormatStringExpr(S, Init, Args,
- HasVAListArg, format_idx,
- firstDataArg, Type, CallType,
- /*InFunctionCall*/ false, CheckedVarArgs,
- UncoveredArg, Offset);
+ return checkFormatStringExpr(
+ S, Init, Args, APK, format_idx, firstDataArg, Type, CallType,
+ /*InFunctionCall*/ false, CheckedVarArgs, UncoveredArg, Offset);
}
}
- // For vprintf* functions (i.e., HasVAListArg==true), we add a
- // special check to see if the format string is a function parameter
- // of the function calling the printf function. If the function
- // has an attribute indicating it is a printf-like function, then we
- // should suppress warnings concerning non-literals being used in a call
- // to a vprintf function. For example:
+ // When the format argument is an argument of this function, and this
+ // function also has the format attribute, there are several interactions
+ // for which there shouldn't be a warning. For instance, when calling
+ // v*printf from a function that has the printf format attribute, we
+ // should not emit a warning about using `fmt`, even though it's not
+ // constant, because the arguments have already been checked for the
+ // caller of `logmessage`:
//
- // void
- // logmessage(char const *fmt __attribute__ (format (printf, 1, 2)), ...){
- // va_list ap;
- // va_start(ap, fmt);
- // vprintf(fmt, ap); // Do NOT emit a warning about "fmt".
- // ...
+ // __attribute__((format(printf, 1, 2)))
+ // void logmessage(char const *fmt, ...) {
+ // va_list ap;
+ // va_start(ap, fmt);
+ // vprintf(fmt, ap); /* do not emit a warning about "fmt" */
+ // ...
// }
- if (HasVAListArg) {
- if (const ParmVarDecl *PV = dyn_cast<ParmVarDecl>(VD)) {
- if (const Decl *D = dyn_cast<Decl>(PV->getDeclContext())) {
- int PVIndex = PV->getFunctionScopeIndex() + 1;
- for (const auto *PVFormat : D->specific_attrs<FormatAttr>()) {
- // adjust for implicit parameter
- if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(D))
- if (MD->isInstance())
- ++PVIndex;
+ //
+ // Another interaction that we need to support is calling a variadic
+ // format function from a format function that has fixed arguments. For
+ // instance:
+ //
+ // __attribute__((format(printf, 1, 2)))
+ // void logstring(char const *fmt, char const *str) {
+ // printf(fmt, str); /* do not emit a warning about "fmt" */
+ // }
+ //
+ // Same (and perhaps more relatably) for the variadic template case:
+ //
+ // template<typename... Args>
+ // __attribute__((format(printf, 1, 2)))
+ // void log(const char *fmt, Args&&... args) {
+ // printf(fmt, forward<Args>(args)...);
+ // /* do not emit a warning about "fmt" */
+ // }
+ //
+ // Due to implementation difficulty, we only check the format, not the
+ // format arguments, in all cases.
+ //
+ if (const ParmVarDecl *PV = dyn_cast<ParmVarDecl>(VD)) {
+ if (const Decl *D = dyn_cast<Decl>(PV->getDeclContext())) {
+ for (const auto *PVFormat : D->specific_attrs<FormatAttr>()) {
+ bool IsCXXMember = false;
+ if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(D))
+ IsCXXMember = MD->isInstance();
+
+ bool IsVariadic = false;
+ if (const FunctionType *FnTy = D->getFunctionType())
+ IsVariadic = cast<FunctionProtoType>(FnTy)->isVariadic();
+ else if (const auto *BD = dyn_cast<BlockDecl>(D))
+ IsVariadic = BD->isVariadic();
+ else if (const auto *OMD = dyn_cast<ObjCMethodDecl>(D))
+ IsVariadic = OMD->isVariadic();
+
+ Sema::FormatStringInfo CallerFSI;
+ if (Sema::getFormatStringInfo(PVFormat, IsCXXMember, IsVariadic,
+ &CallerFSI)) {
// We also check if the formats are compatible.
// We can't pass a 'scanf' string to a 'printf' function.
- if (PVIndex == PVFormat->getFormatIdx() &&
- Type == S.GetFormatStringType(PVFormat))
- return SLCT_UncheckedLiteral;
+ if (PV->getFunctionScopeIndex() == CallerFSI.FormatIdx &&
+ Type == S.GetFormatStringType(PVFormat)) {
+ // Lastly, check that argument passing kinds transition in a
+ // way that makes sense:
+ // from a caller with FAPK_VAList, allow FAPK_VAList
+ // from a caller with FAPK_Fixed, allow FAPK_Fixed
+ // from a caller with FAPK_Fixed, allow FAPK_Variadic
+ // from a caller with FAPK_Variadic, allow FAPK_VAList
+ switch (combineFAPK(CallerFSI.ArgPassingKind, APK)) {
+ case combineFAPK(Sema::FAPK_VAList, Sema::FAPK_VAList):
+ case combineFAPK(Sema::FAPK_Fixed, Sema::FAPK_Fixed):
+ case combineFAPK(Sema::FAPK_Fixed, Sema::FAPK_Variadic):
+ case combineFAPK(Sema::FAPK_Variadic, Sema::FAPK_VAList):
+ return SLCT_UncheckedLiteral;
+ }
+ }
}
}
}
@@ -8624,8 +8673,8 @@
for (const auto *FA : ND->specific_attrs<FormatArgAttr>()) {
const Expr *Arg = CE->getArg(FA->getFormatIdx().getASTIndex());
StringLiteralCheckType Result = checkFormatStringExpr(
- S, Arg, Args, HasVAListArg, format_idx, firstDataArg, Type,
- CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset,
+ S, Arg, Args, APK, format_idx, firstDataArg, Type, CallType,
+ InFunctionCall, CheckedVarArgs, UncoveredArg, Offset,
IgnoreStringsWithoutSpecifiers);
if (IsFirst) {
CommonResult = Result;
@@ -8640,12 +8689,10 @@
if (BuiltinID == Builtin::BI__builtin___CFStringMakeConstantString ||
BuiltinID == Builtin::BI__builtin___NSStringMakeConstantString) {
const Expr *Arg = CE->getArg(0);
- return checkFormatStringExpr(S, Arg, Args,
- HasVAListArg, format_idx,
- firstDataArg, Type, CallType,
- InFunctionCall, CheckedVarArgs,
- UncoveredArg, Offset,
- IgnoreStringsWithoutSpecifiers);
+ return checkFormatStringExpr(
+ S, Arg, Args, APK, format_idx, firstDataArg, Type, CallType,
+ InFunctionCall, CheckedVarArgs, UncoveredArg, Offset,
+ IgnoreStringsWithoutSpecifiers);
}
}
}
@@ -8673,8 +8720,8 @@
const Expr *Arg = ME->getArg(FA->getFormatIdx().getASTIndex());
return checkFormatStringExpr(
- S, Arg, Args, HasVAListArg, format_idx, firstDataArg, Type,
- CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset,
+ S, Arg, Args, APK, format_idx, firstDataArg, Type, CallType,
+ InFunctionCall, CheckedVarArgs, UncoveredArg, Offset,
IgnoreStringsWithoutSpecifiers);
}
}
@@ -8697,9 +8744,8 @@
return SLCT_NotALiteral;
}
FormatStringLiteral FStr(StrE, Offset.sextOrTrunc(64).getSExtValue());
- CheckFormatString(S, &FStr, E, Args, HasVAListArg, format_idx,
- firstDataArg, Type, InFunctionCall, CallType,
- CheckedVarArgs, UncoveredArg,
+ CheckFormatString(S, &FStr, E, Args, APK, format_idx, firstDataArg, Type,
+ InFunctionCall, CallType, CheckedVarArgs, UncoveredArg,
IgnoreStringsWithoutSpecifiers);
return SLCT_CheckedLiteral;
}
@@ -8758,7 +8804,7 @@
default:
return SLCT_NotALiteral;
}
-}
+ }
Sema::FormatStringType Sema::GetFormatStringType(const FormatAttr *Format) {
return llvm::StringSwitch<FormatStringType>(Format->getType()->getName())
@@ -8778,24 +8824,25 @@
/// functions) for correct use of format strings.
/// Returns true if a format string has been fully checked.
bool Sema::CheckFormatArguments(const FormatAttr *Format,
- ArrayRef<const Expr *> Args,
- bool IsCXXMember,
- VariadicCallType CallType,
- SourceLocation Loc, SourceRange Range,
+ ArrayRef<const Expr *> Args, bool IsCXXMember,
+ VariadicCallType CallType, SourceLocation Loc,
+ SourceRange Range,
llvm::SmallBitVector &CheckedVarArgs) {
FormatStringInfo FSI;
- if (getFormatStringInfo(Format, IsCXXMember, &FSI))
- return CheckFormatArguments(Args, FSI.HasVAListArg, FSI.FormatIdx,
+ if (getFormatStringInfo(Format, IsCXXMember, CallType != VariadicDoesNotApply,
+ &FSI))
+ return CheckFormatArguments(Args, FSI.ArgPassingKind, FSI.FormatIdx,
FSI.FirstDataArg, GetFormatStringType(Format),
CallType, Loc, Range, CheckedVarArgs);
return false;
}
bool Sema::CheckFormatArguments(ArrayRef<const Expr *> Args,
- bool HasVAListArg, unsigned format_idx,
- unsigned firstDataArg, FormatStringType Type,
- VariadicCallType CallType,
- SourceLocation Loc, SourceRange Range,
+ Sema::FormatArgumentPassingKind APK,
+ unsigned format_idx, unsigned firstDataArg,
+ FormatStringType Type,
+ VariadicCallType CallType, SourceLocation Loc,
+ SourceRange Range,
llvm::SmallBitVector &CheckedVarArgs) {
// CHECK: printf/scanf-like function is called with no format string.
if (format_idx >= Args.size()) {
@@ -8818,12 +8865,11 @@
// ObjC string uses the same format specifiers as C string, so we can use
// the same format string checking logic for both ObjC and C strings.
UncoveredArgHandler UncoveredArg;
- StringLiteralCheckType CT =
- checkFormatStringExpr(*this, OrigFormatExpr, Args, HasVAListArg,
- format_idx, firstDataArg, Type, CallType,
- /*IsFunctionCall*/ true, CheckedVarArgs,
- UncoveredArg,
- /*no string offset*/ llvm::APSInt(64, false) = 0);
+ StringLiteralCheckType CT = checkFormatStringExpr(
+ *this, OrigFormatExpr, Args, APK, format_idx, firstDataArg, Type,
+ CallType,
+ /*IsFunctionCall*/ true, CheckedVarArgs, UncoveredArg,
+ /*no string offset*/ llvm::APSInt(64, false) = 0);
// Generate a diagnostic where an uncovered argument is detected.
if (UncoveredArg.hasUncoveredArg()) {
@@ -8886,7 +8932,7 @@
const unsigned FirstDataArg;
const unsigned NumDataArgs;
const char *Beg; // Start of format string.
- const bool HasVAListArg;
+ const Sema::FormatArgumentPassingKind ArgPassingKind;
ArrayRef<const Expr *> Args;
unsigned FormatIdx;
llvm::SmallBitVector CoveredArgs;
@@ -8901,14 +8947,15 @@
CheckFormatHandler(Sema &s, const FormatStringLiteral *fexpr,
const Expr *origFormatExpr,
const Sema::FormatStringType type, unsigned firstDataArg,
- unsigned numDataArgs, const char *beg, bool hasVAListArg,
+ unsigned numDataArgs, const char *beg,
+ Sema::FormatArgumentPassingKind APK,
ArrayRef<const Expr *> Args, unsigned formatIdx,
bool inFunctionCall, Sema::VariadicCallType callType,
llvm::SmallBitVector &CheckedVarArgs,
UncoveredArgHandler &UncoveredArg)
: S(s), FExpr(fexpr), OrigFormatExpr(origFormatExpr), FSType(type),
FirstDataArg(firstDataArg), NumDataArgs(numDataArgs), Beg(beg),
- HasVAListArg(hasVAListArg), Args(Args), FormatIdx(formatIdx),
+ ArgPassingKind(APK), Args(Args), FormatIdx(formatIdx),
inFunctionCall(inFunctionCall), CallType(callType),
CheckedVarArgs(CheckedVarArgs), UncoveredArg(UncoveredArg) {
CoveredArgs.resize(numDataArgs);
@@ -9144,8 +9191,8 @@
void CheckFormatHandler::DoneProcessing() {
// Does the number of data arguments exceed the number of
// format conversions in the format string?
- if (!HasVAListArg) {
- // Find any arguments that weren't covered.
+ if (ArgPassingKind != Sema::FAPK_VAList) {
+ // Find any arguments that weren't covered.
CoveredArgs.flip();
signed notCoveredArg = CoveredArgs.find_first();
if (notCoveredArg >= 0) {
@@ -9340,13 +9387,13 @@
const Expr *origFormatExpr,
const Sema::FormatStringType type, unsigned firstDataArg,
unsigned numDataArgs, bool isObjC, const char *beg,
- bool hasVAListArg, ArrayRef<const Expr *> Args,
- unsigned formatIdx, bool inFunctionCall,
- Sema::VariadicCallType CallType,
+ Sema::FormatArgumentPassingKind APK,
+ ArrayRef<const Expr *> Args, unsigned formatIdx,
+ bool inFunctionCall, Sema::VariadicCallType CallType,
llvm::SmallBitVector &CheckedVarArgs,
UncoveredArgHandler &UncoveredArg)
: CheckFormatHandler(s, fexpr, origFormatExpr, type, firstDataArg,
- numDataArgs, beg, hasVAListArg, Args, formatIdx,
+ numDataArgs, beg, APK, Args, formatIdx,
inFunctionCall, CallType, CheckedVarArgs,
UncoveredArg) {}
@@ -9421,17 +9468,16 @@
}
bool CheckPrintfHandler::HandleAmount(
- const analyze_format_string::OptionalAmount &Amt,
- unsigned k, const char *startSpecifier,
- unsigned specifierLen) {
+ const analyze_format_string::OptionalAmount &Amt, unsigned k,
+ const char *startSpecifier, unsigned specifierLen) {
if (Amt.hasDataArgument()) {
- if (!HasVAListArg) {
+ if (ArgPassingKind != Sema::FAPK_VAList) {
unsigned argIndex = Amt.getArgIndex();
if (argIndex >= NumDataArgs) {
EmitFormatDiagnostic(S.PDiag(diag::warn_printf_asterisk_missing_arg)
- << k,
+ << k,
getLocationOfByte(Amt.getStart()),
- /*IsStringLocation*/true,
+ /*IsStringLocation*/ true,
getSpecifierRange(startSpecifier, specifierLen));
// Don't do any more checking. We will just emit
// spurious errors.
@@ -9827,7 +9873,7 @@
HandleNonStandardConversionSpecifier(CS, startSpecifier, specifierLen);
// The remaining checks depend on the data arguments.
- if (HasVAListArg)
+ if (ArgPassingKind == Sema::FAPK_VAList)
return true;
if (!CheckNumArgs(FS, CS, startSpecifier, specifierLen, argIndex))
@@ -9975,6 +10021,12 @@
ExprTy = TET->getUnderlyingExpr()->getType();
}
+ // When using the format attribute in C++, you can receive a function or an
+ // array that will necessarily decay to a pointer when passed to the final
+ // format consumer. Apply decay before type comparison.
+ if (S.Context.getAsArrayType(ExprTy) || ExprTy->isFunctionType())
+ ExprTy = S.Context.getDecayedType(ExprTy);
+
// Diagnose attempts to print a boolean value as a character. Unlike other
// -Wformat diagnostics, this is fine from a type perspective, but it still
// doesn't make sense.
@@ -10195,6 +10247,7 @@
// Since the warning for passing non-POD types to variadic functions
// was deferred until now, we emit a warning for non-POD
// arguments here.
+ bool EmitTypeMismatch = false;
switch (S.isValidVarArgType(ExprTy)) {
case Sema::VAK_Valid:
case Sema::VAK_ValidInCXX11: {
@@ -10220,17 +10273,23 @@
}
case Sema::VAK_Undefined:
case Sema::VAK_MSVCUndefined:
- EmitFormatDiagnostic(S.PDiag(diag::warn_non_pod_vararg_with_format_string)
- << S.getLangOpts().CPlusPlus11 << ExprTy
- << CallType
- << AT.getRepresentativeTypeName(S.Context) << CSR
- << E->getSourceRange(),
- E->getBeginLoc(), /*IsStringLocation*/ false, CSR);
- checkForCStrMembers(AT, E);
+ if (CallType == Sema::VariadicDoesNotApply) {
+ EmitTypeMismatch = true;
+ } else {
+ EmitFormatDiagnostic(
+ S.PDiag(diag::warn_non_pod_vararg_with_format_string)
+ << S.getLangOpts().CPlusPlus11 << ExprTy << CallType
+ << AT.getRepresentativeTypeName(S.Context) << CSR
+ << E->getSourceRange(),
+ E->getBeginLoc(), /*IsStringLocation*/ false, CSR);
+ checkForCStrMembers(AT, E);
+ }
break;
case Sema::VAK_Invalid:
- if (ExprTy->isObjCObjectType())
+ if (CallType == Sema::VariadicDoesNotApply)
+ EmitTypeMismatch = true;
+ else if (ExprTy->isObjCObjectType())
EmitFormatDiagnostic(
S.PDiag(diag::err_cannot_pass_objc_interface_to_vararg_format)
<< S.getLangOpts().CPlusPlus11 << ExprTy << CallType
@@ -10246,6 +10305,19 @@
break;
}
+ if (EmitTypeMismatch) {
+ // The function is not variadic, so we do not generate warnings about
+ // being allowed to pass that object as a variadic argument. Instead,
+ // since there are inherently no printf specifiers for types which cannot
+ // be passed as variadic arguments, emit a plain old specifier mismatch
+ // argument.
+ EmitFormatDiagnostic(
+ S.PDiag(diag::warn_format_conversion_argument_type_mismatch)
+ << AT.getRepresentativeTypeName(S.Context) << ExprTy << false
+ << E->getSourceRange(),
+ E->getBeginLoc(), false, CSR);
+ }
+
assert(FirstDataArg + FS.getArgIndex() < CheckedVarArgs.size() &&
"format string specifier index out of range");
CheckedVarArgs[FirstDataArg + FS.getArgIndex()] = true;
@@ -10263,13 +10335,13 @@
CheckScanfHandler(Sema &s, const FormatStringLiteral *fexpr,
const Expr *origFormatExpr, Sema::FormatStringType type,
unsigned firstDataArg, unsigned numDataArgs,
- const char *beg, bool hasVAListArg,
+ const char *beg, Sema::FormatArgumentPassingKind APK,
ArrayRef<const Expr *> Args, unsigned formatIdx,
bool inFunctionCall, Sema::VariadicCallType CallType,
llvm::SmallBitVector &CheckedVarArgs,
UncoveredArgHandler &UncoveredArg)
: CheckFormatHandler(s, fexpr, origFormatExpr, type, firstDataArg,
- numDataArgs, beg, hasVAListArg, Args, formatIdx,
+ numDataArgs, beg, APK, Args, formatIdx,
inFunctionCall, CallType, CheckedVarArgs,
UncoveredArg) {}
@@ -10373,7 +10445,7 @@
HandleNonStandardConversionSpecifier(CS, startSpecifier, specifierLen);
// The remaining checks depend on the data arguments.
- if (HasVAListArg)
+ if (ArgPassingKind == Sema::FAPK_VAList)
return true;
if (!CheckNumArgs(FS, CS, startSpecifier, specifierLen, argIndex))
@@ -10430,17 +10502,13 @@
return true;
}
-static void CheckFormatString(Sema &S, const FormatStringLiteral *FExpr,
- const Expr *OrigFormatExpr,
- ArrayRef<const Expr *> Args,
- bool HasVAListArg, unsigned format_idx,
- unsigned firstDataArg,
- Sema::FormatStringType Type,
- bool inFunctionCall,
- Sema::VariadicCallType CallType,
- llvm::SmallBitVector &CheckedVarArgs,
- UncoveredArgHandler &UncoveredArg,
- bool IgnoreStringsWithoutSpecifiers) {
+static void CheckFormatString(
+ Sema &S, const FormatStringLiteral *FExpr, const Expr *OrigFormatExpr,
+ ArrayRef<const Expr *> Args, Sema::FormatArgumentPassingKind APK,
+ unsigned format_idx, unsigned firstDataArg, Sema::FormatStringType Type,
+ bool inFunctionCall, Sema::VariadicCallType CallType,
+ llvm::SmallBitVector &CheckedVarArgs, UncoveredArgHandler &UncoveredArg,
+ bool IgnoreStringsWithoutSpecifiers) {
// CHECK: is the format string a wide literal?
if (!FExpr->isAscii() && !FExpr->isUTF8()) {
CheckFormatHandler::EmitFormatDiagnostic(
@@ -10491,23 +10559,21 @@
Type == Sema::FST_OSTrace) {
CheckPrintfHandler H(
S, FExpr, OrigFormatExpr, Type, firstDataArg, numDataArgs,
- (Type == Sema::FST_NSString || Type == Sema::FST_OSTrace), Str,
- HasVAListArg, Args, format_idx, inFunctionCall, CallType,
- CheckedVarArgs, UncoveredArg);
-
- if (!analyze_format_string::ParsePrintfString(H, Str, Str + StrLen,
- S.getLangOpts(),
- S.Context.getTargetInfo(),
- Type == Sema::FST_FreeBSDKPrintf))
+ (Type == Sema::FST_NSString || Type == Sema::FST_OSTrace), Str, APK,
+ Args, format_idx, inFunctionCall, CallType, CheckedVarArgs,
+ UncoveredArg);
+
+ if (!analyze_format_string::ParsePrintfString(
+ H, Str, Str + StrLen, S.getLangOpts(), S.Context.getTargetInfo(),
+ Type == Sema::FST_FreeBSDKPrintf))
H.DoneProcessing();
} else if (Type == Sema::FST_Scanf) {
CheckScanfHandler H(S, FExpr, OrigFormatExpr, Type, firstDataArg,
- numDataArgs, Str, HasVAListArg, Args, format_idx,
- inFunctionCall, CallType, CheckedVarArgs, UncoveredArg);
+ numDataArgs, Str, APK, Args, format_idx, inFunctionCall,
+ CallType, CheckedVarArgs, UncoveredArg);
- if (!analyze_format_string::ParseScanfString(H, Str, Str + StrLen,
- S.getLangOpts(),
- S.Context.getTargetInfo()))
+ if (!analyze_format_string::ParseScanfString(
+ H, Str, Str + StrLen, S.getLangOpts(), S.Context.getTargetInfo()))
H.DoneProcessing();
} // TODO: handle other formats
}
Index: clang/lib/AST/FormatString.cpp
===================================================================
--- clang/lib/AST/FormatString.cpp
+++ clang/lib/AST/FormatString.cpp
@@ -321,6 +321,12 @@
clang::analyze_format_string::ArgType::MatchKind
ArgType::matchesType(ASTContext &C, QualType argTy) const {
+ // When using the format attribute in C++, you can receive a function or an
+ // array that will necessarily decay to a pointer when passed to the final
+ // format consumer. Apply decay before type comparison.
+ if (C.getAsArrayType(argTy) || argTy->isFunctionType())
+ argTy = C.getDecayedType(argTy);
+
if (Ptr) {
// It has to be a pointer.
const PointerType *PT = argTy->getAs<PointerType>();
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -12853,21 +12853,29 @@
SourceLocation getLocationOfStringLiteralByte(const StringLiteral *SL,
unsigned ByteNo) const;
-private:
- void CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr,
- const ArraySubscriptExpr *ASE=nullptr,
- bool AllowOnePastEnd=true, bool IndexNegated=false);
- void CheckArrayAccess(const Expr *E);
+ enum FormatArgumentPassingKind {
+ FAPK_Fixed, // values to format are fixed (no C-style variadic arguments)
+ FAPK_Variadic, // values to format are passed as variadic arguments
+ FAPK_VAList, // values to format are passed in a va_list
+ };
+
// Used to grab the relevant information from a FormatAttr and a
// FunctionDeclaration.
struct FormatStringInfo {
unsigned FormatIdx;
unsigned FirstDataArg;
- bool HasVAListArg;
+ FormatArgumentPassingKind ArgPassingKind;
};
static bool getFormatStringInfo(const FormatAttr *Format, bool IsCXXMember,
- FormatStringInfo *FSI);
+ bool IsVariadic, FormatStringInfo *FSI);
+
+private:
+ void CheckArrayAccess(const Expr *BaseExpr, const Expr *IndexExpr,
+ const ArraySubscriptExpr *ASE = nullptr,
+ bool AllowOnePastEnd = true, bool IndexNegated = false);
+ void CheckArrayAccess(const Expr *E);
+
bool CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall,
const FunctionProtoType *Proto);
bool CheckObjCMethodCall(ObjCMethodDecl *Method, SourceLocation loc,
@@ -13022,16 +13030,15 @@
private:
bool CheckFormatArguments(const FormatAttr *Format,
- ArrayRef<const Expr *> Args,
- bool IsCXXMember,
- VariadicCallType CallType,
- SourceLocation Loc, SourceRange Range,
+ ArrayRef<const Expr *> Args, bool IsCXXMember,
+ VariadicCallType CallType, SourceLocation Loc,
+ SourceRange Range,
llvm::SmallBitVector &CheckedVarArgs);
bool CheckFormatArguments(ArrayRef<const Expr *> Args,
- bool HasVAListArg, unsigned format_idx,
+ FormatArgumentPassingKind FAPK, unsigned format_idx,
unsigned firstDataArg, FormatStringType Type,
- VariadicCallType CallType,
- SourceLocation Loc, SourceRange range,
+ VariadicCallType CallType, SourceLocation Loc,
+ SourceRange range,
llvm::SmallBitVector &CheckedVarArgs);
void CheckAbsoluteValueFunction(const CallExpr *Call,
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3113,8 +3113,6 @@
"declared with index %0 here">;
def err_format_strftime_third_parameter : Error<
"strftime format attribute requires 3rd parameter to be 0">;
-def err_format_attribute_requires_variadic : Error<
- "format attribute requires variadic function">;
def err_format_attribute_not : Error<"format argument not a string type">;
def err_format_attribute_result_not : Error<"function does not return %0">;
def err_format_attribute_implicit_this_format_string : Error<
@@ -4124,6 +4122,9 @@
def warn_gcc_ignores_type_attr : Warning<
"GCC does not allow the %0 attribute to be written on a type">,
InGroup<GccCompat>;
+def warn_gcc_requires_variadic_function : Warning<
+ "GCC requires functions with the %0 attribute to be variadic">,
+ InGroup<GccCompat>;
// Clang-Specific Attributes
def warn_attribute_iboutlet : Warning<
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
