baloghadamsoftware updated this revision to Diff 120214.
baloghadamsoftware added a comment.

Updated according to comments.


https://reviews.llvm.org/D39121

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp
  clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-misplaced-operator-in-strlen-in-alloc.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.cpp

Index: test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.cpp
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s bugprone-misplaced-operator-in-strlen-in-alloc %t
+
+typedef __typeof(sizeof(int)) size_t;
+void *malloc(size_t);
+void *alloca(size_t);
+void *calloc(size_t, size_t);
+void *realloc(void *, size_t);
+
+size_t strlen(const char*);
+
+void bad_malloc(char *name) {
+  char *new_name = (char*) malloc(strlen(name + 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: Binary operator + 1 is inside strlen
+  // CHECK-FIXES: {{^  char \*new_name = \(char\*\) malloc\(}}strlen(name) + 1{{\);$}}
+}
+
+void bad_alloca(char *name) {
+  char *new_name = (char*) alloca(strlen(name + 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: Binary operator + 1 is inside strlen
+  // CHECK-FIXES: {{^  char \*new_name = \(char\*\) alloca\(}}strlen(name) + 1{{\);$}}
+}
+
+void bad_calloc(char *name) {
+  char *new_names = (char*) calloc(2, strlen(name + 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: Binary operator + 1 is inside strlen
+  // CHECK-FIXES: {{^  char \*new_names = \(char\*\) calloc\(2, }}strlen(name) + 1{{\);$}}
+}
+
+void bad_realloc(char * old_name, char *name) {
+  char *new_name = (char*) realloc(old_name, strlen(name + 1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: Binary operator + 1 is inside strlen
+  // CHECK-FIXES: {{^  char \*new_name = \(char\*\) realloc\(old_name, }}strlen(name) + 1{{\);$}}
+}
+
+void intentional1(char *name) {
+  char *new_name = (char*) malloc(strlen(name + 1) + 1);
+}
+
+
+void intentional2(char *name) {
+  char *new_name = (char*) malloc(strlen(name + 2));
+}
+
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -18,6 +18,7 @@
    android-cloexec-socket
    boost-use-to-string
    bugprone-integer-division
+   bugprone-misplaced-operator-in-strlen-in-alloc
    bugprone-suspicious-memset-usage
    bugprone-undefined-memory-manipulation
    cert-dcl03-c (redirects to misc-static-assert) <cert-dcl03-c>
Index: docs/clang-tidy/checks/bugprone-misplaced-operator-in-strlen-in-alloc.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/bugprone-misplaced-operator-in-strlen-in-alloc.rst
@@ -0,0 +1,26 @@
+.. title:: clang-tidy - bugprone-misplaced-operator-in-strlen-in-alloc
+
+bugprone-misplaced-operator-in-strlen-in-alloc
+==============================================
+
+Finds cases a value is added to or subtracted from the string in the parameter
+of ``strlen()`` method instead of to the result and use its return value as an
+argument of a memory allocation function (``malloc()``, ``calloc()``,
+``realloc()``).
+
+Example code:
+
+.. code-block:: c
+
+    void bad_malloc(char *str) {
+      char *c = (char*) malloc(strlen(str + 1));
+    }
+
+
+The suggested fix is to add value to the return value of ``strlen()`` and not
+to its argument. In the example above the fix would be
+
+.. code-block:: c
+
+      char *c = (char*) malloc(strlen(str) + 1);
+
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,6 +57,14 @@
 Improvements to clang-tidy
 --------------------------
 
+- New `bugprone-misplaced-operator-in-strlen-in-alloc
+  <http://clang.llvm.org/extra/clang-tidy/checks/bugprone-misplaced-operator-in-strlen-in-alloc.html>`_ check
+
+  Finds cases a value is added to or subtracted from the string in the parameter
+  of ``strlen()`` method instead of to the result and use its return value as an
+  argument of a memory allocation function (``malloc()``, ``calloc()``,
+  ``realloc()``). The check also suggests the appropriate fix.
+
 - Renamed checks to use correct term "implicit conversion" instead of "implicit
   cast" and modified messages and option names accordingly:
 
Index: clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.h
@@ -0,0 +1,37 @@
+//===--- MisplacedOperatorInStrlenInAllocCheck.h - clang-tidy----*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MISPLACED_OPERATOR_IN_STRLEN_IN_ALLOC_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MISPLACED_OPERATOR_IN_STRLEN_IN_ALLOC_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Finds cases a value is added to or subtracted from the string in the
+/// parameter of ``strlen()`` method instead of to the result and use its return
+/// value as an argument of a memory allocation function.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-misplaced-operator-in-strlen-in-alloc.html
+class MisplacedOperatorInStrlenInAllocCheck : public ClangTidyCheck {
+public:
+  MisplacedOperatorInStrlenInAllocCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MISPLACED_OPERATOR_IN_STRLEN_IN_ALLOC_H
Index: clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp
@@ -0,0 +1,73 @@
+//===--- MisplacedOperatorInStrlenInAllocCheck.cpp - clang-tidy------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "MisplacedOperatorInStrlenInAllocCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+void MisplacedOperatorInStrlenInAllocCheck::registerMatchers(
+    MatchFinder *Finder) {
+  const auto BadUse =
+      callExpr(callee(functionDecl(hasName("strlen"))),
+               hasAnyArgument(ignoringParenImpCasts(
+                   binaryOperator(allOf(hasOperatorName("+"),
+                                        hasRHS(ignoringParenImpCasts(
+                                            integerLiteral(equals(1))))))
+                       .bind("BinOp"))))
+          .bind("StrLen");
+  const auto BadArg = anyOf(
+      allOf(hasDescendant(BadUse),
+            unless(binaryOperator(allOf(
+                hasOperatorName("+"), hasLHS(BadUse),
+                hasRHS(ignoringParenImpCasts(integerLiteral(equals(1)))))))),
+      BadUse);
+  Finder->addMatcher(callExpr(callee(functionDecl(
+                                  anyOf(hasName("malloc"), hasName("alloca")))),
+                              hasArgument(0, BadArg))
+                         .bind("Alloc"),
+                     this);
+  Finder->addMatcher(callExpr(callee(functionDecl(anyOf(hasName("calloc"),
+                                                        hasName("realloc")))),
+                              hasArgument(1, BadArg))
+                         .bind("Alloc"),
+                     this);
+}
+
+void MisplacedOperatorInStrlenInAllocCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *Alloc = Result.Nodes.getNodeAs<CallExpr>("Alloc");
+  const auto *StrLen = Result.Nodes.getNodeAs<CallExpr>("StrLen");
+  const auto *BinOp = Result.Nodes.getNodeAs<BinaryOperator>("BinOp");
+
+  const std::string LHSText = Lexer::getSourceText(
+      CharSourceRange::getTokenRange(BinOp->getLHS()->getSourceRange()),
+      *Result.SourceManager, getLangOpts());
+  const std::string RHSText = Lexer::getSourceText(
+      CharSourceRange::getTokenRange(BinOp->getRHS()->getSourceRange()),
+      *Result.SourceManager, getLangOpts());
+
+  auto Hint = FixItHint::CreateReplacement(
+      StrLen->getSourceRange(),
+      "strlen(" + LHSText + ")" +
+          ((BinOp->getOpcode() == BO_Add) ? " + " : " - ") + RHSText);
+
+  diag(Alloc->getLocStart(), "Binary operator %0 %1 is inside strlen")
+      << ((BinOp->getOpcode() == BO_Add) ? "+" : "-") << BinOp->getRHS()
+      << Hint;
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/bugprone/CMakeLists.txt
===================================================================
--- clang-tidy/bugprone/CMakeLists.txt
+++ clang-tidy/bugprone/CMakeLists.txt
@@ -3,6 +3,7 @@
 add_clang_library(clangTidyBugproneModule
   BugproneTidyModule.cpp
   IntegerDivisionCheck.cpp
+  MisplacedOperatorInStrlenInAllocCheck.cpp
   SuspiciousMemsetUsageCheck.cpp
   UndefinedMemoryManipulationCheck.cpp
 
Index: clang-tidy/bugprone/BugproneTidyModule.cpp
===================================================================
--- clang-tidy/bugprone/BugproneTidyModule.cpp
+++ clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -11,6 +11,7 @@
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "IntegerDivisionCheck.h"
+#include "MisplacedOperatorInStrlenInAllocCheck.h"
 #include "SuspiciousMemsetUsageCheck.h"
 #include "UndefinedMemoryManipulationCheck.h"
 
@@ -23,6 +24,8 @@
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<IntegerDivisionCheck>(
         "bugprone-integer-division");
+    CheckFactories.registerCheck<MisplacedOperatorInStrlenInAllocCheck>(
+        "bugprone-misplaced-operator-in-strlen-in-alloc");
     CheckFactories.registerCheck<SuspiciousMemsetUsageCheck>(
         "bugprone-suspicious-memset-usage");
     CheckFactories.registerCheck<UndefinedMemoryManipulationCheck>(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to