Yoorkin updated this revision to Diff 454722.
Yoorkin added a comment.
Now it can find usage of both `sprintf` and `vsprintf`, and then change a small
piece of code. Here is an option `PreferSafe`, when value is true, the check
prefers `snprintf_s` `vsnprintf_s` over `snprintf` `vsnprintf`.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D132294/new/
https://reviews.llvm.org/D132294
Files:
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/clang-tidy/bugprone/PrintfWithFixedSizeBufferCheck.cpp
clang-tools-extra/clang-tidy/bugprone/PrintfWithFixedSizeBufferCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone/printf-with-fixed-size-buffer.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/printf-with-fixed-size-buffer.cpp
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 <stdio.h>
+
+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);
+}
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 <bugprone/not-null-terminated-result.html>`_, "Yes"
`bugprone-parent-virtual-call <bugprone/parent-virtual-call.html>`_, "Yes"
`bugprone-posix-return <bugprone/posix-return.html>`_, "Yes"
+ `bugprone-printf-with-fixed-size-buffer <bugprone/printf-with-fixed-size-buffer.html>`_, "Yes"
`bugprone-redundant-branch-condition <bugprone/redundant-branch-condition.html>`_, "Yes"
`bugprone-reserved-identifier <bugprone/reserved-identifier.html>`_, "Yes"
`bugprone-shared-ptr-array-mismatch <bugprone/shared-ptr-array-mismatch.html>`_, "Yes"
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/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
+ <clang-tidy/checks/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
<clang-tidy/checks/cppcoreguidelines/avoid-const-or-ref-data-members>` check.
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<bool>("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<bool>(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>("MemberExpr");
+ const auto *DR = Result.Nodes.getNodeAs<DeclRefExpr>("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<CallExpr>("sprintf"))
+ Recommendation = "snprintf";
+ else if (Call = Result.Nodes.getNodeAs<CallExpr>("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/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/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<MultipleStatementMacroCheck>(
"bugprone-multiple-statement-macro");
+ CheckFactories.registerCheck<PrintfWithFixedSizeBufferCheck>(
+ "bugprone-printf-with-fixed-size-buffer");
CheckFactories.registerCheck<RedundantBranchConditionCheck>(
"bugprone-redundant-branch-condition");
CheckFactories.registerCheck<cppcoreguidelines::NarrowingConversionsCheck>(
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
