[PATCH] D39367: [clang-tidy] Add support for operator new[] in check bugprone-misplaced-operator-in-strlen-in-alloc

[Balogh , Ádám via Phabricator via cfe-commits]

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

Reformatted according to the comments.


https://reviews.llvm.org/D39367

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


Index: test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.cpp
===================================================================
--- test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.cpp
+++ test/clang-tidy/bugprone-misplaced-operator-in-strlen-in-alloc.cpp
@@ -29,3 +29,9 @@
   char *new_name = (char*) std::malloc(non_std::strlen(name + 1));
   // Ignore functions of the strlen family in custom namespaces
 }
+
+void bad_new_strlen(char *name) {
+  char *new_name = new char[std::strlen(name + 1)];
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: Addition operator is applied to the argument of strlen
+  // CHECK-FIXES: {{^  char \*new_name = new char\[}}std::strlen(name) + 1{{\];$}}
+}
Index: docs/clang-tidy/checks/bugprone-misplaced-operator-in-strlen-in-alloc.rst
===================================================================
--- docs/clang-tidy/checks/bugprone-misplaced-operator-in-strlen-in-alloc.rst
+++ docs/clang-tidy/checks/bugprone-misplaced-operator-in-strlen-in-alloc.rst
@@ -7,11 +7,12 @@
 ``strnlen()``, ``strnlen_s()``, ``wcslen()``, ``wcsnlen()`` and ``wcsnlen_s()``
 functions instead of to the result and use its return value as an argument of a
 memory allocation function (``malloc()``, ``calloc()``, ``realloc()``,
-``alloca()``). Cases where ``1`` is added both to the parameter and the result
-of the ``strlen()``-like function are ignored, as are cases where the whole
-addition is surrounded by extra parentheses.
+``alloca()``) or the ``new[]`` operator in `C++`. Cases where ``1`` is added
+both to the parameter and the result of the ``strlen()``-like function are
+ignored, as are cases where the whole addition is surrounded by extra
+parentheses.
 
-Example code:
+`C` example code:
 
 .. code-block:: c
 
@@ -28,6 +29,24 @@
       char *c = (char*) malloc(strlen(str) + 1);
 
 
+`C++` example code:
+
+.. code-block:: c++
+
+    void bad_new(char *str) {
+      char *c = new char[strlen(str + 1)];
+    }
+
+
+As in the `C` code with the ``malloc()`` function, the suggested fix is to
+add ``1`` to the return value of ``strlen()`` and not to its argument. In the
+example above the fix would be
+
+.. code-block:: c++
+
+      char *c = new char[strlen(str) + 1];
+
+
 Example for silencing the bug report:
 
 .. code-block:: c
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -64,7 +64,7 @@
   ``strlen()``, ``strnlen()``, ``strnlen_s()``, ``wcslen()``, ``wcsnlen()`` and
   ``wcsnlen_s()`` functions instead of to the result and use its return value as
   an argument of a memory allocation function (``malloc()``, ``calloc()``,
-  ``realloc()``, ``alloca()``).
+  ``realloc()``, ``alloca()``) or the ``new[]`` operator in `C++`.
 
 - Renamed checks to use correct term "implicit conversion" instead of "implicit
   cast" and modified messages and option names accordingly:
Index: clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp
===================================================================
--- clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp
+++ clang-tidy/bugprone/MisplacedOperatorInStrlenInAllocCheck.cpp
@@ -48,26 +48,32 @@
   const auto Alloc1Func =
       functionDecl(anyOf(hasName("::calloc"), hasName("std::calloc"),
                          hasName("::realloc"), hasName("std::realloc")));
-
   Finder->addMatcher(
       callExpr(callee(Alloc0Func), hasArgument(0, BadArg)).bind("Alloc"), this);
   Finder->addMatcher(
       callExpr(callee(Alloc1Func), hasArgument(1, BadArg)).bind("Alloc"), this);
+  Finder->addMatcher(
+      cxxNewExpr(isArrayForm(), hasArraySize(BadArg)).bind("Alloc"), this);
 }
 
 void MisplacedOperatorInStrlenInAllocCheck::check(
     const MatchFinder::MatchResult &Result) {
-  const auto *Alloc = Result.Nodes.getNodeAs<CallExpr>("Alloc");
+  const Expr *Alloc = Result.Nodes.getNodeAs<CallExpr>("Alloc");
+  if (!Alloc)
+    Alloc = Result.Nodes.getNodeAs<CXXNewExpr>("Alloc");
+  assert(Alloc && "Matched node bound by `Alloc` shoud be either `CallExpr`"
+         " or `CXXNewExpr`");
+
   const auto *StrLen = Result.Nodes.getNodeAs<CallExpr>("StrLen");
   const auto *BinOp = Result.Nodes.getNodeAs<BinaryOperator>("BinOp");
 
   const StringRef StrLenText = Lexer::getSourceText(
       CharSourceRange::getTokenRange(StrLen->getSourceRange()),
       *Result.SourceManager, getLangOpts());
-  const StringRef StrLenBegin = StrLenText.substr(0, StrLenText.find('(') + 1);
   const StringRef Arg0Text = Lexer::getSourceText(
       CharSourceRange::getTokenRange(StrLen->getArg(0)->getSourceRange()),
       *Result.SourceManager, getLangOpts());
+  const StringRef StrLenBegin = StrLenText.substr(0, StrLenText.find(Arg0Text));
   const StringRef StrLenEnd = StrLenText.substr(
       StrLenText.find(Arg0Text) + Arg0Text.size(), StrLenText.size());
 

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits