diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 3dc03fa..788bb12 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -4681,6 +4681,11 @@ def err_cannot_pass_objc_interface_to_vararg : Error< "cannot pass object with interface type %0 by-value through variadic " "%select{function|block|method}1">; +def warn_non_pod_string_passed_to_printf : Warning< + "cannot pass non-POD object of type %0 to variadic function; " + "expected type was %1">, + InGroup>, DefaultError; + def warn_cannot_pass_non_pod_arg_to_vararg : Warning< "cannot pass object of %select{non-POD|non-trivial}0 type %1 through variadic" " %select{function|block|method|constructor}2; call will abort at runtime">, @@ -5161,7 +5166,8 @@ def warn_scanf_scanlist_incomplete : Warning< "no closing ']' for '%%[' in scanf format string">, InGroup; def note_format_string_defined : Note<"format string is defined here">; - +def note_printf_c_str: Note< "did you mean to call .c_str()?">; + def warn_null_arg : Warning< "null passed to a callee which requires a non-null argument">, InGroup; diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index f0d3213..affab04 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -6409,7 +6409,8 @@ public: // DefaultVariadicArgumentPromotion - Like DefaultArgumentPromotion, but // will warn if the resulting type is not a POD type. ExprResult DefaultVariadicArgumentPromotion(Expr *E, VariadicCallType CT, - FunctionDecl *FDecl); + FunctionDecl *FDecl, + bool DeferErrors = false); // UsualArithmeticConversions - performs the UsualUnaryConversions on it's // operands and then handles various conversions that are common to binary @@ -6985,6 +6986,7 @@ private: const ArraySubscriptExpr *ASE=0, bool AllowOnePastEnd=true, bool IndexNegated=false); void CheckArrayAccess(const Expr *E); + bool IsPrintfLike(FunctionDecl *FDecl, Expr **Args, unsigned NumArgs); bool CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall); bool CheckObjCMethodCall(ObjCMethodDecl *Method, SourceLocation loc, Expr **Args, unsigned NumArgs); diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index e818f5f..fa73452 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -16,6 +16,7 @@ #include "clang/Sema/Sema.h" #include "clang/Sema/SemaInternal.h" #include "clang/Sema/Initialization.h" +#include "clang/Sema/Lookup.h" #include "clang/Sema/ScopeInfo.h" #include "clang/Analysis/Analyses/FormatString.h" #include "clang/AST/ASTContext.h" @@ -418,6 +419,30 @@ bool Sema::CheckARMBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall) { return false; } +// Given a variadic function, decides whether it looks like we have a +// printf-style format string and arguments) +// This doesn't work for member functions. +bool Sema::IsPrintfLike(FunctionDecl *FDecl, Expr **Args, unsigned NumArgs) { + // Anonymous and lambda functions will be assumed not to be printf. + if (!FDecl) + return false; + + for (specific_attr_iterator + i = FDecl->specific_attr_begin(), + e = FDecl->specific_attr_end(); i != e ; ++i) { + const FormatAttr *Format = *i; + unsigned format_idx = Format->getFormatIdx() - 1; + + if (format_idx >= NumArgs) { + // There must be some other format + continue; + } + + return true; + } + return false; +} + /// CheckFunctionCall - Check a direct function call for various correctness /// and safety properties not strictly enforced by the C type system. bool Sema::CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall) { @@ -2138,7 +2163,11 @@ public: bool HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier &FS, const char *startSpecifier, unsigned specifierLen); - + bool CheckFormatExpr(const analyze_printf::PrintfSpecifier &FS, + const char *startSpecifier, + unsigned specifierLen, + const Expr *Ex); + bool HandleAmount(const analyze_format_string::OptionalAmount &Amt, unsigned k, const char *startSpecifier, unsigned specifierLen); void HandleInvalidAmount(const analyze_printf::PrintfSpecifier &FS, @@ -2152,6 +2181,9 @@ public: const analyze_printf::OptionalFlag &ignoredFlag, const analyze_printf::OptionalFlag &flag, const char *startSpecifier, unsigned specifierLen); + bool CheckForCStrMembers(const analyze_printf::ArgTypeResult &ATR, + const Expr *Ex, const CharSourceRange &CSR); + }; } @@ -2269,12 +2301,70 @@ void CheckPrintfHandler::HandleIgnoredFlag( getSpecifierRange(ignoredFlag.getPosition(), 1))); } +// Determines if the specified is a C++ class or struct containing +// a member with the specified name and kind (e.g. a CXXMethodDecl named +// "c_str()" +template +static llvm::SmallPtrSet +CXXRecordMembersNamed(StringRef Name, Sema &S, QualType Ty) { + const RecordType *RT = Ty->getAs(); + llvm::SmallPtrSet Results; + + if (!RT) + return Results; + const CXXRecordDecl *RD = dyn_cast(RT->getDecl()); + if (!RD) + return Results; + + LookupResult R(S, &S.PP.getIdentifierTable().get(Name), SourceLocation(), + Sema::LookupMemberName); + + // We just need to include all members of the right kind turned up by the + // filter, at this point. + if (S.LookupQualifiedName(R, RT->getDecl())) { + LookupResult::Filter filter = R.makeFilter(); + while (filter.hasNext()) { + NamedDecl *decl = filter.next()->getUnderlyingDecl(); + if (FieldKind *FK = dyn_cast(decl)) { + Results.insert(FK); + } + } + filter.done(); + } + return Results; +} + +// Check if a (w)string was passed when a (w)char* was needed, and offer a +// better diagnostic if so. ATR is assumed to be valid. +// Returns true when a c_str() conversion method is found. +bool +CheckPrintfHandler::CheckForCStrMembers( + const analyze_printf::ArgTypeResult &ATR, const Expr *Ex, + const CharSourceRange &CSR) { + typedef llvm::SmallPtrSet MethodSet; + MethodSet Results = + CXXRecordMembersNamed("c_str", S, Ex->getType()); + + for (MethodSet::iterator it = Results.begin(), end = Results.end(); + it != end; ++it) { + const CXXMethodDecl *Method = (*it); + if (Method->getNumParams() == 0 && + ATR.matchesType(S.Context, Method->getResultType())) { + + S.Diag(Ex->getLocStart(), diag::note_printf_c_str) + << FixItHint::CreateInsertion(Ex->getLocEnd(), ".c_str()"); + return true; + } + } + + return false; +} + bool CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier &FS, const char *startSpecifier, unsigned specifierLen) { - using namespace analyze_format_string; using namespace analyze_printf; const PrintfConversionSpecifier &CS = FS.getConversionSpecifier(); @@ -2396,9 +2486,19 @@ CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier if (!CheckNumArgs(FS, CS, startSpecifier, specifierLen, argIndex)) return false; + return CheckFormatExpr(FS, startSpecifier, specifierLen, + getDataArg(argIndex)); +} + +bool +CheckPrintfHandler::CheckFormatExpr(const analyze_printf::PrintfSpecifier &FS, + const char *startSpecifier, + unsigned specifierLen, + const Expr *Ex) { + using namespace analyze_format_string; + using namespace analyze_printf; // Now type check the data expression that matches the // format specifier. - const Expr *Ex = getDataArg(argIndex); const analyze_printf::ArgTypeResult &ATR = FS.getArgType(S.Context, ObjCContext); if (ATR.isValid() && !ATR.matchesType(S.Context, Ex->getType())) { @@ -2446,14 +2546,31 @@ CheckPrintfHandler::HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier os.str())); } else { - EmitFormatDiagnostic( - S.PDiag(diag::warn_printf_conversion_argument_type_mismatch) - << ATR.getRepresentativeTypeName(S.Context) << Ex->getType() - << getSpecifierRange(startSpecifier, specifierLen) - << Ex->getSourceRange(), - Ex->getLocStart(), - /*IsStringLocation*/false, - getSpecifierRange(startSpecifier, specifierLen)); + const CharSourceRange &CSR = getSpecifierRange(startSpecifier, + specifierLen); + // This should really be identical to the checks in SemaExpr.cpp. + // Since the warning for passing non-POD types to variadic functions + // was deferred until now. + if (!Ex->getType()->isIncompleteType() && + !Ex->getType().isCXX98PODType(S.Context)) { + EmitFormatDiagnostic( + S.PDiag(diag::warn_non_pod_string_passed_to_printf) + << Ex->getType() + << ATR.getRepresentativeTypeName(S.Context) + << CSR + << Ex->getSourceRange(), + Ex->getLocStart(), /*IsStringLocation*/false, CSR); + + CheckForCStrMembers(ATR, Ex, CSR); + } + else { + EmitFormatDiagnostic( + S.PDiag(diag::warn_printf_conversion_argument_type_mismatch) + << ATR.getRepresentativeTypeName(S.Context) << Ex->getType() + << CSR + << Ex->getSourceRange(), + Ex->getLocStart(), /*IsStringLocation*/false, CSR); + } } } diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 9f0378a..1b3902d 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -528,7 +528,8 @@ ExprResult Sema::DefaultArgumentPromotion(Expr *E) { /// will warn if the resulting type is not a POD type, and rejects ObjC /// interfaces passed by value. ExprResult Sema::DefaultVariadicArgumentPromotion(Expr *E, VariadicCallType CT, - FunctionDecl *FDecl) { + FunctionDecl *FDecl, + bool DeferErrors) { if (const BuiltinType *PlaceholderTy = E->getType()->getAsPlaceholderType()) { // Strip the unbridged-cast placeholder expression off, if applicable. if (PlaceholderTy->getKind() == BuiltinType::ARCUnbridgedCast && @@ -585,9 +586,14 @@ ExprResult Sema::DefaultVariadicArgumentPromotion(Expr *E, VariadicCallType CT, getLangOpts().ObjCAutoRefCount && E->getType()->isObjCLifetimeType()) TrivialEnough = true; - + if (TrivialEnough) { // Nothing to diagnose. This is okay. + } else if (DeferErrors) { + // In case the function looks sufficiently like printf, + // try to fix non-POD arguments (e.g. an std::string passed rather than + // a char *). + // This is handled in SemaChecking, so we skip the error here. } else if (DiagRuntimeBehavior(E->getLocStart(), 0, PDiag(diag::warn_cannot_pass_non_pod_arg_to_vararg) << getLangOpts().CPlusPlus0x << E->getType() @@ -3532,6 +3538,9 @@ bool Sema::GatherArgumentsForCall(SourceLocation CallLoc, // If this is a variadic call, handle args passed through "...". if (CallType != VariadicDoesNotApply) { + // Decide if we're probably inspecting a printf-like function + + bool DeferErrors = IsPrintfLike(FDecl, Args, NumArgs); // Assume that extern "C" functions with variadic arguments that // return __unknown_anytype aren't *really* variadic. @@ -3542,7 +3551,8 @@ bool Sema::GatherArgumentsForCall(SourceLocation CallLoc, if (isa(Args[i]->IgnoreParens())) arg = DefaultFunctionArrayLvalueConversion(Args[i]); else - arg = DefaultVariadicArgumentPromotion(Args[i], CallType, FDecl); + arg = DefaultVariadicArgumentPromotion(Args[i], CallType, FDecl, + DeferErrors); Invalid |= arg.isInvalid(); AllArgs.push_back(arg.take()); } @@ -3551,7 +3561,7 @@ bool Sema::GatherArgumentsForCall(SourceLocation CallLoc, } else { for (unsigned i = ArgIx; i != NumArgs; ++i) { ExprResult Arg = DefaultVariadicArgumentPromotion(Args[i], CallType, - FDecl); + FDecl, DeferErrors); Invalid |= Arg.isInvalid(); AllArgs.push_back(Arg.take()); } diff --git a/test/SemaCXX/printf-cstr.cpp b/test/SemaCXX/printf-cstr.cpp new file mode 100644 index 0000000..32a6b46 --- /dev/null +++ b/test/SemaCXX/printf-cstr.cpp @@ -0,0 +1,30 @@ +// RUN: %clang_cc1 -Wall -fsyntax-only -Wformat -verify %s + +#include + +extern "C" { +extern int printf(const char *restrict, ...); +} + +class HasCStr { + const char *str; + public: + HasCStr(const char *s): str(s) { } + const char *c_str() {return str;} +}; + +class HasNoCStr { + const char *str; + public: + HasNoCStr(const char *s): str(s) { } + const char *not_c_str() {return str;} +}; + +void f() { + char str[] = "test"; + HasCStr hcs(str); + HasNoCStr hncs(str); + printf("%d: %s\n", 10, hcs.c_str()); + printf("%d: %s\n", 10, hcs); // expected-error{{cannot pass non-POD object of type 'HasCStr' to variadic function; expected type was 'char *'}} expected-note{{did you mean to call .c_str()?}} + printf("%d: %s\n", 10, hncs); // expected-error{{cannot pass non-POD object of type 'HasNoCStr' to variadic function; expected type was 'char *'}} +}