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 @@ -48,6 +48,7 @@ #include "SignedCharMisuseCheck.h" #include "SizeofContainerCheck.h" #include "SizeofExpressionCheck.h" +#include "SprintfWithFixedSizeBufferCheck.h" #include "SpuriouslyWakeUpFunctionsCheck.h" #include "StringConstructorCheck.h" #include "StringIntegerAssignmentCheck.h" @@ -153,6 +154,8 @@ "bugprone-sizeof-container"); CheckFactories.registerCheck( "bugprone-sizeof-expression"); + CheckFactories.registerCheck( + "bugprone-sprintf-with-fixed-size-buffer"); CheckFactories.registerCheck( "bugprone-spuriously-wake-up-functions"); 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 @@ -44,6 +44,7 @@ SignedCharMisuseCheck.cpp SizeofContainerCheck.cpp SizeofExpressionCheck.cpp + SprintfWithFixedSizeBufferCheck.cpp SpuriouslyWakeUpFunctionsCheck.cpp StringConstructorCheck.cpp StringIntegerAssignmentCheck.cpp Index: clang-tools-extra/clang-tidy/bugprone/SprintfWithFixedSizeBufferCheck.h =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/bugprone/SprintfWithFixedSizeBufferCheck.h @@ -0,0 +1,45 @@ +//===--- SprintfWithFixedSizeBufferCheck.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_SPRINTFWITHFIXEDSIZEBUFFERCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SPRINTFWITHFIXEDSIZEBUFFERCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace bugprone { + +/// Check if `sprintf` or `vsprintf` is called with a fixed size buffer. +/// Replaced with counted versions function. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/sprintf-with-fixed-size-buffer.html +class SprintfWithFixedSizeBufferCheck : public ClangTidyCheck { +public: + SprintfWithFixedSizeBufferCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + PreferSafe(Options.get("PreferSafe", false)), + FlagAllCalls(Options.get("FlagAllCalls", false)) {} + + 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; + bool FlagAllCalls; +}; + +} // namespace bugprone +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SPRINTFWITHFIXEDSIZEBUFFERCHECK_H Index: clang-tools-extra/clang-tidy/bugprone/SprintfWithFixedSizeBufferCheck.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/bugprone/SprintfWithFixedSizeBufferCheck.cpp @@ -0,0 +1,88 @@ +//===--- SprintfWithFixedSizeBufferCheck.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 "SprintfWithFixedSizeBufferCheck.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 SprintfWithFixedSizeBufferCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "PreferSafe", PreferSafe); + Options.store(Opts, "FlagAllCalls", FlagAllCalls); +} + +void SprintfWithFixedSizeBufferCheck::registerMatchers(MatchFinder *Finder) { + auto SprintfCallee = + callee(functionDecl(hasAnyName("::sprintf", "::vsprintf"))); + + auto HasFixedSizeBuffer = + hasArgument(0, mapAnyOf(memberExpr, declRefExpr) + .with(hasType(constantArrayType())) + .bind("FixedSizeArray")); + + Finder->addMatcher( + callExpr(allOf(HasFixedSizeBuffer, SprintfCallee)).bind("Call"), this); + if (FlagAllCalls) { + Finder->addMatcher( + callExpr(allOf(unless(HasFixedSizeBuffer), SprintfCallee)).bind("Call"), + this); + } +} + +void SprintfWithFixedSizeBufferCheck::check( + const MatchFinder::MatchResult &Result) { + + const auto *FA = Result.Nodes.getNodeAs("FixedSizeArray"); + const auto *Call = Result.Nodes.getNodeAs("Call"); + const auto *Callee = dyn_cast(Call->getCalleeDecl()); + + StringRef Recommendation; + if (Callee->getName() == "sprintf") + Recommendation = "snprintf_s"; + else if (Callee->getName() == "vsprintf") + Recommendation = "vsnprintf_s"; + + if (!PreferSafe) { + Recommendation = Recommendation.drop_back(2); + } + + auto D = + diag(Call->getBeginLoc(), "string to be written may exceeds the size of " + "buffer; use %0 instead") + << Recommendation << Call->getCallee()->getSourceRange(); + + // Generate a fix-it if detected fixed size buffer + if (FA != nullptr) { + const Expr *SecondArgument = Call->getArg(1); + ; + // Insert the size of buffer after first argument. + std::string SizeofText = "sizeof("; + SizeofText += Lexer::getSourceText( + CharSourceRange::getTokenRange(FA->getSourceRange()), + *Result.SourceManager, getLangOpts()); + SizeofText += "), "; + + FixItHint ReplacementHint = FixItHint::CreateReplacement( + Call->getCallee()->getSourceRange(), Recommendation); + FixItHint SizeofHint = + FixItHint::CreateInsertion(SecondArgument->getBeginLoc(), SizeofText); + D << 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-sprintf-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/sprintf-with-fixed-size-buffer.rst =================================================================== --- /dev/null +++ clang-tools-extra/docs/clang-tidy/checks/bugprone/sprintf-with-fixed-size-buffer.rst @@ -0,0 +1,39 @@ +.. title:: clang-tidy - bugprone-sprintf-with-fixed-size-buffer + +bugprone-sprintf-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`. + +.. option:: FlagAllCalls + Flag all calls to ``sprintf`` and ``vsprintf``. + 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 @@ -114,6 +114,7 @@ `bugprone-signed-char-misuse `_, `bugprone-sizeof-container `_, `bugprone-sizeof-expression `_, + `bugprone-sprintf-with-fixed-size-buffer `_, "Yes" `bugprone-spuriously-wake-up-functions `_, `bugprone-string-constructor `_, "Yes" `bugprone-string-integer-assignment `_, "Yes" Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/sprintf-with-fixed-size-buffer.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/bugprone/sprintf-with-fixed-size-buffer.cpp @@ -0,0 +1,33 @@ +// RUN: %check_clang_tidy %s bugprone-sprintf-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); +}