diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -328,6 +328,10 @@ builtins (corresponding to the specific names listed in the attribute) in the body of the function the attribute is on. +- The ``format`` attribute can now be applied to non-variadic functions. The + format string must correctly format the fixed parameter types of the function. + Using the attribute this way emits a GCC compatibility diagnostic. + Windows Support --------------- diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -3088,8 +3088,8 @@ let Content = [{ Clang supports the ``format`` attribute, which indicates that the function -accepts a ``printf`` or ``scanf``-like format string and corresponding -arguments or a ``va_list`` that contains these arguments. +accepts (among other possibilities) a ``printf`` or ``scanf``-like format string +and corresponding arguments or a ``va_list`` that contains these arguments. Please see `GCC documentation about format attribute `_ to find details @@ -3143,6 +3143,27 @@ In this case Clang does not warn because the format string ``s`` and the corresponding arguments are annotated. If the arguments are incorrect, the caller of ``foo`` will receive a warning. + +As an extension to GCC's behavior, Clang accepts the format attribute on +non-variadic functions. Clang checks non-variadic format functions for the same +classes of issues that can be found on variadic functions, as controlled by the +same warning flags, except that the types of formatted arguments is forced by +the function signature. For example: + +.. code-block:: c + + __attribute__((__format__(__printf__, 1, 2))) + void fmt(const char *s, const char *a, int b); + + void bar(void) { + fmt("%s %i", "hello", 123); // OK + fmt("%i %g", "hello", 123); // warning: arguments don't match format + extern const char *fmt; + fmt(fmt, "hello", 123); // warning: format string is not a string literal + } + +Using the format attribute on a non-variadic function emits a GCC compatibility +diagnostic. }]; } diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/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; +def warn_gcc_requires_variadic_function : Warning< + "GCC requires a function with the %0 attribute to be variadic">, + InGroup; // Clang-Specific Attributes def warn_attribute_iboutlet : Warning< diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h --- a/clang/include/clang/Sema/Sema.h +++ b/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 Args, - bool IsCXXMember, - VariadicCallType CallType, - SourceLocation Loc, SourceRange Range, + ArrayRef Args, bool IsCXXMember, + VariadicCallType CallType, SourceLocation Loc, + SourceRange Range, llvm::SmallBitVector &CheckedVarArgs); bool CheckFormatArguments(ArrayRef 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, diff --git a/clang/lib/AST/FormatString.cpp b/clang/lib/AST/FormatString.cpp --- a/clang/lib/AST/FormatString.cpp +++ b/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 (argTy->canDecayToPointerType()) + argTy = C.getDecayedType(argTy); + if (Ptr) { // It has to be a pointer. const PointerType *PT = argTy->getAs(); diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp --- a/clang/lib/Sema/SemaChecking.cpp +++ b/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,16 @@ /// 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 +5494,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 +7728,7 @@ llvm::SmallBitVector CheckedVarArgs(NumArgs, false); ArrayRef 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) @@ -8434,19 +8445,15 @@ SourceLocation getEndLoc() const LLVM_READONLY { return FExpr->getEndLoc(); } }; -} // namespace +} // namespace -static void CheckFormatString(Sema &S, const FormatStringLiteral *FExpr, - const Expr *OrigFormatExpr, - ArrayRef 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 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,16 +8461,15 @@ // True string literals are then checked by CheckFormatString. static StringLiteralCheckType checkFormatStringExpr(Sema &S, const Expr *E, ArrayRef 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; - tryAgain: +tryAgain: assert(Offset.isSigned() && "invalid offset"); if (E->isTypeDependent() || E->isValueDependent()) @@ -8508,9 +8514,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 +8524,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 +8575,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(VD)) { - if (const Decl *D = dyn_cast(PV->getDeclContext())) { - int PVIndex = PV->getFunctionScopeIndex() + 1; - for (const auto *PVFormat : D->specific_attrs()) { - // adjust for implicit parameter - if (const CXXMethodDecl *MD = dyn_cast(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 + // __attribute__((format(printf, 1, 2))) + // void log(const char *fmt, Args&&... args) { + // printf(fmt, forward(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 auto *PV = dyn_cast(VD)) { + if (const auto *D = dyn_cast(PV->getDeclContext())) { + for (const auto *PVFormat : D->specific_attrs()) { + bool IsCXXMember = false; + if (const auto *MD = dyn_cast(D)) + IsCXXMember = MD->isInstance(); + + bool IsVariadic = false; + if (const FunctionType *FnTy = D->getFunctionType()) + IsVariadic = cast(FnTy)->isVariadic(); + else if (const auto *BD = dyn_cast(D)) + IsVariadic = BD->isVariadic(); + else if (const auto *OMD = dyn_cast(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 +8672,8 @@ for (const auto *FA : ND->specific_attrs()) { 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 +8688,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 +8719,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 +8743,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; } @@ -8778,24 +8823,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 Args, - bool IsCXXMember, - VariadicCallType CallType, - SourceLocation Loc, SourceRange Range, + ArrayRef 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 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 +8864,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 +8931,7 @@ const unsigned FirstDataArg; const unsigned NumDataArgs; const char *Beg; // Start of format string. - const bool HasVAListArg; + const Sema::FormatArgumentPassingKind ArgPassingKind; ArrayRef Args; unsigned FormatIdx; llvm::SmallBitVector CoveredArgs; @@ -8901,14 +8946,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 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 +9190,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 +9386,13 @@ const Expr *origFormatExpr, const Sema::FormatStringType type, unsigned firstDataArg, unsigned numDataArgs, bool isObjC, const char *beg, - bool hasVAListArg, ArrayRef Args, - unsigned formatIdx, bool inFunctionCall, - Sema::VariadicCallType CallType, + Sema::FormatArgumentPassingKind APK, + ArrayRef 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 +9467,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 +9872,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 +10020,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 (ExprTy->canDecayToPointerType()) + 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 +10246,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 +10272,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 +10304,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 +10334,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 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 +10444,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 +10501,13 @@ return true; } -static void CheckFormatString(Sema &S, const FormatStringLiteral *FExpr, - const Expr *OrigFormatExpr, - ArrayRef 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 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 +10558,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 } diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -3878,12 +3878,10 @@ // check if the function is variadic if the 3rd argument non-zero if (FirstArg != 0) { - if (isFunctionOrMethodVariadic(D)) { + if (isFunctionOrMethodVariadic(D)) ++NumArgs; // +1 for ... - } else { - S.Diag(D->getLocation(), diag::err_format_attribute_requires_variadic); - return; - } + else + S.Diag(D->getLocation(), diag::warn_gcc_requires_variadic_function) << AL; } // strftime requires FirstArg to be 0 because it doesn't read from any diff --git a/clang/test/Sema/attr-format.c b/clang/test/Sema/attr-format.c --- a/clang/test/Sema/attr-format.c +++ b/clang/test/Sema/attr-format.c @@ -2,18 +2,18 @@ #include -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 a function 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}} +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}} int (*f_ptr)(char*,...) __attribute__((format(printf, 1,2))); // no-error int (*f2_ptr)(double,...) __attribute__((format(printf, 1, 2))); // expected-error {{format argument not a string type}} @@ -33,19 +33,17 @@ 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 a function 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))) -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}} +#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) (void)0/* vprintf(...) would go here */; } @@ -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 a function 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); @@ -77,13 +73,24 @@ } // 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_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}} + +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 a function with the 'format' attribute to be variadic}} + forward_fixed(fmt, i); + a(fmt, i); +} diff --git a/clang/test/Sema/format-strings.c b/clang/test/Sema/format-strings.c --- a/clang/test/Sema/format-strings.c +++ b/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); }; diff --git a/clang/test/SemaCXX/attr-format.cpp b/clang/test/SemaCXX/attr-format.cpp --- a/clang/test/SemaCXX/attr-format.cpp +++ b/clang/test/SemaCXX/attr-format.cpp @@ -1,7 +1,11 @@ // RUN: %clang_cc1 -fsyntax-only -Wformat-nonliteral -verify %s +#include + +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 +void format(const char *fmt, Args &&...args) // expected-warning{{GCC requires a function with the 'format' attribute to be variadic}} + __attribute__((format(printf, 1, 2))); + +template +Arg &expand(Arg &a) { return a; } + +struct foo { + int big[10]; + foo(); + ~foo(); + + template + void format(const char *const fmt, Args &&...args) // expected-warning{{GCC requires a function 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 a function 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}} +}