From c7101188fb3f17176e9152b1d733da6d7199d317 Mon Sep 17 00:00:00 2001 From: apple-fcloutier <75502309+apple-fcloutier@users.noreply.github.com> Date: Mon, 24 Feb 2025 18:58:59 -0800 Subject: [PATCH] [clang] Implement __attribute__((format_matches)) (#116708) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This implements ``__attribute__((format_matches))``, as described in the RFC: https://discourse.llvm.org/t/rfc-format-attribute-attribute-format-like/83076 The ``format`` attribute only allows the compiler to check that a format string matches its arguments. If the format string is passed independently of its arguments, there is no way to have the compiler check it. ``format_matches(flavor, fmtidx, example)`` allows the compiler to check format strings against the ``example`` format string instead of against format arguments. See the changes to AttrDocs.td in this diff for more information. Implementation-wise, this change subclasses CheckPrintfHandler and CheckScanfHandler to allow them to collect specifiers into arrays, and implements comparing that two specifiers are equivalent. `checkFormatStringExpr` gets a new `ReferenceFormatString` argument that is piped down when calling a function with the `format_matches` attribute (and is `nullptr` otherwise); this is the string that the actual format string is compared against. Although this change does not enable -Wformat-nonliteral by default, IMO, all the pieces are now in place such that it could be. --- clang/docs/ReleaseNotes.rst | 44 + clang/include/clang/AST/FormatString.h | 3 +- clang/include/clang/Basic/Attr.td | 10 + clang/include/clang/Basic/AttrDocs.td | 126 +++ .../clang/Basic/DiagnosticSemaKinds.td | 24 + clang/include/clang/Sema/Sema.h | 57 +- clang/lib/AST/AttrImpl.cpp | 4 + clang/lib/AST/FormatString.cpp | 91 ++ clang/lib/Sema/SemaChecking.cpp | 829 +++++++++++++++--- clang/lib/Sema/SemaDecl.cpp | 3 + clang/lib/Sema/SemaDeclAttr.cpp | 122 ++- clang/lib/Sema/SemaObjC.cpp | 3 +- clang/test/Sema/format-string-matches.c | 273 ++++++ clang/test/Sema/format-strings.c | 2 +- 14 files changed, 1411 insertions(+), 180 deletions(-) create mode 100644 clang/test/Sema/format-string-matches.c diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 699cbb17edca7..657340c170503 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -132,6 +132,50 @@ Attribute Changes in Clang This forces the global to be considered small or large in regards to the x86-64 code model, regardless of the code model specified for the compilation. +- There is a new ``format_matches`` attribute to complement the existing + ``format`` attribute. ``format_matches`` allows the compiler to verify that + a format string argument is equivalent to a reference format string: it is + useful when a function accepts a format string without its accompanying + arguments to format. For instance: + + .. code-block:: c + + static int status_code; + static const char *status_string; + + void print_status(const char *fmt) { + fprintf(stderr, fmt, status_code, status_string); + // ^ warning: format string is not a string literal [-Wformat-nonliteral] + } + + void stuff(void) { + print_status("%s (%#08x)\n"); + // order of %s and %x is swapped but there is no diagnostic + } + + Before the introducion of ``format_matches``, this code cannot be verified + at compile-time. ``format_matches`` plugs that hole: + + .. code-block:: c + + __attribute__((format_matches(printf, 1, "%x %s"))) + void print_status(const char *fmt) { + fprintf(stderr, fmt, status_code, status_string); + // ^ `fmt` verified as if it was "%x %s" here; no longer triggers + // -Wformat-nonliteral, would warn if arguments did not match "%x %s" + } + + void stuff(void) { + print_status("%s (%#08x)\n"); + // warning: format specifier 's' is incompatible with 'x' + // warning: format specifier 'x' is incompatible with 's' + } + + Like with ``format``, the first argument is the format string flavor and the + second argument is the index of the format string parameter. + ``format_matches`` accepts an example valid format string as its third + argument. For more information, see the Clang attributes documentation. + Improvements to Clang's diagnostics ----------------------------------- diff --git a/clang/include/clang/AST/FormatString.h b/clang/include/clang/AST/FormatString.h index a074dd23e2ad4..3560766433fe2 100644 --- a/clang/include/clang/AST/FormatString.h +++ b/clang/include/clang/AST/FormatString.h @@ -292,7 +292,7 @@ class ArgType { }; private: - const Kind K; + Kind K; QualType T; const char *Name = nullptr; bool Ptr = false; @@ -338,6 +338,7 @@ class ArgType { } MatchKind matchesType(ASTContext &C, QualType argTy) const; + MatchKind matchesArgType(ASTContext &C, const ArgType &other) const; QualType getRepresentativeType(ASTContext &C) const; diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 65c91ccd75ecc..8bbd096bb1f72 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -1834,6 +1834,16 @@ def Format : InheritableAttr { let Documentation = [FormatDocs]; } +def FormatMatches : InheritableAttr { + let Spellings = [GCC<"format_matches">]; + let Args = [IdentifierArgument<"Type">, IntArgument<"FormatIdx">, ExprArgument<"ExpectedFormat">]; + let AdditionalMembers = [{ + StringLiteral *getFormatString() const; + }]; + let Subjects = SubjectList<[ObjCMethod, Block, HasFunctionProto]>; + let Documentation = [FormatMatchesDocs]; +} + def FormatArg : InheritableAttr { let Spellings = [GCC<"format_arg">]; let Args = [ParamIdxArgument<"FormatIdx">]; diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index f5362b4d59142..79c615be754c4 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -3886,6 +3886,132 @@ behavior of the program is undefined. }]; } +def FormatMatchesDocs : Documentation { + let Category = DocCatFunction; + let Content = [{ + +The ``format`` attribute is the basis for the enforcement of diagnostics in the +``-Wformat`` family, but it only handles the case where the format string is +passed along with the arguments it is going to format. It cannot handle the case +where the format string and the format arguments are passed separately from each +other. For instance: + +.. code-block:: c + + static const char *first_name; + static double todays_temperature; + static int wind_speed; + + void say_hi(const char *fmt) { + printf(fmt, first_name, todays_temperature); + // ^ warning: format string is not a string literal + printf(fmt, first_name, wind_speed); + // ^ warning: format string is not a string literal + } + + int main() { + say_hi("hello %s, it is %g degrees outside"); + say_hi("hello %s, it is %d degrees outside!"); + // ^ no diagnostic, but %d cannot format doubles + } + +In this example, ``fmt`` is expected to format a ``const char *`` and a +``double``, but these values are not passed to ``say_hi``. Without the +``format`` attribute (which cannot apply in this case), the -Wformat-nonliteral +diagnostic unnecessarily triggers in the body of ``say_hi``, and incorrect +``say_hi`` call sites do not trigger a diagnostic. + +To complement the ``format`` attribute, Clang also defines the +``format_matches`` attribute. Its syntax is similar to the ``format`` +attribute's, but instead of taking the index of the first formatted value +argument, it takes a C string literal with the expected specifiers: + +.. code-block:: c + + static const char *first_name; + static double todays_temperature; + static int wind_speed; + + __attribute__((__format_matches__(printf, 1, "%s %g"))) + void say_hi(const char *fmt) { + printf(fmt, first_name, todays_temperature); // no dignostic + printf(fmt, first_name, wind_speed); // warning: format specifies type 'int' but the argument has type 'double' + } + + int main() { + say_hi("hello %s, it is %g degrees outside"); + say_hi("it is %g degrees outside, have a good day %s!"); + // warning: format specifies 'double' where 'const char *' is required + // warning: format specifies 'const char *' where 'double' is required + } + +The third argument to ``format_matches`` is expected to evaluate to a **C string +literal** even when the format string would normally be a different type for the +given flavor, like a ``CFStringRef`` or a ``NSString *``. + +The only requirement on the format string literal is that it has specifiers +that are compatible with the arguments that will be used. It can contain +arbitrary non-format characters. For instance, for the purposes of compile-time +validation, ``"%s scored %g%% on her test"`` and ``"%s%g"`` are interchangeable +as the format string argument. As a means of self-documentation, users may +prefer the former when it provides a useful example of an expected format +string. + +In the implementation of a function with the ``format_matches`` attribute, +format verification works as if the format string was identical to the one +specified in the attribute. + +.. code-block:: c + + __attribute__((__format_matches__(printf, 1, "%s %g"))) + void say_hi(const char *fmt) { + printf(fmt, "person", 546); + // ^ warning: format specifies type 'double' but the + // argument has type 'int' + // note: format string is defined here: + // __attribute__((__format_matches__(printf, 1, "%s %g"))) + // ^~ + } + + +At the call sites of functions with the ``format_matches`` attribute, format +verification instead compares the two format strings to evaluate their +equivalence. Each format flavor defines equivalence between format specifiers. +Generally speaking, two specifiers are equivalent if they format the same type. +For instance, in the ``printf`` flavor, ``%2i`` and ``%-0.5d`` are compatible. +When ``-Wformat-signedness`` is disabled, ``%d`` and ``%u`` are compatible. For +a negative example, ``%ld`` is incompatible with ``%d``. + +Do note the following un-obvious cases: + +* Passing ``NULL`` as the format string does not trigger format diagnostics. +* When the format string is not NULL, it cannot _miss_ specifiers, even in + trailing positions. For instance, ``%d`` is not accepted when the required + format is ``%d %d %d``. +* While checks for the ``format`` attribute tolerate sone size mismatches + that standard argument promotion renders immaterial (such as formatting an + ``int`` with ``%hhd``, which specifies a ``char``-sized integer), checks for + ``format_matches`` require specified argument sizes to match exactly. +* Format strings expecting a variable modifier (such as ``%*s``) are + incompatible with format strings that would itemize the variable modifiers + (such as ``%i %s``), even if the two specify ABI-compatible argument lists. +* All pointer specifiers, modifiers aside, are mutually incompatible. For + instance, ``%s`` is not compatible with ``%p``, and ``%p`` is not compatible + with ``%n``, and ``%hhn`` is incompatible with ``%s``, even if the pointers + are ABI-compatible or identical on the selected platform. However, ``%0.5s`` + is compatible with ``%s``, since the difference only exists in modifier flags. + This is not overridable with ``-Wformat-pedantic`` or its inverse, which + control similar behavior in ``-Wformat``. + +At this time, clang implements ``format_matches`` only for format types in the +``printf`` family. This includes variants such as Apple's NSString format and +the FreeBSD ``kprintf``, but excludes ``scanf``. Using a known but unsupported +format silently fails in order to be compatible with other implementations that +would support these formats. + + }]; +} + def FlagEnumDocs : Documentation { let Category = DocCatDecl; let Content = [{ diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index f4f1bc67724a1..51301d95e55b9 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -7764,6 +7764,7 @@ def warn_format_nonliteral_noargs : Warning< def warn_format_nonliteral : Warning< "format string is not a string literal">, InGroup, DefaultIgnore; +def err_format_nonliteral : Error<"format string is not a string literal">; def err_unexpected_interface : Error< "unexpected interface name %0: expected expression">; @@ -10066,6 +10067,8 @@ def note_previous_declaration_as : Note< def warn_printf_insufficient_data_args : Warning< "more '%%' conversions than data arguments">, InGroup; +def warn_format_cmp_specifier_arity : Warning< + "%select{fewer|more}0 specifiers in format string than expected">, InGroup; def warn_printf_data_arg_not_used : Warning< "data argument not used by format string">, InGroup; def warn_format_invalid_conversion : Warning< @@ -10183,6 +10186,27 @@ def note_format_fix_specifier : Note<"did you mean to use '%0'?">; def note_printf_c_str: Note<"did you mean to call the %0 method?">; def note_format_security_fixit: Note< "treat the string as an argument to avoid this">; +def warn_format_string_type_incompatible : Warning< + "passing '%0' format string where '%1' format string is expected">, + InGroup; +def warn_format_cmp_role_mismatch : Warning< + "format argument is %select{a value|an indirect field width|an indirect " + "precision|an auxiliary value}0, but it should be %select{a value|an indirect " + "field width|an indirect precision|an auxiliary value}1">, InGroup; +def warn_format_cmp_modifierfor_mismatch : Warning< + "format argument modifies specifier at position %0, but it should modify " + "specifier at position %1">, InGroup; +def warn_format_cmp_sensitivity_mismatch : Warning< + "argument sensitivity is %select{unspecified|private|public|sensitive}0, but " + "it should be %select{unspecified|private|public|sensitive}1">, InGroup; +def warn_format_cmp_specifier_mismatch : Warning< + "format specifier '%0' is incompatible with '%1'">, InGroup; +def warn_format_cmp_specifier_sign_mismatch : Warning< + "signedness of format specifier '%0' is incompatible with '%1'">, InGroup; +def warn_format_cmp_specifier_mismatch_pedantic : Extension< + warn_format_cmp_specifier_sign_mismatch.Summary>, InGroup; +def note_format_cmp_with : Note< + "comparing with this %select{specifier|format string}0">; def warn_null_arg : Warning< "null passed to a callee that requires a non-null argument">, diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 093e9a06b00ce..476abe86cb2d2 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -2177,6 +2177,7 @@ class Sema final : public SemaBase { 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 + FAPK_Elsewhere, // values to format are not passed to this function }; // Used to grab the relevant information from a FormatAttr and a @@ -2187,12 +2188,15 @@ class Sema final : public SemaBase { FormatArgumentPassingKind ArgPassingKind; }; - /// Given a FunctionDecl's FormatAttr, attempts to populate the - /// FomatStringInfo parameter with the FormatAttr's correct format_idx and - /// firstDataArg. Returns true when the format fits the function and the - /// FormatStringInfo has been populated. - static bool getFormatStringInfo(const FormatAttr *Format, bool IsCXXMember, - bool IsVariadic, FormatStringInfo *FSI); + /// Given a function and its FormatAttr or FormatMatchesAttr info, attempts to + /// populate the FomatStringInfo parameter with the attribute's correct + /// format_idx and firstDataArg. Returns true when the format fits the + /// function and the FormatStringInfo has been populated. + static bool getFormatStringInfo(const Decl *Function, unsigned FormatIdx, + unsigned FirstArg, FormatStringInfo *FSI); + static bool getFormatStringInfo(unsigned FormatIdx, unsigned FirstArg, + bool IsCXXMember, bool IsVariadic, + FormatStringInfo *FSI); // Used by C++ template instantiation. ExprResult BuiltinShuffleVector(CallExpr *TheCall); @@ -2215,7 +2219,10 @@ class Sema final : public SemaBase { FST_Syslog, FST_Unknown }; + static StringRef GetFormatStringTypeName(FormatStringType FST); + static FormatStringType GetFormatStringType(StringRef FormatFlavor); static FormatStringType GetFormatStringType(const FormatAttr *Format); + static FormatStringType GetFormatStringType(const FormatMatchesAttr *Format); bool FormatStringHasSArg(const StringLiteral *FExpr); @@ -2362,6 +2369,25 @@ class Sema final : public SemaBase { bool IsMemberFunction, SourceLocation Loc, SourceRange Range, VariadicCallType CallType); + /// Verify that two format strings (as understood by attribute(format) and + /// attribute(format_matches) are compatible. If they are incompatible, + /// diagnostics are emitted with the assumption that \c + /// AuthoritativeFormatString is correct and + /// \c TestedFormatString is wrong. If \c FunctionCallArg is provided, + /// diagnostics will point to it and a note will refer to \c + /// TestedFormatString or \c AuthoritativeFormatString as appropriate. + bool + CheckFormatStringsCompatible(FormatStringType FST, + const StringLiteral *AuthoritativeFormatString, + const StringLiteral *TestedFormatString, + const Expr *FunctionCallArg = nullptr); + + /// Verify that one format string (as understood by attribute(format)) is + /// self-consistent; for instance, that it doesn't have multiple positional + /// arguments referring to the same argument in incompatible ways. Diagnose + /// if it isn't. + bool ValidateFormatString(FormatStringType FST, const StringLiteral *Str); + /// \brief Enforce the bounds of a TCB /// CheckTCBEnforcement - Enforces that every function in a named TCB only /// directly calls other functions in the same TCB as marked by the @@ -2577,11 +2603,17 @@ class Sema final : public SemaBase { VariadicCallType CallType, SourceLocation Loc, SourceRange Range, llvm::SmallBitVector &CheckedVarArgs); + bool CheckFormatString(const FormatMatchesAttr *Format, + ArrayRef Args, bool IsCXXMember, + VariadicCallType CallType, SourceLocation Loc, + SourceRange Range, + llvm::SmallBitVector &CheckedVarArgs); bool CheckFormatArguments(ArrayRef Args, - FormatArgumentPassingKind FAPK, unsigned format_idx, - unsigned firstDataArg, FormatStringType Type, - VariadicCallType CallType, SourceLocation Loc, - SourceRange range, + FormatArgumentPassingKind FAPK, + const StringLiteral *ReferenceFormatString, + unsigned format_idx, unsigned firstDataArg, + FormatStringType Type, VariadicCallType CallType, + SourceLocation Loc, SourceRange range, llvm::SmallBitVector &CheckedVarArgs); void CheckInfNaNFunction(const CallExpr *Call, const FunctionDecl *FDecl); @@ -4576,6 +4608,11 @@ class Sema final : public SemaBase { FormatAttr *mergeFormatAttr(Decl *D, const AttributeCommonInfo &CI, IdentifierInfo *Format, int FormatIdx, int FirstArg); + FormatMatchesAttr *mergeFormatMatchesAttr(Decl *D, + const AttributeCommonInfo &CI, + IdentifierInfo *Format, + int FormatIdx, + StringLiteral *FormatStr); /// AddAlignedAttr - Adds an aligned attribute to a particular declaration. void AddAlignedAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E, diff --git a/clang/lib/AST/AttrImpl.cpp b/clang/lib/AST/AttrImpl.cpp index f198a9acf8481..fefb8f55a9ee2 100644 --- a/clang/lib/AST/AttrImpl.cpp +++ b/clang/lib/AST/AttrImpl.cpp @@ -270,4 +270,8 @@ unsigned AlignedAttr::getAlignment(ASTContext &Ctx) const { return Ctx.getTargetDefaultAlignForAttributeAligned(); } +StringLiteral *FormatMatchesAttr::getFormatString() const { + return cast(getExpectedFormat()); +} + #include "clang/AST/AttrImpl.inc" diff --git a/clang/lib/AST/FormatString.cpp b/clang/lib/AST/FormatString.cpp index e892c1592df98..5d3b56fc4e713 100644 --- a/clang/lib/AST/FormatString.cpp +++ b/clang/lib/AST/FormatString.cpp @@ -595,6 +595,97 @@ ArgType::matchesType(ASTContext &C, QualType argTy) const { llvm_unreachable("Invalid ArgType Kind!"); } +static analyze_format_string::ArgType::MatchKind +integerTypeMatch(ASTContext &C, QualType A, QualType B, bool CheckSign) { + using MK = analyze_format_string::ArgType::MatchKind; + + uint64_t IntSize = C.getTypeSize(C.IntTy); + uint64_t ASize = C.getTypeSize(A); + uint64_t BSize = C.getTypeSize(B); + if (std::max(ASize, IntSize) != std::max(BSize, IntSize)) + return MK::NoMatch; + if (CheckSign && A->isSignedIntegerType() != B->isSignedIntegerType()) + return MK::NoMatchSignedness; + if (ASize != BSize) + return MK::MatchPromotion; + return MK::Match; +} + +analyze_format_string::ArgType::MatchKind +ArgType::matchesArgType(ASTContext &C, const ArgType &Other) const { + using AK = analyze_format_string::ArgType::Kind; + + // Per matchesType. + if (K == AK::InvalidTy || Other.K == AK::InvalidTy) + return NoMatch; + if (K == AK::UnknownTy || Other.K == AK::UnknownTy) + return Match; + + // Handle whether either (or both, or neither) sides has Ptr set, + // in addition to whether either (or both, or neither) sides is a SpecificTy + // that is a pointer. + ArgType Left = *this; + bool LeftWasPointer = false; + ArgType Right = Other; + bool RightWasPointer = false; + if (Left.Ptr) { + Left.Ptr = false; + LeftWasPointer = true; + } else if (Left.K == AK::SpecificTy && Left.T->isPointerType()) { + Left.T = Left.T->getPointeeType(); + LeftWasPointer = true; + } + if (Right.Ptr) { + Right.Ptr = false; + RightWasPointer = true; + } else if (Right.K == AK::SpecificTy && Right.T->isPointerType()) { + Right.T = Right.T->getPointeeType(); + RightWasPointer = true; + } + + if (LeftWasPointer != RightWasPointer) + return NoMatch; + + // Ensure that if at least one side is a SpecificTy, then Left is a + // SpecificTy. + if (Right.K == AK::SpecificTy) + std::swap(Left, Right); + + if (Left.K == AK::SpecificTy) { + if (Right.K == AK::SpecificTy) { + auto Canon1 = C.getCanonicalType(Left.T); + auto Canon2 = C.getCanonicalType(Right.T); + if (Canon1 == Canon2) + return Match; + + auto *BT1 = QualType(Canon1)->getAs(); + auto *BT2 = QualType(Canon2)->getAs(); + if (BT1 == nullptr || BT2 == nullptr) + return NoMatch; + if (BT1 == BT2) + return Match; + + if (!LeftWasPointer && BT1->isInteger() && BT2->isInteger()) + return integerTypeMatch(C, Canon1, Canon2, true); + return NoMatch; + } else if (Right.K == AK::AnyCharTy) { + if (!LeftWasPointer && Left.T->isIntegerType()) + return integerTypeMatch(C, Left.T, C.CharTy, false); + return NoMatch; + } else if (Right.K == AK::WIntTy) { + if (!LeftWasPointer && Left.T->isIntegerType()) + return integerTypeMatch(C, Left.T, C.WIntTy, true); + return NoMatch; + } + // It's hypothetically possible to create an AK::SpecificTy ArgType + // that matches another kind of ArgType, but in practice Clang doesn't + // do that, so ignore that case. + return NoMatch; + } + + return Left.K == Right.K ? Match : NoMatch; +} + ArgType ArgType::makeVectorType(ASTContext &C, unsigned NumElts) const { // Check for valid vector element types. if (T.isNull()) diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 74f425d32648f..81209f2242f59 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -3021,17 +3021,33 @@ bool Sema::ValueIsRunOfOnes(CallExpr *TheCall, unsigned ArgNum) { << ArgNum << Arg->getSourceRange(); } -bool Sema::getFormatStringInfo(const FormatAttr *Format, bool IsCXXMember, - bool IsVariadic, FormatStringInfo *FSI) { - if (Format->getFirstArg() == 0) +bool Sema::getFormatStringInfo(const Decl *D, unsigned FormatIdx, + unsigned FirstArg, FormatStringInfo *FSI) { + 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(); + + return getFormatStringInfo(FormatIdx, FirstArg, IsCXXMember, IsVariadic, FSI); +} + +bool Sema::getFormatStringInfo(unsigned FormatIdx, unsigned FirstArg, + bool IsCXXMember, bool IsVariadic, + FormatStringInfo *FSI) { + if (FirstArg == 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->ArgPassingKind == FAPK_VAList ? 0 : Format->getFirstArg() - 1; + FSI->FormatIdx = FormatIdx - 1; + FSI->FirstDataArg = FSI->ArgPassingKind == FAPK_VAList ? 0 : FirstArg - 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 @@ -3285,10 +3301,15 @@ void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto, // Printf and scanf checking. llvm::SmallBitVector CheckedVarArgs; if (FDecl) { - for (const auto *I : FDecl->specific_attrs()) { + for (const auto *I : FDecl->specific_attrs()) { // Only create vector if there are format attributes. CheckedVarArgs.resize(Args.size()); + CheckFormatString(I, Args, IsMemberFunction, CallType, Loc, Range, + CheckedVarArgs); + } + for (const auto *I : FDecl->specific_attrs()) { + CheckedVarArgs.resize(Args.size()); CheckFormatArguments(I, Args, IsMemberFunction, CallType, Loc, Range, CheckedVarArgs); } @@ -5460,7 +5481,7 @@ bool Sema::BuiltinOSLogFormat(CallExpr *TheCall) { llvm::SmallBitVector CheckedVarArgs(NumArgs, false); ArrayRef Args(TheCall->getArgs(), TheCall->getNumArgs()); bool Success = CheckFormatArguments( - Args, FAPK_Variadic, FormatIdx, FirstDataArg, FST_OSLog, + Args, FAPK_Variadic, nullptr, FormatIdx, FirstDataArg, FST_OSLog, VariadicFunction, TheCall->getBeginLoc(), SourceRange(), CheckedVarArgs); if (!Success) @@ -5856,13 +5877,13 @@ class FormatStringLiteral { const StringLiteral *FExpr; int64_t Offset; - public: +public: FormatStringLiteral(const StringLiteral *fexpr, int64_t Offset = 0) : FExpr(fexpr), Offset(Offset) {} - StringRef getString() const { - return FExpr->getString().drop_front(Offset); - } + const StringLiteral *getFormatString() const { return FExpr; } + + StringRef getString() const { return FExpr->getString().drop_front(Offset); } unsigned getByteLength() const { return FExpr->getByteLength() - getCharByteWidth() * Offset; @@ -5900,7 +5921,8 @@ class FormatStringLiteral { } // namespace static void CheckFormatString( - Sema &S, const FormatStringLiteral *FExpr, const Expr *OrigFormatExpr, + Sema &S, const FormatStringLiteral *FExpr, + const StringLiteral *ReferenceFormatString, const Expr *OrigFormatExpr, ArrayRef Args, Sema::FormatArgumentPassingKind APK, unsigned format_idx, unsigned firstDataArg, Sema::FormatStringType Type, bool inFunctionCall, Sema::VariadicCallType CallType, @@ -5914,14 +5936,13 @@ static const Expr *maybeConstEvalStringLiteral(ASTContext &Context, // If this function returns false on the arguments to a function expecting a // format string, we will usually need to emit a warning. // True string literals are then checked by CheckFormatString. -static StringLiteralCheckType -checkFormatStringExpr(Sema &S, const Expr *E, ArrayRef Args, - Sema::FormatArgumentPassingKind APK, unsigned format_idx, - unsigned firstDataArg, Sema::FormatStringType Type, - Sema::VariadicCallType CallType, bool InFunctionCall, - llvm::SmallBitVector &CheckedVarArgs, - UncoveredArgHandler &UncoveredArg, llvm::APSInt Offset, - bool IgnoreStringsWithoutSpecifiers = false) { +static StringLiteralCheckType checkFormatStringExpr( + Sema &S, const StringLiteral *ReferenceFormatString, const Expr *E, + ArrayRef Args, Sema::FormatArgumentPassingKind APK, + unsigned format_idx, unsigned firstDataArg, Sema::FormatStringType Type, + Sema::VariadicCallType CallType, bool InFunctionCall, + llvm::SmallBitVector &CheckedVarArgs, UncoveredArgHandler &UncoveredArg, + llvm::APSInt Offset, bool IgnoreStringsWithoutSpecifiers = false) { if (S.isConstantEvaluatedContext()) return SLCT_NotALiteral; tryAgain: @@ -5943,10 +5964,10 @@ checkFormatStringExpr(Sema &S, const Expr *E, ArrayRef Args, case Stmt::InitListExprClass: // Handle expressions like {"foobar"}. if (const clang::Expr *SLE = maybeConstEvalStringLiteral(S.Context, E)) { - return checkFormatStringExpr(S, SLE, Args, APK, format_idx, firstDataArg, - Type, CallType, /*InFunctionCall*/ false, - CheckedVarArgs, UncoveredArg, Offset, - IgnoreStringsWithoutSpecifiers); + return checkFormatStringExpr( + S, ReferenceFormatString, SLE, Args, APK, format_idx, firstDataArg, + Type, CallType, /*InFunctionCall*/ false, CheckedVarArgs, + UncoveredArg, Offset, IgnoreStringsWithoutSpecifiers); } return SLCT_NotALiteral; case Stmt::BinaryConditionalOperatorClass: @@ -5978,19 +5999,19 @@ checkFormatStringExpr(Sema &S, const Expr *E, ArrayRef Args, if (!CheckLeft) Left = SLCT_UncheckedLiteral; else { - Left = checkFormatStringExpr(S, C->getTrueExpr(), Args, APK, format_idx, - firstDataArg, Type, CallType, InFunctionCall, - CheckedVarArgs, UncoveredArg, Offset, - IgnoreStringsWithoutSpecifiers); + Left = checkFormatStringExpr( + S, ReferenceFormatString, C->getTrueExpr(), Args, APK, format_idx, + firstDataArg, Type, CallType, InFunctionCall, CheckedVarArgs, + UncoveredArg, Offset, IgnoreStringsWithoutSpecifiers); if (Left == SLCT_NotALiteral || !CheckRight) { return Left; } } StringLiteralCheckType Right = checkFormatStringExpr( - S, C->getFalseExpr(), Args, APK, format_idx, firstDataArg, Type, - CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, Offset, - IgnoreStringsWithoutSpecifiers); + S, ReferenceFormatString, C->getFalseExpr(), Args, APK, format_idx, + firstDataArg, Type, CallType, InFunctionCall, CheckedVarArgs, + UncoveredArg, Offset, IgnoreStringsWithoutSpecifiers); return (CheckLeft && Left < Right) ? Left : Right; } @@ -6040,7 +6061,8 @@ checkFormatStringExpr(Sema &S, const Expr *E, ArrayRef Args, Init = InitList->getInit(0)->IgnoreParenImpCasts(); } return checkFormatStringExpr( - S, Init, Args, APK, format_idx, firstDataArg, Type, CallType, + S, ReferenceFormatString, Init, Args, APK, format_idx, + firstDataArg, Type, CallType, /*InFunctionCall*/ false, CheckedVarArgs, UncoveredArg, Offset); } } @@ -6061,7 +6083,17 @@ checkFormatStringExpr(Sema &S, const Expr *E, ArrayRef Args, // ... // } // - // Another interaction that we need to support is calling a variadic + // Another interaction that we need to support is using a format string + // specified by the format_matches attribute: + // + // __attribute__((format_matches(printf, 1, "%s %d"))) + // void logmessage(char const *fmt, const char *a, int b) { + // printf(fmt, a, b); /* do not emit a warning about "fmt" */ + // printf(fmt, 123.4); /* emit warnings that "%s %d" is incompatible */ + // ... + // } + // + // Yet another interaction that we need to support is calling a variadic // format function from a format function that has fixed arguments. For // instance: // @@ -6084,39 +6116,63 @@ checkFormatStringExpr(Sema &S, const Expr *E, ArrayRef Args, // 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(); + for (const auto *PVFormatMatches : + D->specific_attrs()) { + Sema::FormatStringInfo CalleeFSI; + if (!Sema::getFormatStringInfo(D, PVFormatMatches->getFormatIdx(), + 0, &CalleeFSI)) + continue; + if (PV->getFunctionScopeIndex() == CalleeFSI.FormatIdx) { + // If using the wrong type of format string, emit a diagnostic + // here and stop checking to avoid irrelevant diagnostics. + if (Type != S.GetFormatStringType(PVFormatMatches)) { + S.Diag(Args[format_idx]->getBeginLoc(), + diag::warn_format_string_type_incompatible) + << PVFormatMatches->getType()->getName() + << S.GetFormatStringTypeName(Type); + if (!InFunctionCall) { + S.Diag(PVFormatMatches->getFormatString()->getBeginLoc(), + diag::note_format_string_defined); + } + return SLCT_UncheckedLiteral; + } + return checkFormatStringExpr( + S, ReferenceFormatString, PVFormatMatches->getFormatString(), + Args, APK, format_idx, firstDataArg, Type, CallType, + /*InFunctionCall*/ false, CheckedVarArgs, UncoveredArg, + Offset, IgnoreStringsWithoutSpecifiers); + } + } + for (const auto *PVFormat : D->specific_attrs()) { 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 (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; - } + if (!Sema::getFormatStringInfo(D, PVFormat->getFormatIdx(), + PVFormat->getFirstArg(), &CallerFSI)) + continue; + // We also check if the formats are compatible. + // We can't pass a 'scanf' string to a 'printf' function. + if (Type != S.GetFormatStringType(PVFormat)) { + S.Diag(Args[format_idx]->getBeginLoc(), + diag::warn_format_string_type_incompatible) + << PVFormat->getType()->getName() + << S.GetFormatStringTypeName(Type); + if (!InFunctionCall) { + S.Diag(E->getBeginLoc(), diag::note_format_string_defined); + } + return SLCT_UncheckedLiteral; + } else if (PV->getFunctionScopeIndex() == CallerFSI.FormatIdx) { + // 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; } } } @@ -6136,9 +6192,9 @@ checkFormatStringExpr(Sema &S, const Expr *E, ArrayRef Args, for (const auto *FA : ND->specific_attrs()) { const Expr *Arg = CE->getArg(FA->getFormatIdx().getASTIndex()); StringLiteralCheckType Result = checkFormatStringExpr( - S, Arg, Args, APK, format_idx, firstDataArg, Type, CallType, - InFunctionCall, CheckedVarArgs, UncoveredArg, Offset, - IgnoreStringsWithoutSpecifiers); + S, ReferenceFormatString, Arg, Args, APK, format_idx, firstDataArg, + Type, CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, + Offset, IgnoreStringsWithoutSpecifiers); if (IsFirst) { CommonResult = Result; IsFirst = false; @@ -6153,17 +6209,17 @@ checkFormatStringExpr(Sema &S, const Expr *E, ArrayRef Args, BuiltinID == Builtin::BI__builtin___NSStringMakeConstantString) { const Expr *Arg = CE->getArg(0); return checkFormatStringExpr( - S, Arg, Args, APK, format_idx, firstDataArg, Type, CallType, - InFunctionCall, CheckedVarArgs, UncoveredArg, Offset, - IgnoreStringsWithoutSpecifiers); + S, ReferenceFormatString, Arg, Args, APK, format_idx, + firstDataArg, Type, CallType, InFunctionCall, CheckedVarArgs, + UncoveredArg, Offset, IgnoreStringsWithoutSpecifiers); } } } if (const Expr *SLE = maybeConstEvalStringLiteral(S.Context, E)) - return checkFormatStringExpr(S, SLE, Args, APK, format_idx, firstDataArg, - Type, CallType, /*InFunctionCall*/ false, - CheckedVarArgs, UncoveredArg, Offset, - IgnoreStringsWithoutSpecifiers); + return checkFormatStringExpr( + S, ReferenceFormatString, SLE, Args, APK, format_idx, firstDataArg, + Type, CallType, /*InFunctionCall*/ false, CheckedVarArgs, + UncoveredArg, Offset, IgnoreStringsWithoutSpecifiers); return SLCT_NotALiteral; } case Stmt::ObjCMessageExprClass: { @@ -6187,9 +6243,9 @@ checkFormatStringExpr(Sema &S, const Expr *E, ArrayRef Args, const Expr *Arg = ME->getArg(FA->getFormatIdx().getASTIndex()); return checkFormatStringExpr( - S, Arg, Args, APK, format_idx, firstDataArg, Type, CallType, - InFunctionCall, CheckedVarArgs, UncoveredArg, Offset, - IgnoreStringsWithoutSpecifiers); + S, ReferenceFormatString, Arg, Args, APK, format_idx, firstDataArg, + Type, CallType, InFunctionCall, CheckedVarArgs, UncoveredArg, + Offset, IgnoreStringsWithoutSpecifiers); } } @@ -6211,8 +6267,9 @@ checkFormatStringExpr(Sema &S, const Expr *E, ArrayRef Args, return SLCT_NotALiteral; } FormatStringLiteral FStr(StrE, Offset.sextOrTrunc(64).getSExtValue()); - CheckFormatString(S, &FStr, E, Args, APK, format_idx, firstDataArg, Type, - InFunctionCall, CallType, CheckedVarArgs, UncoveredArg, + CheckFormatString(S, &FStr, ReferenceFormatString, E, Args, APK, + format_idx, firstDataArg, Type, InFunctionCall, + CallType, CheckedVarArgs, UncoveredArg, IgnoreStringsWithoutSpecifiers); return SLCT_CheckedLiteral; } @@ -6289,8 +6346,31 @@ static const Expr *maybeConstEvalStringLiteral(ASTContext &Context, return nullptr; } -Sema::FormatStringType Sema::GetFormatStringType(const FormatAttr *Format) { - return llvm::StringSwitch(Format->getType()->getName()) +StringRef Sema::GetFormatStringTypeName(Sema::FormatStringType FST) { + switch (FST) { + case FST_Scanf: + return "scanf"; + case FST_Printf: + return "printf"; + case FST_NSString: + return "NSString"; + case FST_Strftime: + return "strftime"; + case FST_Strfmon: + return "strfmon"; + case FST_Kprintf: + return "kprintf"; + case FST_FreeBSDKPrintf: + return "freebsd_kprintf"; + case FST_OSLog: + return "os_log"; + default: + return ""; + } +} + +Sema::FormatStringType Sema::GetFormatStringType(StringRef Flavor) { + return llvm::StringSwitch(Flavor) .Case("scanf", FST_Scanf) .Cases("printf", "printf0", "syslog", FST_Printf) .Cases("NSString", "CFString", FST_NSString) @@ -6303,22 +6383,49 @@ Sema::FormatStringType Sema::GetFormatStringType(const FormatAttr *Format) { .Default(FST_Unknown); } +Sema::FormatStringType Sema::GetFormatStringType(const FormatAttr *Format) { + return GetFormatStringType(Format->getType()->getName()); +} + +Sema::FormatStringType +Sema::GetFormatStringType(const FormatMatchesAttr *Format) { + return GetFormatStringType(Format->getType()->getName()); +} + bool Sema::CheckFormatArguments(const FormatAttr *Format, ArrayRef Args, bool IsCXXMember, VariadicCallType CallType, SourceLocation Loc, SourceRange Range, llvm::SmallBitVector &CheckedVarArgs) { FormatStringInfo FSI; - if (getFormatStringInfo(Format, IsCXXMember, CallType != VariadicDoesNotApply, - &FSI)) - return CheckFormatArguments(Args, FSI.ArgPassingKind, FSI.FormatIdx, + if (getFormatStringInfo(Format->getFormatIdx(), Format->getFirstArg(), + IsCXXMember, CallType != VariadicDoesNotApply, &FSI)) + return CheckFormatArguments( + Args, FSI.ArgPassingKind, nullptr, FSI.FormatIdx, FSI.FirstDataArg, + GetFormatStringType(Format), CallType, Loc, Range, CheckedVarArgs); + return false; +} + +bool Sema::CheckFormatString(const FormatMatchesAttr *Format, + ArrayRef Args, bool IsCXXMember, + VariadicCallType CallType, SourceLocation Loc, + SourceRange Range, + llvm::SmallBitVector &CheckedVarArgs) { + FormatStringInfo FSI; + if (getFormatStringInfo(Format->getFormatIdx(), 0, IsCXXMember, false, + &FSI)) { + FSI.ArgPassingKind = Sema::FAPK_Elsewhere; + return CheckFormatArguments(Args, FSI.ArgPassingKind, + Format->getFormatString(), FSI.FormatIdx, FSI.FirstDataArg, GetFormatStringType(Format), CallType, Loc, Range, CheckedVarArgs); + } return false; } bool Sema::CheckFormatArguments(ArrayRef Args, Sema::FormatArgumentPassingKind APK, + const StringLiteral *ReferenceFormatString, unsigned format_idx, unsigned firstDataArg, FormatStringType Type, VariadicCallType CallType, SourceLocation Loc, @@ -6346,8 +6453,8 @@ bool Sema::CheckFormatArguments(ArrayRef Args, // the same format string checking logic for both ObjC and C strings. UncoveredArgHandler UncoveredArg; StringLiteralCheckType CT = checkFormatStringExpr( - *this, OrigFormatExpr, Args, APK, format_idx, firstDataArg, Type, - CallType, + *this, ReferenceFormatString, OrigFormatExpr, Args, APK, format_idx, + firstDataArg, Type, CallType, /*IsFunctionCall*/ true, CheckedVarArgs, UncoveredArg, /*no string offset*/ llvm::APSInt(64, false) = 0); @@ -6443,6 +6550,11 @@ class CheckFormatHandler : public analyze_format_string::FormatStringHandler { CoveredArgs.reset(); } + bool HasFormatArguments() const { + return ArgPassingKind == Sema::FAPK_Fixed || + ArgPassingKind == Sema::FAPK_Variadic; + } + void DoneProcessing(); void HandleIncompleteSpecifier(const char *startSpecifier, @@ -6678,7 +6790,7 @@ const Expr *CheckFormatHandler::getDataArg(unsigned i) const { void CheckFormatHandler::DoneProcessing() { // Does the number of data arguments exceed the number of // format conversions in the format string? - if (ArgPassingKind != Sema::FAPK_VAList) { + if (HasFormatArguments()) { // Find any arguments that weren't covered. CoveredArgs.flip(); signed notCoveredArg = CoveredArgs.find_first(); @@ -6787,7 +6899,7 @@ CheckFormatHandler::CheckNumArgs( const analyze_format_string::ConversionSpecifier &CS, const char *startSpecifier, unsigned specifierLen, unsigned argIndex) { - if (argIndex >= NumDataArgs) { + if (HasFormatArguments() && argIndex >= NumDataArgs) { PartialDiagnostic PDiag = FS.usesPositionalArg() ? (S.PDiag(diag::warn_printf_positional_arg_exceeds_data_args) << (argIndex+1) << NumDataArgs) @@ -6928,18 +7040,112 @@ class CheckPrintfHandler : public CheckFormatHandler { void HandleInvalidObjCModifierFlag(const char *startFlag, unsigned flagLen) override; - void HandleObjCFlagsWithNonObjCConversion(const char *flagsStart, - const char *flagsEnd, - const char *conversionPosition) - override; + void + HandleObjCFlagsWithNonObjCConversion(const char *flagsStart, + const char *flagsEnd, + const char *conversionPosition) override; +}; + +/// Keeps around the information needed to verify that two specifiers are +/// compatible. +class EquatableFormatArgument { +public: + enum SpecifierSensitivity : unsigned { + SS_None, + SS_Private, + SS_Public, + SS_Sensitive + }; + + enum FormatArgumentRole : unsigned { + FAR_Data, + FAR_FieldWidth, + FAR_Precision, + FAR_Auxiliary, // FreeBSD kernel %b and %D + }; + +private: + analyze_format_string::ArgType ArgType; + analyze_format_string::LengthModifier::Kind LengthMod; + StringRef SpecifierLetter; + CharSourceRange Range; + SourceLocation ElementLoc; + FormatArgumentRole Role : 2; + SpecifierSensitivity Sensitivity : 2; // only set for FAR_Data + unsigned Position : 14; + unsigned ModifierFor : 14; // not set for FAR_Data + + void EmitDiagnostic(Sema &S, PartialDiagnostic PDiag, const Expr *FmtExpr, + bool InFunctionCall) const; + +public: + EquatableFormatArgument(CharSourceRange Range, SourceLocation ElementLoc, + analyze_format_string::LengthModifier::Kind LengthMod, + StringRef SpecifierLetter, + analyze_format_string::ArgType ArgType, + FormatArgumentRole Role, + SpecifierSensitivity Sensitivity, unsigned Position, + unsigned ModifierFor) + : ArgType(ArgType), LengthMod(LengthMod), + SpecifierLetter(SpecifierLetter), Range(Range), ElementLoc(ElementLoc), + Role(Role), Sensitivity(Sensitivity), Position(Position), + ModifierFor(ModifierFor) {} + + unsigned getPosition() const { return Position; } + SourceLocation getSourceLocation() const { return ElementLoc; } + CharSourceRange getSourceRange() const { return Range; } + analyze_format_string::LengthModifier getLengthModifier() const { + return analyze_format_string::LengthModifier(nullptr, LengthMod); + } + void setModifierFor(unsigned V) { ModifierFor = V; } + + std::string buildFormatSpecifier() const { + std::string result; + llvm::raw_string_ostream(result) + << getLengthModifier().toString() << SpecifierLetter; + return result; + } + + bool VerifyCompatible(Sema &S, const EquatableFormatArgument &Other, + const Expr *FmtExpr, bool InFunctionCall) const; +}; + +/// Turns format strings into lists of EquatableSpecifier objects. +class DecomposePrintfHandler : public CheckPrintfHandler { + llvm::SmallVectorImpl &Specs; + bool HadError; + + DecomposePrintfHandler( + Sema &s, const FormatStringLiteral *fexpr, const Expr *origFormatExpr, + const Sema::FormatStringType type, unsigned firstDataArg, + unsigned numDataArgs, bool isObjC, const char *beg, + Sema::FormatArgumentPassingKind APK, ArrayRef Args, + unsigned formatIdx, bool inFunctionCall, Sema::VariadicCallType CallType, + llvm::SmallBitVector &CheckedVarArgs, UncoveredArgHandler &UncoveredArg, + llvm::SmallVectorImpl &Specs) + : CheckPrintfHandler(s, fexpr, origFormatExpr, type, firstDataArg, + numDataArgs, isObjC, beg, APK, Args, formatIdx, + inFunctionCall, CallType, CheckedVarArgs, + UncoveredArg), + Specs(Specs), HadError(false) {} + +public: + static bool + GetSpecifiers(Sema &S, const FormatStringLiteral *FSL, const Expr *FmtExpr, + Sema::FormatStringType type, bool IsObjC, bool InFunctionCall, + llvm::SmallVectorImpl &Args); + + virtual bool HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier &FS, + const char *startSpecifier, + unsigned specifierLen, + const TargetInfo &Target) override; }; } // namespace bool CheckPrintfHandler::HandleInvalidPrintfConversionSpecifier( - const analyze_printf::PrintfSpecifier &FS, - const char *startSpecifier, - unsigned specifierLen) { + const analyze_printf::PrintfSpecifier &FS, const char *startSpecifier, + unsigned specifierLen) { const analyze_printf::PrintfConversionSpecifier &CS = FS.getConversionSpecifier(); @@ -6957,7 +7163,7 @@ bool CheckPrintfHandler::HandleAmount( const analyze_format_string::OptionalAmount &Amt, unsigned k, const char *startSpecifier, unsigned specifierLen) { if (Amt.hasDataArgument()) { - if (ArgPassingKind != Sema::FAPK_VAList) { + if (HasFormatArguments()) { unsigned argIndex = Amt.getArgIndex(); if (argIndex >= NumDataArgs) { EmitFormatDiagnostic(S.PDiag(diag::warn_printf_asterisk_missing_arg) @@ -7082,8 +7288,216 @@ void CheckPrintfHandler::HandleObjCFlagsWithNonObjCConversion( auto diag = diag::warn_printf_ObjCflags_without_ObjCConversion; EmitFormatDiagnostic(S.PDiag(diag) << StringRef(conversionPosition, 1), getLocationOfByte(conversionPosition), - /*IsStringLocation*/true, - Range, FixItHint::CreateRemoval(Range)); + /*IsStringLocation*/ true, Range, + FixItHint::CreateRemoval(Range)); +} + +void EquatableFormatArgument::EmitDiagnostic(Sema &S, PartialDiagnostic PDiag, + const Expr *FmtExpr, + bool InFunctionCall) const { + CheckFormatHandler::EmitFormatDiagnostic(S, InFunctionCall, FmtExpr, PDiag, + ElementLoc, true, Range); +} + +bool EquatableFormatArgument::VerifyCompatible( + Sema &S, const EquatableFormatArgument &Other, const Expr *FmtExpr, + bool InFunctionCall) const { + using MK = analyze_format_string::ArgType::MatchKind; + if (Role != Other.Role) { + // diagnose and stop + EmitDiagnostic( + S, S.PDiag(diag::warn_format_cmp_role_mismatch) << Role << Other.Role, + FmtExpr, InFunctionCall); + S.Diag(Other.ElementLoc, diag::note_format_cmp_with) << 0 << Other.Range; + return false; + } + + if (Role != FAR_Data) { + if (ModifierFor != Other.ModifierFor) { + // diagnose and stop + EmitDiagnostic(S, + S.PDiag(diag::warn_format_cmp_modifierfor_mismatch) + << (ModifierFor + 1) << (Other.ModifierFor + 1), + FmtExpr, InFunctionCall); + S.Diag(Other.ElementLoc, diag::note_format_cmp_with) << 0 << Other.Range; + return false; + } + return true; + } + + bool HadError = false; + if (Sensitivity != Other.Sensitivity) { + // diagnose and continue + EmitDiagnostic(S, + S.PDiag(diag::warn_format_cmp_sensitivity_mismatch) + << Sensitivity << Other.Sensitivity, + FmtExpr, InFunctionCall); + HadError = S.Diag(Other.ElementLoc, diag::note_format_cmp_with) + << 0 << Other.Range; + } + + switch (ArgType.matchesArgType(S.Context, Other.ArgType)) { + case MK::Match: + break; + + case MK::MatchPromotion: + // Per consensus reached at https://discourse.llvm.org/t/-/83076/12, + // MatchPromotion is treated as a failure by format_matches. + case MK::NoMatch: + case MK::NoMatchTypeConfusion: + case MK::NoMatchPromotionTypeConfusion: + EmitDiagnostic(S, + S.PDiag(diag::warn_format_cmp_specifier_mismatch) + << buildFormatSpecifier() + << Other.buildFormatSpecifier(), + FmtExpr, InFunctionCall); + HadError = S.Diag(Other.ElementLoc, diag::note_format_cmp_with) + << 0 << Other.Range; + break; + + case MK::NoMatchPedantic: + EmitDiagnostic(S, + S.PDiag(diag::warn_format_cmp_specifier_mismatch_pedantic) + << buildFormatSpecifier() + << Other.buildFormatSpecifier(), + FmtExpr, InFunctionCall); + HadError = S.Diag(Other.ElementLoc, diag::note_format_cmp_with) + << 0 << Other.Range; + break; + + case MK::NoMatchSignedness: + if (!S.getDiagnostics().isIgnored( + diag::warn_format_conversion_argument_type_mismatch_signedness, + ElementLoc)) { + EmitDiagnostic(S, + S.PDiag(diag::warn_format_cmp_specifier_sign_mismatch) + << buildFormatSpecifier() + << Other.buildFormatSpecifier(), + FmtExpr, InFunctionCall); + HadError = S.Diag(Other.ElementLoc, diag::note_format_cmp_with) + << 0 << Other.Range; + } + break; + } + return !HadError; +} + +bool DecomposePrintfHandler::GetSpecifiers( + Sema &S, const FormatStringLiteral *FSL, const Expr *FmtExpr, + Sema::FormatStringType Type, bool IsObjC, bool InFunctionCall, + llvm::SmallVectorImpl &Args) { + StringRef Data = FSL->getString(); + const char *Str = Data.data(); + llvm::SmallBitVector BV; + UncoveredArgHandler UA; + DecomposePrintfHandler H(S, FSL, FSL->getFormatString(), Type, 0, 0, IsObjC, + Str, Sema::FAPK_Elsewhere, {FSL->getFormatString()}, + 0, InFunctionCall, Sema::VariadicDoesNotApply, BV, + UA, Args); + + if (!analyze_format_string::ParsePrintfString( + H, Str, Str + Data.size(), S.getLangOpts(), S.Context.getTargetInfo(), + Type == Sema::FST_FreeBSDKPrintf)) + H.DoneProcessing(); + if (H.HadError) + return false; + + std::sort( + Args.begin(), Args.end(), + [](const EquatableFormatArgument &A, const EquatableFormatArgument &B) { + return A.getPosition() < B.getPosition(); + }); + return true; +} + +bool DecomposePrintfHandler::HandlePrintfSpecifier( + const analyze_printf::PrintfSpecifier &FS, const char *startSpecifier, + unsigned specifierLen, const TargetInfo &Target) { + if (!CheckPrintfHandler::HandlePrintfSpecifier(FS, startSpecifier, + specifierLen, Target)) { + HadError = true; + return false; + } + + // Do not add any specifiers to the list for %%. This is possibly incorrect + // if using a precision/width with a data argument, but that combination is + // meaningless and we wouldn't know which format to attach the + // precision/width to. + const auto &CS = FS.getConversionSpecifier(); + if (CS.getKind() == analyze_format_string::ConversionSpecifier::PercentArg) + return true; + + // have to patch these to have the right ModifierFor if they are used + const unsigned Unset = ~0; + unsigned FieldWidthIndex = Unset; + unsigned PrecisionIndex = Unset; + + // field width? + const auto &FieldWidth = FS.getFieldWidth(); + if (!FieldWidth.isInvalid() && FieldWidth.hasDataArgument()) { + FieldWidthIndex = Specs.size(); + Specs.emplace_back(getSpecifierRange(startSpecifier, specifierLen), + getLocationOfByte(FieldWidth.getStart()), + analyze_format_string::LengthModifier::None, "*", + FieldWidth.getArgType(S.Context), + EquatableFormatArgument::FAR_FieldWidth, + EquatableFormatArgument::SS_None, + FieldWidth.usesPositionalArg() + ? FieldWidth.getPositionalArgIndex() - 1 + : FieldWidthIndex, + 0); + } + // precision? + const auto &Precision = FS.getPrecision(); + if (!Precision.isInvalid() && Precision.hasDataArgument()) { + PrecisionIndex = Specs.size(); + Specs.emplace_back( + getSpecifierRange(startSpecifier, specifierLen), + getLocationOfByte(Precision.getStart()), + analyze_format_string::LengthModifier::None, ".*", + Precision.getArgType(S.Context), EquatableFormatArgument::FAR_Precision, + EquatableFormatArgument::SS_None, + Precision.usesPositionalArg() ? Precision.getPositionalArgIndex() - 1 + : PrecisionIndex, + 0); + } + + // this specifier + unsigned SpecIndex = + FS.usesPositionalArg() ? FS.getPositionalArgIndex() - 1 : Specs.size(); + if (FieldWidthIndex != Unset) + Specs[FieldWidthIndex].setModifierFor(SpecIndex); + if (PrecisionIndex != Unset) + Specs[PrecisionIndex].setModifierFor(SpecIndex); + + EquatableFormatArgument::SpecifierSensitivity Sensitivity; + if (FS.isPrivate()) + Sensitivity = EquatableFormatArgument::SS_Private; + else if (FS.isPublic()) + Sensitivity = EquatableFormatArgument::SS_Public; + else if (FS.isSensitive()) + Sensitivity = EquatableFormatArgument::SS_Sensitive; + else + Sensitivity = EquatableFormatArgument::SS_None; + + Specs.emplace_back( + getSpecifierRange(startSpecifier, specifierLen), + getLocationOfByte(CS.getStart()), FS.getLengthModifier().getKind(), + CS.getCharacters(), FS.getArgType(S.Context, isObjCContext()), + EquatableFormatArgument::FAR_Data, Sensitivity, SpecIndex, 0); + + // auxiliary argument? + if (CS.getKind() == analyze_format_string::ConversionSpecifier::FreeBSDbArg || + CS.getKind() == analyze_format_string::ConversionSpecifier::FreeBSDDArg) { + Specs.emplace_back(getSpecifierRange(startSpecifier, specifierLen), + getLocationOfByte(CS.getStart()), + analyze_format_string::LengthModifier::None, + CS.getCharacters(), + analyze_format_string::ArgType::CStrTy, + EquatableFormatArgument::FAR_Auxiliary, Sensitivity, + SpecIndex + 1, SpecIndex); + } + return true; } // Determines if the specified is a C++ class or struct containing @@ -7212,34 +7626,36 @@ bool CheckPrintfHandler::HandlePrintfSpecifier( if (!CheckNumArgs(FS, CS, startSpecifier, specifierLen, argIndex + 1)) return false; - // Claim the second argument. - CoveredArgs.set(argIndex + 1); - - // Type check the first argument (int for %b, pointer for %D) - const Expr *Ex = getDataArg(argIndex); - const analyze_printf::ArgType &AT = - (CS.getKind() == ConversionSpecifier::FreeBSDbArg) ? - ArgType(S.Context.IntTy) : ArgType::CPointerTy; - if (AT.isValid() && !AT.matchesType(S.Context, Ex->getType())) - EmitFormatDiagnostic( - S.PDiag(diag::warn_format_conversion_argument_type_mismatch) - << AT.getRepresentativeTypeName(S.Context) << Ex->getType() - << false << Ex->getSourceRange(), - Ex->getBeginLoc(), /*IsStringLocation*/ false, - getSpecifierRange(startSpecifier, specifierLen)); - - // Type check the second argument (char * for both %b and %D) - Ex = getDataArg(argIndex + 1); - const analyze_printf::ArgType &AT2 = ArgType::CStrTy; - if (AT2.isValid() && !AT2.matchesType(S.Context, Ex->getType())) - EmitFormatDiagnostic( - S.PDiag(diag::warn_format_conversion_argument_type_mismatch) - << AT2.getRepresentativeTypeName(S.Context) << Ex->getType() - << false << Ex->getSourceRange(), - Ex->getBeginLoc(), /*IsStringLocation*/ false, - getSpecifierRange(startSpecifier, specifierLen)); - - return true; + if (HasFormatArguments()) { + // Claim the second argument. + CoveredArgs.set(argIndex + 1); + + // Type check the first argument (int for %b, pointer for %D) + const Expr *Ex = getDataArg(argIndex); + const analyze_printf::ArgType &AT = + (CS.getKind() == ConversionSpecifier::FreeBSDbArg) + ? ArgType(S.Context.IntTy) + : ArgType::CPointerTy; + if (AT.isValid() && !AT.matchesType(S.Context, Ex->getType())) + EmitFormatDiagnostic( + S.PDiag(diag::warn_format_conversion_argument_type_mismatch) + << AT.getRepresentativeTypeName(S.Context) << Ex->getType() + << false << Ex->getSourceRange(), + Ex->getBeginLoc(), /*IsStringLocation*/ false, + getSpecifierRange(startSpecifier, specifierLen)); + + // Type check the second argument (char * for both %b and %D) + Ex = getDataArg(argIndex + 1); + const analyze_printf::ArgType &AT2 = ArgType::CStrTy; + if (AT2.isValid() && !AT2.matchesType(S.Context, Ex->getType())) + EmitFormatDiagnostic( + S.PDiag(diag::warn_format_conversion_argument_type_mismatch) + << AT2.getRepresentativeTypeName(S.Context) << Ex->getType() + << false << Ex->getSourceRange(), + Ex->getBeginLoc(), /*IsStringLocation*/ false, + getSpecifierRange(startSpecifier, specifierLen)); + } + return true; } // Check for using an Objective-C specific conversion specifier @@ -7359,7 +7775,7 @@ bool CheckPrintfHandler::HandlePrintfSpecifier( HandleNonStandardConversionSpecifier(CS, startSpecifier, specifierLen); // The remaining checks depend on the data arguments. - if (ArgPassingKind == Sema::FAPK_VAList) + if (!HasFormatArguments()) return true; if (!CheckNumArgs(FS, CS, startSpecifier, specifierLen, argIndex)) @@ -8016,7 +8432,7 @@ bool CheckScanfHandler::HandleScanfSpecifier( HandleNonStandardConversionSpecifier(CS, startSpecifier, specifierLen); // The remaining checks depend on the data arguments. - if (ArgPassingKind == Sema::FAPK_VAList) + if (!HasFormatArguments()) return true; if (!CheckNumArgs(FS, CS, startSpecifier, specifierLen, argIndex)) @@ -8074,8 +8490,67 @@ bool CheckScanfHandler::HandleScanfSpecifier( return true; } +static bool CompareFormatSpecifiers(Sema &S, const StringLiteral *Ref, + ArrayRef RefArgs, + const StringLiteral *Fmt, + ArrayRef FmtArgs, + const Expr *FmtExpr, bool InFunctionCall) { + bool HadError = false; + auto FmtIter = FmtArgs.begin(), FmtEnd = FmtArgs.end(); + auto RefIter = RefArgs.begin(), RefEnd = RefArgs.end(); + while (FmtIter < FmtEnd && RefIter < RefEnd) { + // In positional-style format strings, the same specifier can appear + // multiple times (like %2$i %2$d). Specifiers in both RefArgs and FmtArgs + // are sorted by getPosition(), and we process each range of equal + // getPosition() values as one group. + // RefArgs are taken from a string literal that was given to + // attribute(format_matches), and if we got this far, we have already + // verified that if it has positional specifiers that appear in multiple + // locations, then they are all mutually compatible. What's left for us to + // do is verify that all specifiers with the same position in FmtArgs are + // compatible with the RefArgs specifiers. We check each specifier from + // FmtArgs against the first member of the RefArgs group. + for (; FmtIter < FmtEnd; ++FmtIter) { + // Clang does not diagnose missing format specifiers in positional-style + // strings (TODO: which it probably should do, as it is UB to skip over a + // format argument). Skip specifiers if needed. + if (FmtIter->getPosition() < RefIter->getPosition()) + continue; + + // Delimits a new getPosition() value. + if (FmtIter->getPosition() > RefIter->getPosition()) + break; + + HadError |= + !FmtIter->VerifyCompatible(S, *RefIter, FmtExpr, InFunctionCall); + } + + // Jump RefIter to the start of the next group. + RefIter = std::find_if(RefIter + 1, RefEnd, [=](const auto &Arg) { + return Arg.getPosition() != RefIter->getPosition(); + }); + } + + if (FmtIter < FmtEnd) { + CheckFormatHandler::EmitFormatDiagnostic( + S, InFunctionCall, FmtExpr, + S.PDiag(diag::warn_format_cmp_specifier_arity) << 1, + FmtExpr->getBeginLoc(), false, FmtIter->getSourceRange()); + HadError = S.Diag(Ref->getBeginLoc(), diag::note_format_cmp_with) << 1; + } else if (RefIter < RefEnd) { + CheckFormatHandler::EmitFormatDiagnostic( + S, InFunctionCall, FmtExpr, + S.PDiag(diag::warn_format_cmp_specifier_arity) << 0, + FmtExpr->getBeginLoc(), false, Fmt->getSourceRange()); + HadError = S.Diag(Ref->getBeginLoc(), diag::note_format_cmp_with) + << 1 << RefIter->getSourceRange(); + } + return !HadError; +} + static void CheckFormatString( - Sema &S, const FormatStringLiteral *FExpr, const Expr *OrigFormatExpr, + Sema &S, const FormatStringLiteral *FExpr, + const StringLiteral *ReferenceFormatString, const Expr *OrigFormatExpr, ArrayRef Args, Sema::FormatArgumentPassingKind APK, unsigned format_idx, unsigned firstDataArg, Sema::FormatStringType Type, bool inFunctionCall, Sema::VariadicCallType CallType, @@ -8130,16 +8605,22 @@ static void CheckFormatString( Type == Sema::FST_Kprintf || Type == Sema::FST_FreeBSDKPrintf || Type == Sema::FST_OSLog || Type == Sema::FST_OSTrace || Type == Sema::FST_Syslog) { - CheckPrintfHandler H( - S, FExpr, OrigFormatExpr, Type, firstDataArg, numDataArgs, - (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_Kprintf || Type == Sema::FST_FreeBSDKPrintf)) - H.DoneProcessing(); + bool IsObjC = Type == Sema::FST_NSString || Type == Sema::FST_OSTrace; + if (ReferenceFormatString == nullptr) { + CheckPrintfHandler H(S, FExpr, OrigFormatExpr, Type, firstDataArg, + numDataArgs, IsObjC, 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_Kprintf || Type == Sema::FST_FreeBSDKPrintf)) + H.DoneProcessing(); + } else { + S.CheckFormatStringsCompatible( + Type, ReferenceFormatString, FExpr->getFormatString(), + inFunctionCall ? nullptr : Args[format_idx]); + } } else if (Type == Sema::FST_Scanf) { CheckScanfHandler H(S, FExpr, OrigFormatExpr, Type, firstDataArg, numDataArgs, Str, APK, Args, format_idx, inFunctionCall, @@ -8151,6 +8632,74 @@ static void CheckFormatString( } // TODO: handle other formats } +bool Sema::CheckFormatStringsCompatible( + FormatStringType Type, const StringLiteral *AuthoritativeFormatString, + const StringLiteral *TestedFormatString, const Expr *FunctionCallArg) { + if (Type != Sema::FST_Printf && Type != Sema::FST_NSString && + Type != Sema::FST_Kprintf && Type != Sema::FST_FreeBSDKPrintf && + Type != Sema::FST_OSLog && Type != Sema::FST_OSTrace && + Type != Sema::FST_Syslog) + return true; + + bool IsObjC = Type == Sema::FST_NSString || Type == Sema::FST_OSTrace; + llvm::SmallVector RefArgs, FmtArgs; + FormatStringLiteral RefLit = AuthoritativeFormatString; + FormatStringLiteral TestLit = TestedFormatString; + const Expr *Arg; + bool DiagAtStringLiteral; + if (FunctionCallArg) { + Arg = FunctionCallArg; + DiagAtStringLiteral = false; + } else { + Arg = TestedFormatString; + DiagAtStringLiteral = true; + } + if (DecomposePrintfHandler::GetSpecifiers(*this, &RefLit, + AuthoritativeFormatString, Type, + IsObjC, true, RefArgs) && + DecomposePrintfHandler::GetSpecifiers(*this, &TestLit, Arg, Type, IsObjC, + DiagAtStringLiteral, FmtArgs)) { + return CompareFormatSpecifiers(*this, AuthoritativeFormatString, RefArgs, + TestedFormatString, FmtArgs, Arg, + DiagAtStringLiteral); + } + return false; +} + +bool Sema::ValidateFormatString(FormatStringType Type, + const StringLiteral *Str) { + if (Type != Sema::FST_Printf && Type != Sema::FST_NSString && + Type != Sema::FST_Kprintf && Type != Sema::FST_FreeBSDKPrintf && + Type != Sema::FST_OSLog && Type != Sema::FST_OSTrace && + Type != Sema::FST_Syslog) + return true; + + FormatStringLiteral RefLit = Str; + llvm::SmallVector Args; + bool IsObjC = Type == Sema::FST_NSString || Type == Sema::FST_OSTrace; + if (!DecomposePrintfHandler::GetSpecifiers(*this, &RefLit, Str, Type, IsObjC, + true, Args)) + return false; + + // Group arguments by getPosition() value, and check that each member of the + // group is compatible with the first member. This verifies that when + // positional arguments are used multiple times (such as %2$i %2$d), all uses + // are mutually compatible. As an optimization, don't test the first member + // against itself. + bool HadError = false; + auto Iter = Args.begin(); + auto End = Args.end(); + while (Iter != End) { + const auto &FirstInGroup = *Iter; + for (++Iter; + Iter != End && Iter->getPosition() == FirstInGroup.getPosition(); + ++Iter) { + HadError |= !Iter->VerifyCompatible(*this, FirstInGroup, Str, true); + } + } + return !HadError; +} + bool Sema::FormatStringHasSArg(const StringLiteral *FExpr) { // Str - The format string. NOTE: this is NOT null-terminated! StringRef StrRef = FExpr->getString(); diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 4b5351410f459..285bd27a35a76 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -2818,6 +2818,9 @@ static bool mergeDeclAttribute(Sema &S, NamedDecl *D, else if (const auto *FA = dyn_cast(Attr)) NewAttr = S.mergeFormatAttr(D, *FA, FA->getType(), FA->getFormatIdx(), FA->getFirstArg()); + else if (const auto *FMA = dyn_cast(Attr)) + NewAttr = S.mergeFormatMatchesAttr( + D, *FMA, FMA->getType(), FMA->getFormatIdx(), FMA->getFormatString()); else if (const auto *SA = dyn_cast(Attr)) NewAttr = S.mergeSectionAttr(D, *SA, SA->getName()); else if (const auto *CSA = dyn_cast(Attr)) diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 620290af9509f..fdf3b9c636127 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -3701,61 +3701,95 @@ FormatAttr *Sema::mergeFormatAttr(Decl *D, const AttributeCommonInfo &CI, return ::new (Context) FormatAttr(Context, CI, Format, FormatIdx, FirstArg); } +FormatMatchesAttr *Sema::mergeFormatMatchesAttr(Decl *D, + const AttributeCommonInfo &CI, + IdentifierInfo *Format, + int FormatIdx, + StringLiteral *FormatStr) { + // Check whether we already have an equivalent FormatMatches attribute. + for (auto *F : D->specific_attrs()) { + if (F->getType() == Format && F->getFormatIdx() == FormatIdx) { + if (!CheckFormatStringsCompatible(GetFormatStringType(Format->getName()), + F->getFormatString(), FormatStr)) + return nullptr; + + // If we don't have a valid location for this attribute, adopt the + // location. + if (F->getLocation().isInvalid()) + F->setRange(CI.getRange()); + return nullptr; + } + } + + return ::new (Context) + FormatMatchesAttr(Context, CI, Format, FormatIdx, FormatStr); +} + +struct FormatAttrCommon { + FormatAttrKind Kind; + IdentifierInfo *Identifier; + unsigned NumArgs; + unsigned FormatStringIdx; +}; + /// Handle __attribute__((format(type,idx,firstarg))) attributes based on /// http://gcc.gnu.org/onlinedocs/gcc/Function-Attributes.html -static void handleFormatAttr(Sema &S, Decl *D, const ParsedAttr &AL) { +static bool handleFormatAttrCommon(Sema &S, Decl *D, const ParsedAttr &AL, + FormatAttrCommon *Info) { + // Checks the first two arguments of the attribute; this is shared between + // Format and FormatMatches attributes. + if (!AL.isArgIdent(0)) { S.Diag(AL.getLoc(), diag::err_attribute_argument_n_type) << AL << 1 << AANT_ArgumentIdentifier; - return; + return false; } // In C++ the implicit 'this' function parameter also counts, and they are // counted from one. bool HasImplicitThisParam = isInstanceMethod(D); - unsigned NumArgs = getFunctionOrMethodNumParams(D) + HasImplicitThisParam; + Info->NumArgs = getFunctionOrMethodNumParams(D) + HasImplicitThisParam; - IdentifierInfo *II = AL.getArgAsIdent(0)->Ident; - StringRef Format = II->getName(); + Info->Identifier = AL.getArgAsIdent(0)->Ident; + StringRef Format = Info->Identifier->getName(); if (normalizeName(Format)) { // If we've modified the string name, we need a new identifier for it. - II = &S.Context.Idents.get(Format); + Info->Identifier = &S.Context.Idents.get(Format); } // Check for supported formats. - FormatAttrKind Kind = getFormatAttrKind(Format); + Info->Kind = getFormatAttrKind(Format); - if (Kind == IgnoredFormat) - return; + if (Info->Kind == IgnoredFormat) + return false; - if (Kind == InvalidFormat) { + if (Info->Kind == InvalidFormat) { S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported) - << AL << II->getName(); - return; + << AL << Info->Identifier->getName(); + return false; } // checks for the 2nd argument Expr *IdxExpr = AL.getArgAsExpr(1); - uint32_t Idx; - if (!S.checkUInt32Argument(AL, IdxExpr, Idx, 2)) - return; + if (!S.checkUInt32Argument(AL, IdxExpr, Info->FormatStringIdx, 2)) + return false; - if (Idx < 1 || Idx > NumArgs) { + if (Info->FormatStringIdx < 1 || Info->FormatStringIdx > Info->NumArgs) { S.Diag(AL.getLoc(), diag::err_attribute_argument_out_of_bounds) << AL << 2 << IdxExpr->getSourceRange(); - return; + return false; } // FIXME: Do we need to bounds check? - unsigned ArgIdx = Idx - 1; + unsigned ArgIdx = Info->FormatStringIdx - 1; if (HasImplicitThisParam) { if (ArgIdx == 0) { S.Diag(AL.getLoc(), diag::err_format_attribute_implicit_this_format_string) - << IdxExpr->getSourceRange(); - return; + << IdxExpr->getSourceRange(); + return false; } ArgIdx--; } @@ -3767,10 +3801,19 @@ static void handleFormatAttr(Sema &S, Decl *D, const ParsedAttr &AL) { (!Ty->isPointerType() || !Ty->castAs()->getPointeeType()->isCharType())) { S.Diag(AL.getLoc(), diag::err_format_attribute_not) - << IdxExpr->getSourceRange() << getFunctionOrMethodParamRange(D, ArgIdx); - return; + << IdxExpr->getSourceRange() + << getFunctionOrMethodParamRange(D, ArgIdx); + return false; } + return true; +} + +static void handleFormatAttr(Sema &S, Decl *D, const ParsedAttr &AL) { + FormatAttrCommon Info; + if (!handleFormatAttrCommon(S, D, AL, &Info)) + return; + // check the 3rd argument Expr *FirstArgExpr = AL.getArgAsExpr(2); uint32_t FirstArg; @@ -3779,7 +3822,7 @@ static void handleFormatAttr(Sema &S, Decl *D, const ParsedAttr &AL) { // FirstArg == 0 is is always valid. if (FirstArg != 0) { - if (Kind == StrftimeFormat) { + if (Info.Kind == StrftimeFormat) { // If the kind is strftime, FirstArg must be 0 because strftime does not // use any variadic arguments. S.Diag(AL.getLoc(), diag::err_format_strftime_third_parameter) @@ -3790,17 +3833,17 @@ static void handleFormatAttr(Sema &S, Decl *D, const ParsedAttr &AL) { // Else, if the function is variadic, then FirstArg must be 0 or the // "position" of the ... parameter. It's unusual to use 0 with variadic // functions, so the fixit proposes the latter. - if (FirstArg != NumArgs + 1) { + if (FirstArg != Info.NumArgs + 1) { S.Diag(AL.getLoc(), diag::err_attribute_argument_out_of_bounds) << AL << 3 << FirstArgExpr->getSourceRange() << FixItHint::CreateReplacement(FirstArgExpr->getSourceRange(), - std::to_string(NumArgs + 1)); + std::to_string(Info.NumArgs + 1)); return; } } else { // Inescapable GCC compatibility diagnostic. S.Diag(D->getLocation(), diag::warn_gcc_requires_variadic_function) << AL; - if (FirstArg <= Idx) { + if (FirstArg <= Info.FormatStringIdx) { // Else, the function is not variadic, and FirstArg must be 0 or any // parameter after the format parameter. We don't offer a fixit because // there are too many possible good values. @@ -3811,11 +3854,33 @@ static void handleFormatAttr(Sema &S, Decl *D, const ParsedAttr &AL) { } } - FormatAttr *NewAttr = S.mergeFormatAttr(D, AL, II, Idx, FirstArg); + FormatAttr *NewAttr = + S.mergeFormatAttr(D, AL, Info.Identifier, Info.FormatStringIdx, FirstArg); if (NewAttr) D->addAttr(NewAttr); } +static void handleFormatMatchesAttr(Sema &S, Decl *D, const ParsedAttr &AL) { + FormatAttrCommon Info; + if (!handleFormatAttrCommon(S, D, AL, &Info)) + return; + + Expr *FormatStrExpr = AL.getArgAsExpr(2)->IgnoreParenImpCasts(); + if (auto *SL = dyn_cast(FormatStrExpr)) { + Sema::FormatStringType FST = + S.GetFormatStringType(Info.Identifier->getName()); + if (S.ValidateFormatString(FST, SL)) + if (auto *NewAttr = S.mergeFormatMatchesAttr(D, AL, Info.Identifier, + Info.FormatStringIdx, SL)) + D->addAttr(NewAttr); + return; + } + + S.Diag(AL.getLoc(), diag::err_format_nonliteral) + << FormatStrExpr->getSourceRange(); + return; +} + /// Handle __attribute__((callback(CalleeIdx, PayloadIdx0, ...))) attributes. static void handleCallbackAttr(Sema &S, Decl *D, const ParsedAttr &AL) { // The index that identifies the callback callee is mandatory. @@ -6850,6 +6915,9 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL, case ParsedAttr::AT_Format: handleFormatAttr(S, D, AL); break; + case ParsedAttr::AT_FormatMatches: + handleFormatMatchesAttr(S, D, AL); + break; case ParsedAttr::AT_FormatArg: handleFormatArgAttr(S, D, AL); break; diff --git a/clang/lib/Sema/SemaObjC.cpp b/clang/lib/Sema/SemaObjC.cpp index 57eda18c2d8e1..073d9791d037b 100644 --- a/clang/lib/Sema/SemaObjC.cpp +++ b/clang/lib/Sema/SemaObjC.cpp @@ -2259,7 +2259,8 @@ void SemaObjC::handleExternallyRetainedAttr(Decl *D, const ParsedAttr &AL) { bool SemaObjC::GetFormatNSStringIdx(const FormatAttr *Format, unsigned &Idx) { Sema::FormatStringInfo FSI; if ((SemaRef.GetFormatStringType(Format) == Sema::FST_NSString) && - SemaRef.getFormatStringInfo(Format, false, true, &FSI)) { + SemaRef.getFormatStringInfo(Format->getFormatIdx(), Format->getFirstArg(), + false, true, &FSI)) { Idx = FSI.FormatIdx; return true; } diff --git a/clang/test/Sema/format-string-matches.c b/clang/test/Sema/format-string-matches.c new file mode 100644 index 0000000000000..9a6e591659589 --- /dev/null +++ b/clang/test/Sema/format-string-matches.c @@ -0,0 +1,273 @@ +// RUN: %clang_cc1 -verify -fblocks -fsyntax-only -Wformat-nonliteral -Wformat-signedness -isystem %S/Inputs %s +// RUN: %clang_cc1 -verify -fblocks -fsyntax-only -Wformat-nonliteral -Wformat-signedness -isystem %S/Inputs -fno-signed-char %s + +#include + +__attribute__((format_matches(printf, -1, "%s"))) // expected-error{{'format_matches' attribute parameter 2 is out of bounds}} +int test_out_of_bounds(void); + +__attribute__((format_matches(printf, 0, "%s"))) // expected-error{{'format_matches' attribute parameter 2 is out of bounds}} +int test_out_of_bounds(void); + +__attribute__((format_matches(printf, 1, "%s"))) // expected-error{{'format_matches' attribute parameter 2 is out of bounds}} +int test_out_of_bounds(void); + +__attribute__((format_matches(printf, 1, "%s"))) // expected-error{{format argument not a string type}} +int test_out_of_bounds_int(int x); + +// MARK: - +// Calling printf with a format from format_matches(printf) diagnoses with +// that format string +__attribute__((format(printf, 1, 2))) +int printf(const char *fmt, ...); + +__attribute__((format(printf, 1, 0))) +int vprintf(const char *fmt, va_list); + +__attribute__((format_matches(printf, 1, "%s %1.5s"))) +void format_str_str0(const char *fmt) { + printf(fmt, "hello", "world"); +} + +__attribute__((format_matches(printf, 1, "%s" "%1.5s"))) +void format_str_str1(const char *fmt) { + printf(fmt, "hello", "world"); +} + +__attribute__((format_matches(printf, 1, ("%s" "%1.5s") + 5))) // expected-error{{format string is not a string literal}} +void format_str_str2(const char *fmt); + +__attribute__((format_matches(printf, 1, "%s %g"))) // expected-note{{format string is defined here}} +void format_str_double_warn(const char *fmt) { + printf(fmt, "hello", "world"); // expected-warning{{format specifies type 'double' but the argument has type 'char *'}} +} + +__attribute__((format_matches(printf, 1, "%s %g"))) +void vformat(const char *fmt, ...) { + va_list ap; + va_start(ap, fmt); + vprintf(fmt, ap); // XXX: ideally this would be a diagnostic + va_end(ap); +} + +// MARK: - +// Calling a function with format_matches diagnoses for incompatible formats. + +void cvt_percent(const char *c) __attribute__((format_matches(printf, 1, "%%"))); // expected-note 2{{comparing with this format string}} +void cvt_at(const char *c) __attribute__((format_matches(NSString, 1, "%@"))); // \ + expected-note{{comparing with this specifier}} \ + expected-note 3{{comparing with this format string}} +void cvt_c(const char *c) __attribute__((format_matches(printf, 1, "%c"))); // expected-note{{comparing with this specifier}} +void cvt_u(const char *c) __attribute__((format_matches(printf, 1, "%u"))); // expected-note 2{{comparing with this specifier}} +void cvt_hhi(const char *c) __attribute__((format_matches(printf, 1, "%hhi"))); // expected-note 3{{comparing with this specifier}} +void cvt_i(const char *c) __attribute__((format_matches(printf, 1, "%i"))); // expected-note 5{{comparing with this specifier}} +void cvt_p(const char *c) __attribute__((format_matches(printf, 1, "%p"))); +void cvt_s(const char *c) __attribute__((format_matches(printf, 1, "%s"))); // expected-note{{comparing with this specifier}} +void cvt_n(const char *c) __attribute__((format_matches(printf, 1, "%n"))); // expected-note{{comparing with this specifier}} + +void test_compatibility(void) { + cvt_c("%i"); + const char *const fmt_i = "%i"; + cvt_c(fmt_i); + + cvt_i("%c"); + cvt_c("%u"); // expected-warning{{signedness of format specifier 'u' is incompatible with 'c'}} + cvt_u("%c"); // expected-warning{{signedness of format specifier 'c' is incompatible with 'u'}} + + const char *const fmt_c = "%c"; // expected-note{{format string is defined here}} + cvt_u(fmt_c); // expected-warning{{signedness of format specifier 'c' is incompatible with 'u'}} + + cvt_i("%hi"); // expected-warning{{format specifier 'hi' is incompatible with 'i'}} + cvt_i("%hhi"); // expected-warning{{format specifier 'hhi' is incompatible with 'i'}} + cvt_i("%lli"); // expected-warning{{format specifier 'lli' is incompatible with 'i'}} + cvt_i("%p"); // expected-warning{{format specifier 'p' is incompatible with 'i'}} + cvt_hhi("%hhi"); + cvt_hhi("%hi"); // expected-warning{{format specifier 'hi' is incompatible with 'hhi'}} + cvt_hhi("%i"); // expected-warning{{format specifier 'i' is incompatible with 'hhi'}} + cvt_hhi("%li"); // expected-warning{{format specifier 'li' is incompatible with 'hhi'}} + cvt_n("%s"); // expected-warning{{format specifier 's' is incompatible with 'n'}} + cvt_s("%hhn"); // expected-warning{{format specifier 'hhn' is incompatible with 's'}} + + cvt_p("%@"); // expected-warning{{invalid conversion specifier '@'}} + cvt_at("%p"); // expected-warning{{format specifier 'p' is incompatible with '@'}} + + cvt_percent("hello"); + cvt_percent("%c"); // expected-warning{{more specifiers in format string than expected}} + + const char *const too_many = "%c"; // expected-note{{format string is defined here}} + cvt_percent(too_many); // expected-warning{{more specifiers in format string than expected}} +} + +void test_too_few_args(void) { + cvt_at("a"); // expected-warning{{fewer specifiers in format string than expected}} + cvt_at("%@ %@"); // expected-warning{{more specifiers in format string than expected}} + + const char *const too_few = "a"; // expected-note{{format string is defined here}} + cvt_at(too_few); // expected-warning{{fewer specifiers in format string than expected}} +} + +void cvt_several(const char *c) __attribute__((format_matches(printf, 1, "%f %i %s"))); // expected-note{{comparing with this specifier}} + +void test_moving_args_around(void) { + cvt_several("%1g %-d %1.5s"); + + cvt_several("%3$s %1$g %2$i"); + + cvt_several("%f %*s"); // expected-warning{{format argument is an indirect field width, but it should be a value}} +} + +void cvt_freebsd_D(const char *c) __attribute__((format_matches(freebsd_kprintf, 1, "%D"))); // expected-note{{comparing with this specifier}} + +void test_freebsd_specifiers(void) { + cvt_freebsd_D("%D"); + cvt_freebsd_D("%b"); + cvt_freebsd_D("%s %i"); // expected-warning{{format argument is a value, but it should be an auxiliary value}} +} + +// passing the wrong kind of string literal +void takes_printf_string(const char *fmt) __attribute__((format_matches(printf, 1, "%s"))); +__attribute__((format_matches(freebsd_kprintf, 1, "%s"))) // expected-note{{format string is defined here}} +void takes_freebsd_kprintf_string(const char *fmt) { + takes_printf_string(fmt); // expected-warning{{passing 'freebsd_kprintf' format string where 'printf' format string is expected}} + + const char *const fmt2 = fmt; + takes_printf_string(fmt2); // expected-warning{{passing 'freebsd_kprintf' format string where 'printf' format string is expected}} +} + +__attribute__((format_matches(printf, 1, "%s"))) // expected-note{{comparing with this specifier}} +__attribute__((format_matches(os_log, 2, "%i"))) // expected-note{{comparing with this specifier}} +void test_recv_multiple_format_strings(const char *fmt1, const char *fmt2); + +__attribute__((format_matches(printf, 1, "%s"))) +__attribute__((format_matches(os_log, 2, "%i"))) +void test_multiple_format_strings(const char *fmt1, const char *fmt2) { + test_recv_multiple_format_strings("%s", "%i"); + test_recv_multiple_format_strings("%s", "%s"); // expected-warning{{format specifier 's' is incompatible with 'i'}} + test_recv_multiple_format_strings("%i", "%i"); // expected-warning{{format specifier 'i' is incompatible with 's'}} + + test_recv_multiple_format_strings(fmt1, fmt2); + test_recv_multiple_format_strings("%.5s", fmt2); + test_recv_multiple_format_strings(fmt1, "%04d"); + + test_recv_multiple_format_strings("%s", fmt1); // expected-warning{{passing 'printf' format string where 'os_log' format string is expected}} + test_recv_multiple_format_strings(fmt2, "%d"); // expected-warning{{passing 'os_log' format string where 'printf' format string is expected}} + + test_recv_multiple_format_strings(fmt2, fmt1); // \ + expected-warning{{passing 'printf' format string where 'os_log' format string is expected}} \ + expected-warning{{passing 'os_log' format string where 'printf' format string is expected}} +} + +__attribute__((format_matches(os_log, 1, "%{public}s"))) // expected-note 4{{comparing with this specifier}} +void call_oslog_public(const char *fmt); + +__attribute__((format_matches(os_log, 1, "%{sensitive}s"))) // expected-note 2{{comparing with this specifier}} +void call_oslog_sensitive(const char *fmt); + +__attribute__((format_matches(os_log, 1, "%{private}s"))) // expected-note 2{{comparing with this specifier}} +void call_oslog_private(const char *fmt); + +void test_oslog(void) { + call_oslog_public("%{public}s"); + call_oslog_public("%{private}s"); // expected-warning{{argument sensitivity is private, but it should be public}} + call_oslog_public("%{sensitive}s"); // expected-warning{{argument sensitivity is sensitive, but it should be public}} + + call_oslog_sensitive("%{public}s"); // expected-warning{{argument sensitivity is public, but it should be sensitive}} + call_oslog_sensitive("%{private}s"); // expected-warning{{argument sensitivity is private, but it should be sensitive}} + call_oslog_sensitive("%{sensitive}s"); + + call_oslog_private("%{public}s"); // expected-warning{{argument sensitivity is public, but it should be private}} + call_oslog_private("%{private}s"); + call_oslog_private("%{sensitive}s"); // expected-warning{{argument sensitivity is sensitive, but it should be private}} + + // expected-warning@+2{{argument sensitivity is private, but it should be public}} + // expected-warning@+1{{format specifier 'i' is incompatible with 's'}} + call_oslog_public("%{private}i"); +} + +// MARK: - +void accept_value(const char *f) __attribute__((format_matches(freebsd_kprintf, 1, "%s%i%i"))); // \ + expected-note 3{{comparing with this specifier}} \ + expected-note 3{{format string is defined here}} +void accept_indirect_field_width(const char *f) __attribute__((format_matches(freebsd_kprintf, 1, "%s%*i"))); // \ + expected-note 3{{comparing with this specifier}} \ + expected-note 3{{format string is defined here}} +void accept_indirect_field_precision(const char *f) __attribute__((format_matches(freebsd_kprintf, 1, "%s%.*i"))); // \ + expected-note 3{{comparing with this specifier}} \ + expected-note 3{{format string is defined here}} +void accept_aux_value(const char *f) __attribute__((format_matches(freebsd_kprintf, 1, "%D%i"))); // \ + expected-note 3{{comparing with this specifier}} \ + expected-note 3{{format string is defined here}} + +void accept_value(const char *f) { + accept_indirect_field_width(f); // expected-warning{{format argument is a value, but it should be an indirect field width}} + accept_indirect_field_precision(f); // expected-warning{{format argument is a value, but it should be an indirect precision}} + accept_aux_value(f); // expected-warning{{format argument is a value, but it should be an auxiliary value}} +} + +void accept_indirect_field_width(const char *f) { + accept_value(f); // expected-warning{{format argument is an indirect field width, but it should be a value}} + accept_indirect_field_precision(f); // expected-warning{{format argument is an indirect field width, but it should be an indirect precision}} + accept_aux_value(f); // expected-warning{{format argument is an indirect field width, but it should be an auxiliary value}} +} + +void accept_indirect_field_precision(const char *f) { + accept_value(f); // expected-warning{{format argument is an indirect precision, but it should be a value}} + accept_indirect_field_width(f); // expected-warning{{format argument is an indirect precision, but it should be an indirect field width}} + accept_aux_value(f); // expected-warning{{format argument is an indirect precision, but it should be an auxiliary value}} +} + +void accept_aux_value(const char *f) { + accept_value(f); // expected-warning{{format argument is an auxiliary value, but it should be a value}} + accept_indirect_field_width(f); // expected-warning{{format argument is an auxiliary value, but it should be an indirect field width}} + accept_indirect_field_precision(f); // expected-warning{{format argument is an auxiliary value, but it should be an indirect precision}} +} + +// MARK: - Merging format attributes +__attribute__((format_matches(printf, 1, "%i"))) +__attribute__((format_matches(printf, 1, "%d"))) +void test_merge_self(const char *f); + +__attribute__((format_matches(printf, 1, "%i"))) // expected-note{{comparing with this specifier}} +__attribute__((format_matches(printf, 1, "%s"))) // expected-warning{{format specifier 's' is incompatible with 'i'}} +void test_merge_self_warn(const char *f); + +__attribute__((format_matches(printf, 1, "%i"))) +void test_merge_redecl(const char *f); + +__attribute__((format_matches(printf, 1, "%d"))) +void test_merge_redecl(const char *f); + +// XXX: ideally the warning and note would be swapped, but this is entirely +// reliant on which decl clang considers to be the "true one", and it might +// upset something else more important if we tried to change it. +__attribute__((format_matches(printf, 1, "%i"))) // expected-warning{{format specifier 'i' is incompatible with 's'}} +void test_merge_redecl_warn(const char *f); + +__attribute__((format_matches(printf, 1, "%s"))) // expected-note{{comparing with this specifier}} +void test_merge_redecl_warn(const char *f); + +// MARK: - +// Positional madness + +__attribute__((format_matches(printf, 1, "%1$s %1$d"))) // \ + expected-warning{{format specifier 'd' is incompatible with 's'}} \ + expected-note{{comparing with this specifier}} +void test_positional_incompatible(const char *f); + +void call_positional_incompatible(void) { + // expect the attribute was dropped and that there is no diagnostic here + test_positional_incompatible("%d %d %d %d %d"); +} + +void test_many_i(void) { + cvt_i("%1$d %1$i"); + cvt_i("%1$d %1$s"); // expected-warning{{format specifier 's' is incompatible with 'i'}} +} + +__attribute__((format_matches(printf, 1, "%*d %*d"))) // expected-note{{comparing with this specifier}} +void accept_modifiers(const char *f); + +void test_modifiers(void) { + accept_modifiers("%2$*1$d %4$*3$d"); + accept_modifiers("%2$*3$d %4$*3$d"); // expected-warning{{format argument modifies specifier at position 2, but it should modify specifier at position 4}} +} diff --git a/clang/test/Sema/format-strings.c b/clang/test/Sema/format-strings.c index 04bad1f13c8cd..efd88d5c18e66 100644 --- a/clang/test/Sema/format-strings.c +++ b/clang/test/Sema/format-strings.c @@ -94,7 +94,7 @@ void check_string_literal2( FILE* fp, const char* s, char *buf, ... ) { // expected-note@-1{{treat the string as an argument to avoid this}} __builtin___vsnprintf_chk(buf,2,0,-1,s,ap); // no-warning - vscanf(s, ap); // expected-warning {{format string is not a string literal}} + vscanf(s, ap); // expected-warning {{passing 'printf' format string where 'scanf' format string is expected}} } void check_conditional_literal(const char* s, int i) {