diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 28ae404..5e05eb0 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -6257,8 +6257,6 @@ private: Expr **Args, unsigned NumArgs); bool CheckBlockCall(NamedDecl *NDecl, CallExpr *TheCall); - bool CheckablePrintfAttr(const FormatAttr *Format, Expr **Args, - unsigned NumArgs, bool IsCXXMemberCall); bool CheckObjCString(Expr *Arg); ExprResult CheckBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall); @@ -6284,13 +6282,13 @@ private: bool SemaCheckStringLiteral(const Expr *E, Expr **Args, unsigned NumArgs, bool HasVAListArg, unsigned format_idx, - unsigned firstDataArg, bool isPrintf, + unsigned firstDataArg, StringRef Type, bool inFunctionCall = true); void CheckFormatString(const StringLiteral *FExpr, const Expr *OrigFormatExpr, Expr **Args, unsigned NumArgs, bool HasVAListArg, unsigned format_idx, unsigned firstDataArg, - bool isPrintf, bool inFunctionCall); + StringRef Type, bool inFunctionCall); void CheckNonNullArguments(const NonNullAttr *NonNull, const Expr * const *ExprArgs, @@ -6302,7 +6300,7 @@ private: SourceLocation Loc, SourceRange Range); void CheckPrintfScanfArguments(Expr **Args, unsigned NumArgs, bool HasVAListArg, unsigned format_idx, - unsigned firstDataArg, bool isPrintf, + unsigned firstDataArg, StringRef Type, SourceLocation Loc, SourceRange range); void CheckMemaccessArguments(const CallExpr *Call, diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index dbb0a0e..4c2b8e6 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -45,31 +45,6 @@ SourceLocation Sema::getLocationOfStringLiteralByte(const StringLiteral *SL, PP.getLangOptions(), PP.getTargetInfo()); } -bool Sema::CheckablePrintfAttr(const FormatAttr *Format, Expr **Args, - unsigned NumArgs, bool IsCXXMemberCall) { - StringRef Type = Format->getType(); - // FIXME: add support for "CFString" Type. They are not string literal though, - // so they need special handling. - if (Type == "printf" || Type == "NSString") return true; - if (Type == "printf0") { - // printf0 allows null "format" string; if so don't check format/args - unsigned format_idx = Format->getFormatIdx() - 1; - // Does the index refer to the implicit object argument? - if (IsCXXMemberCall) { - if (format_idx == 0) - return false; - --format_idx; - } - if (format_idx < NumArgs) { - Expr *Format = Args[format_idx]->IgnoreParenCasts(); - if (!Format->isNullPointerConstant(Context, - Expr::NPC_ValueDependentIsNull)) - return true; - } - } - return false; -} - /// Checks that a call expression's argument count is the desired number. /// This is useful when doing custom type-checking. Returns true on error. static bool checkArgCount(Sema &S, CallExpr *call, unsigned desiredArgCount) { @@ -1383,23 +1358,23 @@ bool Sema::SemaBuiltinLongjmp(CallExpr *TheCall) { bool Sema::SemaCheckStringLiteral(const Expr *E, Expr **Args, unsigned NumArgs, bool HasVAListArg, unsigned format_idx, unsigned firstDataArg, - bool isPrintf, bool inFunctionCall) { + StringRef Type, bool inFunctionCall) { tryAgain: if (E->isTypeDependent() || E->isValueDependent()) return false; - E = E->IgnoreParens(); + E = E->IgnoreParenCasts(); switch (E->getStmtClass()) { case Stmt::BinaryConditionalOperatorClass: case Stmt::ConditionalOperatorClass: { const AbstractConditionalOperator *C = cast(E); return SemaCheckStringLiteral(C->getTrueExpr(), Args, NumArgs, HasVAListArg, - format_idx, firstDataArg, isPrintf, + format_idx, firstDataArg, Type, inFunctionCall) - && SemaCheckStringLiteral(C->getFalseExpr(), Args, NumArgs, HasVAListArg, - format_idx, firstDataArg, isPrintf, - inFunctionCall); + && SemaCheckStringLiteral(C->getFalseExpr(), Args, NumArgs, HasVAListArg, + format_idx, firstDataArg, Type, + inFunctionCall); } case Stmt::IntegerLiteralClass: @@ -1447,7 +1422,7 @@ bool Sema::SemaCheckStringLiteral(const Expr *E, Expr **Args, if (const Expr *Init = VD->getAnyInitializer()) return SemaCheckStringLiteral(Init, Args, NumArgs, HasVAListArg, format_idx, firstDataArg, - isPrintf, /*inFunctionCall*/false); + Type, /*inFunctionCall*/false); } // For vprintf* functions (i.e., HasVAListArg==true), we add a @@ -1487,7 +1462,7 @@ bool Sema::SemaCheckStringLiteral(const Expr *E, Expr **Args, const Expr *Arg = CE->getArg(ArgIndex - 1); return SemaCheckStringLiteral(Arg, Args, NumArgs, HasVAListArg, - format_idx, firstDataArg, isPrintf, + format_idx, firstDataArg, Type, inFunctionCall); } } @@ -1507,7 +1482,7 @@ bool Sema::SemaCheckStringLiteral(const Expr *E, Expr **Args, if (StrE) { CheckFormatString(StrE, E, Args, NumArgs, HasVAListArg, format_idx, - firstDataArg, isPrintf, inFunctionCall); + firstDataArg, Type, inFunctionCall); return true; } @@ -1553,26 +1528,23 @@ void Sema::CheckFormatArguments(const FormatAttr *Format, CallExpr *TheCall) { void Sema::CheckFormatArguments(const FormatAttr *Format, Expr **Args, unsigned NumArgs, bool IsCXXMember, SourceLocation Loc, SourceRange Range) { - const bool b = Format->getType() == "scanf"; - if (b || CheckablePrintfAttr(Format, Args, NumArgs, IsCXXMember)) { - bool HasVAListArg = Format->getFirstArg() == 0; - unsigned format_idx = Format->getFormatIdx() - 1; - unsigned firstDataArg = HasVAListArg ? 0 : Format->getFirstArg() - 1; - if (IsCXXMember) { - if (format_idx == 0) - return; - --format_idx; - if(firstDataArg != 0) - --firstDataArg; - } - CheckPrintfScanfArguments(Args, NumArgs, HasVAListArg, format_idx, - firstDataArg, !b, Loc, Range); + bool HasVAListArg = Format->getFirstArg() == 0; + unsigned format_idx = Format->getFormatIdx() - 1; + unsigned firstDataArg = HasVAListArg ? 0 : Format->getFirstArg() - 1; + if (IsCXXMember) { + if (format_idx == 0) + return; + --format_idx; + if(firstDataArg != 0) + --firstDataArg; } + CheckPrintfScanfArguments(Args, NumArgs, HasVAListArg, format_idx, + firstDataArg, Format->getType(), Loc, Range); } void Sema::CheckPrintfScanfArguments(Expr **Args, unsigned NumArgs, bool HasVAListArg, unsigned format_idx, - unsigned firstDataArg, bool isPrintf, + unsigned firstDataArg, StringRef Type, SourceLocation Loc, SourceRange Range) { // CHECK: printf/scanf-like function is called with no format string. if (format_idx >= NumArgs) { @@ -1595,7 +1567,7 @@ void Sema::CheckPrintfScanfArguments(Expr **Args, unsigned NumArgs, // 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. if (SemaCheckStringLiteral(OrigFormatExpr, Args, NumArgs, HasVAListArg, - format_idx, firstDataArg, isPrintf)) + format_idx, firstDataArg, Type)) return; // Literal format string found, check done! // If there are no arguments specified, warn with -Wformat-security, otherwise @@ -2371,7 +2343,7 @@ void Sema::CheckFormatString(const StringLiteral *FExpr, const Expr *OrigFormatExpr, Expr **Args, unsigned NumArgs, bool HasVAListArg, unsigned format_idx, - unsigned firstDataArg, bool isPrintf, + unsigned firstDataArg, StringRef Type, bool inFunctionCall) { // CHECK: is the format string a wide literal? @@ -2398,6 +2370,7 @@ void Sema::CheckFormatString(const StringLiteral *FExpr, return; } + bool isPrintf = Type == "printf" || Type == "printf0" || Type == "NSString"; if (isPrintf) { CheckPrintfHandler H(*this, FExpr, OrigFormatExpr, firstDataArg, numDataArgs, isa(OrigFormatExpr), @@ -2407,8 +2380,7 @@ void Sema::CheckFormatString(const StringLiteral *FExpr, if (!analyze_format_string::ParsePrintfString(H, Str, Str + StrLen, getLangOptions())) H.DoneProcessing(); - } - else { + } else if (Type == "scanf") { CheckScanfHandler H(*this, FExpr, OrigFormatExpr, firstDataArg, numDataArgs, isa(OrigFormatExpr), Str, HasVAListArg, Args, NumArgs, format_idx, @@ -2417,7 +2389,7 @@ void Sema::CheckFormatString(const StringLiteral *FExpr, if (!analyze_format_string::ParseScanfString(H, Str, Str + StrLen, getLangOptions())) H.DoneProcessing(); - } + } // TODO: handle other formats } //===--- CHECK: Standard memory functions ---------------------------------===// diff --git a/test/Sema/format-strings.c b/test/Sema/format-strings.c index 9daaf3b..341fd59 100644 --- a/test/Sema/format-strings.c +++ b/test/Sema/format-strings.c @@ -480,3 +480,14 @@ void printf_longlong(long long x, unsigned long long y) { printf("%Lx", x); // no-warning printf("%Ls", "hello"); // expected-warning {{length modifier 'L' results in undefined behavior or no effect with 's' conversion specifier}} } + +void __attribute__((format(strfmon,1,2))) monformat(const char *fmt, ...); +void __attribute__((format(strftime,1,0))) dateformat(const char *fmt); + +// Other formats +void test_other_formats() { + char *str = ""; + monformat("", 1); // expected-warning{{format string is empty}} + dateformat(""); // expected-warning{{format string is empty}} + dateformat(str); // expected-warning{{format string is not a string literal (potentially insecure)}} +}