Index: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp =================================================================== --- clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -41,6 +41,7 @@ #include "NotNullTerminatedResultCheck.h" #include "ParentVirtualCallCheck.h" #include "PosixReturnCheck.h" +#include "PrintfWithFixedSizeBufferCheck.h" #include "RedundantBranchConditionCheck.h" #include "ReservedIdentifierCheck.h" #include "SharedPtrArrayMismatchCheck.h" @@ -132,6 +133,8 @@ "bugprone-move-forwarding-reference"); CheckFactories.registerCheck( "bugprone-multiple-statement-macro"); + CheckFactories.registerCheck( + "bugprone-printf-with-fixed-size-buffer"); CheckFactories.registerCheck( "bugprone-redundant-branch-condition"); CheckFactories.registerCheck( Index: clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt =================================================================== --- clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -36,6 +36,7 @@ NotNullTerminatedResultCheck.cpp ParentVirtualCallCheck.cpp PosixReturnCheck.cpp + PrintfWithFixedSizeBufferCheck.cpp RedundantBranchConditionCheck.cpp ReservedIdentifierCheck.cpp SharedPtrArrayMismatchCheck.cpp Index: clang-tools-extra/clang-tidy/bugprone/PrintfWithFixedSizeBufferCheck.h =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/bugprone/PrintfWithFixedSizeBufferCheck.h @@ -0,0 +1,47 @@ +//===--- PrintfWithFixedSizeBufferCheck.h - clang-tidy ----------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_PRINTFWITHFIXEDSIZEBUFFERCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_PRINTFWITHFIXEDSIZEBUFFERCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace bugprone { + +/// Check if `sprintf` is called with a fixed size buffer. Replaced with counted +/// version `snprintf`. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/sprintf-with-fixed-size-buffer.html +class PrintfWithFixedSizeBufferCheck : public ClangTidyCheck { +public: + PrintfWithFixedSizeBufferCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + PreferSafe(Options.get("PreferSafe", false)) {} + + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } + + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + + /// Registering for sprintf and vsprintf calls. + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + bool PreferSafe; +}; + +} // namespace bugprone +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_PRINTFWITHFIXEDSIZEBUFFERCHECK_H Index: clang-tools-extra/clang-tidy/bugprone/PrintfWithFixedSizeBufferCheck.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/bugprone/PrintfWithFixedSizeBufferCheck.cpp @@ -0,0 +1,92 @@ +//===--- PrintfWithFixedSizeBufferCheck.cpp - clang-tidy ------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "PrintfWithFixedSizeBufferCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" +#include "llvm/ADT/StringRef.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace bugprone { + +void PrintfWithFixedSizeBufferCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "PreferSafe", PreferSafe); +} + +void PrintfWithFixedSizeBufferCheck::registerMatchers(MatchFinder *Finder) { + auto HasConstantArrayType = hasType(constantArrayType()); + auto HasFixedSizeBufferMatcher = + hasArgument(0, anyOf(memberExpr(HasConstantArrayType).bind("MemberExpr"), + declRefExpr(HasConstantArrayType).bind("DeclRef"))); + + Finder->addMatcher(callExpr(allOf(HasFixedSizeBufferMatcher, + callee(functionDecl(hasName("::sprintf"))))) + .bind("sprintf"), + this); + + Finder->addMatcher( + callExpr(allOf(HasFixedSizeBufferMatcher, + callee(functionDecl(hasName("::vsprintf"))))) + .bind("vsprintf"), + this); +} + +void PrintfWithFixedSizeBufferCheck::check( + const MatchFinder::MatchResult &Result) { + const Expr *CA; + const auto *ME = Result.Nodes.getNodeAs("MemberExpr"); + const auto *DR = Result.Nodes.getNodeAs("DeclRef"); + + if (ME != nullptr) + CA = ME; + else + CA = DR; + + const CallExpr *Call = nullptr; + const Expr *SecondArgument = nullptr; + + std::string Recommendation; + if (Call = Result.Nodes.getNodeAs("sprintf")) + Recommendation = "snprintf"; + else if (Call = Result.Nodes.getNodeAs("vsprintf")) + Recommendation = "vsnprintf"; + + assert(Call && "Unhandled binding in the Matcher"); + + SecondArgument = Call->getArg(1); + + if (PreferSafe) { + Recommendation += "_s"; + } + + // Insert the size of buffer after first argument. + std::string SizeofText = "sizeof("; + SizeofText += + Lexer::getSourceText(CharSourceRange::getTokenRange(CA->getSourceRange()), + *Result.SourceManager, getLangOpts()); + SizeofText += "), "; + + FixItHint ReplacementHint = FixItHint::CreateReplacement( + Call->getCallee()->getSourceRange(), Recommendation); + FixItHint SizeofHint = + FixItHint::CreateInsertion(SecondArgument->getBeginLoc(), SizeofText); + auto D = + diag(Call->getBeginLoc(), "string to be written may exceeds the size of " + "buffer; use %0 instead") + << Recommendation << Call->getCallee()->getSourceRange() + << SecondArgument->getSourceRange() << ReplacementHint << SizeofHint; +} + +} // namespace bugprone +} // namespace tidy +} // namespace clang Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -99,6 +99,11 @@ New checks ^^^^^^^^^^ +- New :doc:`bugprone-printf-with-fixed-size-buffer + ` check. + + Finds usage of ``sprintf`` or ``vsprintf``, which write output string into a fixed-size array. + - New :doc:`cppcoreguidelines-avoid-const-or-ref-data-members ` check. Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/printf-with-fixed-size-buffer.rst =================================================================== --- /dev/null +++ clang-tools-extra/docs/clang-tidy/checks/bugprone/printf-with-fixed-size-buffer.rst @@ -0,0 +1,36 @@ +.. title:: clang-tidy - bugprone-printf-with-fixed-size-buffer + + +bugprone-printf-with-fixed-size-buffer +======================================= + +Finds usage of ``sprintf`` or ``vsprintf``, which write output string into a fixed-size array. +It will suggest the counted versions instead. + +It's a common idiom to have a fixed-size buffer of characters allocated on +the stack and then to printf into the buffer. It may be leading to errors if the +string to be written exceeds the size of the array pointed to by buffer. + +Example: +.. code-block:: c++ + + void f(){ + char buff[80]; + sprintf(buff, "Hello, %s!\n", "world"); + } + +Becomes: +.. code-block:: c++ + + void f(){ + char buff[80]; + snprintf(buff, sizeof(buff), "Hello, %s!\n", "world"); + } + +Options +------- + +.. option:: PreferSafe + + When `true`, prefer to use ``snprintf_s`` ``vsnprintf_s`` over ``snprintf`` ``vsnprintf``. + Default is `false`. Index: clang-tools-extra/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -107,6 +107,7 @@ `bugprone-not-null-terminated-result `_, "Yes" `bugprone-parent-virtual-call `_, "Yes" `bugprone-posix-return `_, "Yes" + `bugprone-printf-with-fixed-size-buffer `_, "Yes" `bugprone-redundant-branch-condition `_, "Yes" `bugprone-reserved-identifier `_, "Yes" `bugprone-shared-ptr-array-mismatch `_, "Yes" Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/printf-with-fixed-size-buffer.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/bugprone/printf-with-fixed-size-buffer.cpp @@ -0,0 +1,33 @@ +// RUN: %check_clang_tidy %s bugprone-printf-with-fixed-size-buffer %t + +#include + +void f(){ + char buff[80]; + sprintf(buff, "Hello, %s!\n", "world"); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: string to be written may exceeds the size of buffer; use snprintf instead [bugprone-printf-with-fixed-size-buffer] + // CHECK-FIXES: snprintf(buff, sizeof(buff), "Hello, %s!\n", "world"); +} + +char sbuff[80]; + +void f2(){ + sprintf(sbuff, "Hello, %s!\n", "world"); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: string to be written may exceeds the size of buffer; use snprintf instead [bugprone-printf-with-fixed-size-buffer] + // CHECK-FIXES: snprintf(sbuff, sizeof(sbuff), "Hello, %s!\n", "world"); +} + +void f3(){ + char *dynamic_sized_buff = nullptr; + sprintf(dynamic_sized_buff, "Hello, %s!\n", "world"); +} + +void f4(const char * format, ... ){ + char buff[100]; + va_list args; + va_start(args, format); + vsprintf(buff, format, args); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: string to be written may exceeds the size of buffer; use vsnprintf instead [bugprone-printf-with-fixed-size-buffer] + // CHECK-FIXES: vsnprintf(buff, sizeof(buff), format, args); + va_end(args); +}