https://github.com/budimirarandjelovicsyrmia updated https://github.com/llvm/llvm-project/pull/70024
From 845235ed5bb126458a792bc1051bb5b13b78a677 Mon Sep 17 00:00:00 2001 From: budimirarandjelovicsyrmia <budimir.arandjelo...@syrmia.com> Date: Fri, 13 Oct 2023 14:45:15 +0200 Subject: [PATCH] [clang] Catch missing format attributes --- clang/include/clang/Basic/DiagnosticGroups.td | 2 +- .../clang/Basic/DiagnosticSemaKinds.td | 5 + clang/include/clang/Sema/Sema.h | 4 + clang/lib/Sema/SemaChecking.cpp | 4 +- clang/lib/Sema/SemaDeclAttr.cpp | 104 ++++++++++++++++++ clang/test/Sema/attr-format-missing.c | 13 +++ 6 files changed, 130 insertions(+), 2 deletions(-) create mode 100644 clang/test/Sema/attr-format-missing.c diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 17fdcffa2d42740..b8b77df84beb2be 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -482,7 +482,7 @@ def MainReturnType : DiagGroup<"main-return-type">; def MaxUnsignedZero : DiagGroup<"max-unsigned-zero">; def MissingBraces : DiagGroup<"missing-braces">; def MissingDeclarations: DiagGroup<"missing-declarations">; -def : DiagGroup<"missing-format-attribute">; +def MissingFormatAttribute: DiagGroup<"missing-format-attribute">; def : DiagGroup<"missing-include-dirs">; def MissingNoreturn : DiagGroup<"missing-noreturn">; def MultiChar : DiagGroup<"multichar">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 6d6f474f6dcdab9..6864fe3057a1df9 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -936,6 +936,11 @@ def err_opencl_invalid_param : Error< def err_opencl_invalid_return : Error< "declaring function return value of type %0 is not allowed %select{; did you forget * ?|}1">; def warn_enum_value_overflow : Warning<"overflow in enumeration value">; +def warn_missing_format_attribute : Warning< + "diagnostic behavior may be improved by adding the %0 format attribute to the declaration of %1">, + InGroup<MissingFormatAttribute>, DefaultIgnore; +def note_insert_format_attribute_fixit: Note< + "insert %0 to silence this warning">; def warn_pragma_options_align_reset_failed : Warning< "#pragma options align=reset failed: %0">, InGroup<IgnoredPragmas>; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 741c2503127af7a..064506e70960333 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -10615,6 +10615,10 @@ class Sema final { ChangedStateAtExit }; + void DiagnoseMissingFormatAttributes(const FunctionDecl *FDecl, + ArrayRef<const Expr *> Args, + SourceLocation Loc); + void DiagnoseNonDefaultPragmaAlignPack(PragmaAlignPackDiagnoseKind Kind, SourceLocation IncludeLoc); void DiagnoseUnterminatedPragmaAlignPack(); diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 4602284309491c1..d3ac6cb519c56ed 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -6014,8 +6014,10 @@ void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto, } } - if (FD) + if (FD) { diagnoseArgDependentDiagnoseIfAttrs(FD, ThisArg, Args, Loc); + DiagnoseMissingFormatAttributes(FD, Args, Range.getBegin()); + } } /// CheckConstructorCall - Check a constructor call for correctness and safety diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 1a0bfb3d91bcc87..afa9648fdf0069e 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -6849,6 +6849,110 @@ static void handleSwiftAsyncAttr(Sema &S, Decl *D, const ParsedAttr &AL) { checkSwiftAsyncErrorBlock(S, D, ErrorAttr, AsyncAttr); } +// Warn if passed function has format attribute and their parent not. +void Sema::DiagnoseMissingFormatAttributes(const FunctionDecl *FDecl, + ArrayRef<const Expr *> Args, + SourceLocation Loc) { + assert(FDecl); + + unsigned BuiltinID = FDecl->getBuiltinID(/*ConsiderWrappers=*/true); + + // If function is builtin and not listed in switch, exit. + switch (BuiltinID) { + case Builtin::NotBuiltin: + break; + case Builtin::BIprintf: + case Builtin::BIfprintf: + case Builtin::BIsprintf: + case Builtin::BIscanf: + case Builtin::BIfscanf: + case Builtin::BIsscanf: + case Builtin::BIvprintf: + case Builtin::BIvfprintf: + case Builtin::BIvsprintf: + break; + // C99 mode + case Builtin::BIsnprintf: + case Builtin::BIvsnprintf: + case Builtin::BIvscanf: + case Builtin::BIvfscanf: + case Builtin::BIvsscanf: + if (!getLangOpts().C99) + return; + break; + default: + return; + } + + const FunctionDecl *ParentFuncDecl = getCurFunctionDecl(); + if (!ParentFuncDecl) + return; + + // Iterate through function format attributes. Check if parent function + // has these attributes. If parent function does not have format + // attribute, emit warning. + for (const FormatAttr *Attr : FDecl->specific_attrs<FormatAttr>()) { + // Check if parent function has format attribute with correct arguments. + bool HasCorrectFormatAttr = llvm::any_of( + ParentFuncDecl->specific_attrs<FormatAttr>(), + [&](const FormatAttr *ParentAttr) { + // Check if first argument matches child function format argument + // type. + if (ParentAttr->getType() != Attr->getType()) + return false; + + // In C++ the implicit 'this' function parameter also counts + // Number of arguments does not count variadic argument + unsigned NumArgs = getFunctionOrMethodNumParams(ParentFuncDecl) + + isInstanceMethod(ParentFuncDecl); + + // Checks for second argument + unsigned FormatIdx = ParentAttr->getFormatIdx(); + if (FormatIdx < 1 || FormatIdx > NumArgs) + return false; + + // Check if second argument matches correct type. + QualType FormatType = + ParentFuncDecl->parameters()[FormatIdx - 1]->getType(); + if (!isNSStringType(FormatType, Context, true) && + !isCFStringType(FormatType, Context) && + (!FormatType->isPointerType() || + !FormatType->getPointeeType()->isCharType())) + return false; + + // Check third argument, which is valid if it is either zero + // or greater by one than number of parent function arguments. + unsigned FirstArg = ParentAttr->getFirstArg(); + return FirstArg == 0 || FirstArg == NumArgs + 1; + }); + + if (HasCorrectFormatAttr) + continue; + + Diag(Loc, diag::warn_missing_format_attribute) + << Attr->getType() << ParentFuncDecl; + + int StringIndex = 0; + for (ParmVarDecl *Param : ParentFuncDecl->parameters()) { + if (Param->getType() == Args[Attr->getFirstArg()]->getType()) { + StringIndex = Param->getFunctionScopeIndex() + 1; + break; + } + } + + int FirstToCheck = + ParentFuncDecl->isVariadic() ? (ParentFuncDecl->getNumParams() + 1) : 0; + SourceLocation ParentFuncLoc = ParentFuncDecl->getLocation(); + std::string InsertionText; + llvm::raw_string_ostream OS(InsertionText); + OS << "__attribute__((format(" + Attr->getType()->getName() + ", " + + std::to_string(StringIndex) + ", " + + std::to_string(FirstToCheck) + ")))"; + Diag(ParentFuncLoc, diag::note_insert_format_attribute_fixit) + << FixItHint::CreateInsertion(ParentFuncLoc, InsertionText); + } +} + //===----------------------------------------------------------------------===// // Microsoft specific attribute handlers. //===----------------------------------------------------------------------===// diff --git a/clang/test/Sema/attr-format-missing.c b/clang/test/Sema/attr-format-missing.c new file mode 100644 index 000000000000000..1926d3d3163124b --- /dev/null +++ b/clang/test/Sema/attr-format-missing.c @@ -0,0 +1,13 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wmissing-format-attribute %s +// RUN: %clang_cc1 -fsyntax-only -verify -std=c99 -Wmissing-format-attribute %s + +#include <stdarg.h> +#include <stdio.h> + +__attribute__((__format__ (__scanf__, 1, 4))) +void foo(char *out, const size_t len, const char *format, ... /* args */) +{ + va_list args; + + vsnprintf(out, len, format, args); // expected-warning {{Function 'foo' might be candidate for 'printf' format attribute}} +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits