Yoorkin updated this revision to Diff 458155.
Yoorkin marked 5 inline comments as done.
Yoorkin added a comment.
Maybe renamed to bugprone-sprintf-with-fixed-size-buffer would be better. I add
another option `FlagAllCalls` to flag all calls to `sprintf` and `vsprintf`.
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/SprintfWithFixedSizeBufferCheck.cpp
clang-tools-extra/clang-tidy/bugprone/SprintfWithFixedSizeBufferCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone/sprintf-with-fixed-size-buffer.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/sprintf-with-fixed-size-buffer.cpp
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 <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
@@ -114,6 +114,7 @@
`bugprone-signed-char-misuse <bugprone/signed-char-misuse.html>`_,
`bugprone-sizeof-container <bugprone/sizeof-container.html>`_,
`bugprone-sizeof-expression <bugprone/sizeof-expression.html>`_,
+ `bugprone-sprintf-with-fixed-size-buffer <bugprone/sprintf-with-fixed-size-buffer.html>`_, "Yes"
`bugprone-spuriously-wake-up-functions <bugprone/spuriously-wake-up-functions.html>`_,
`bugprone-string-constructor <bugprone/string-constructor.html>`_, "Yes"
`bugprone-string-integer-assignment <bugprone/string-integer-assignment.html>`_, "Yes"
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/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
+ <clang-tidy/checks/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
<clang-tidy/checks/cppcoreguidelines/avoid-const-or-ref-data-members>` check.
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<Expr>("FixedSizeArray");
+ const auto *Call = Result.Nodes.getNodeAs<CallExpr>("Call");
+ const auto *Callee = dyn_cast<FunctionDecl>(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/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/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<SizeofExpressionCheck>(
"bugprone-sizeof-expression");
+ CheckFactories.registerCheck<SprintfWithFixedSizeBufferCheck>(
+ "bugprone-sprintf-with-fixed-size-buffer");
CheckFactories.registerCheck<SpuriouslyWakeUpFunctionsCheck>(
"bugprone-spuriously-wake-up-functions");
CheckFactories.registerCheck<StringConstructorCheck>(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
