baloghadamsoftware updated this revision to Diff 238451.
baloghadamsoftware marked an inline comment as done.
baloghadamsoftware added a comment.

Rebased.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71001/new/

https://reviews.llvm.org/D71001

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  
clang-tools-extra/clang-tidy/bugprone/MisplacedPointerArithmeticInAllocCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/MisplacedPointerArithmeticInAllocCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-misplaced-pointer-arithmetic-in-alloc.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-misplaced-pointer-arithmetic-in-alloc.c
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-misplaced-pointer-arithmetic-in-alloc.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-misplaced-pointer-arithmetic-in-alloc.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-misplaced-pointer-arithmetic-in-alloc.cpp
@@ -0,0 +1,53 @@
+// RUN: %check_clang_tidy %s bugprone-misplaced-pointer-arithmetic-in-alloc %t
+
+class C {
+  int num;
+public:
+  explicit C(int n) : num(n) {}
+};
+
+void bad_new(int n, int m) {
+  C *p = new C(n) + 10;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: arithmetic operation is applied to the result of operator new() instead of its size-like argument
+  // CHECK-FIXES: {{^  C \*p = new C\(n}} + 10{{\);$}}
+
+  p = new C(n) - 10;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: arithmetic operation is applied to the result of operator new() instead of its size-like argument
+  // CHECK-FIXES: {{^  p = new C\(n}} - 10{{\);$}}
+
+  p = new C(n) + m;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: arithmetic operation is applied to the result of operator new() instead of its size-like argument
+  // CHECK-FIXES: {{^  p = new C\(n}} + m{{\);$}}
+
+  p = new C(n) - (m + 10);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: arithmetic operation is applied to the result of operator new() instead of its size-like argument
+  // CHECK-FIXES: {{^  p = new C\(n}} - (m + 10){{\);$}}
+
+  p = new C(n) - m + 10;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: arithmetic operation is applied to the result of operator new() instead of its size-like argument
+  // CHECK-FIXES: {{^  p = new C\(n}} - m{{\) \+ 10;$}}
+  // FIXME: Should be {{^  p = new C\(n}} - m + 10{{\);$}}
+}
+
+void bad_new_array(int n, int m) {
+  char *p = new char[n] + 10;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: arithmetic operation is applied to the result of operator new[]() instead of its size-like argument
+  // CHECK-FIXES: {{^  char \*p = new char\[n}} + 10{{\];$}}
+
+  p = new char[n] - 10;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: arithmetic operation is applied to the result of operator new[]() instead of its size-like argument
+  // CHECK-FIXES: {{^  p = new char\[n}} - 10{{\];$}}
+
+  p = new char[n] + m;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: arithmetic operation is applied to the result of operator new[]() instead of its size-like argument
+  // CHECK-FIXES: {{^  p = new char\[n}} + m{{\];$}}
+
+  p = new char[n] - (m + 10);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: arithmetic operation is applied to the result of operator new[]() instead of its size-like argument
+  // CHECK-FIXES: {{^  p = new char\[n}} - (m + 10){{\];$}}
+
+  p = new char[n] - m + 10;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: arithmetic operation is applied to the result of operator new[]() instead of its size-like argument
+  // CHECK-FIXES: {{^  p = new char\[n}} - m{{\] \+ 10;$}}
+  // FIXME: should be {{^  p = new char\[n}} - m + 10{{\];$}}
+}
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-misplaced-pointer-arithmetic-in-alloc.c
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-misplaced-pointer-arithmetic-in-alloc.c
@@ -0,0 +1,56 @@
+// RUN: %check_clang_tidy %s bugprone-misplaced-pointer-arithmetic-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);
+
+void bad_malloc(int n) {
+  char *p = (char *)malloc(n) + 10;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: arithmetic operation is applied to the result of malloc() instead of its size-like argument
+  // CHECK-FIXES: {{^  char \*p = \(char \*\)malloc\(n}} + 10{{\);$}}
+
+  p = (char *)malloc(n) - 10;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: arithmetic operation is applied to the result of malloc() instead of its size-like argument
+  // CHECK-FIXES: {{^  p = \(char \*\)malloc\(n}} - 10{{\);$}}
+
+  p = (char *)malloc(n) + n;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: arithmetic operation is applied to the result of malloc() instead of its size-like argument
+  // CHECK-FIXES: {{^  p = \(char \*\)malloc\(n}} + n{{\);$}}
+
+  p = (char *)malloc(n) - (n + 10);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: arithmetic operation is applied to the result of malloc() instead of its size-like argument
+  // CHECK-FIXES: {{^  p = \(char \*\)malloc\(n}} - (n + 10){{\);$}}
+
+  p = (char *)malloc(n) - n + 10;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: arithmetic operation is applied to the result of malloc() instead of its size-like argument
+  // CHECK-FIXES: {{^  p = \(char \*\)malloc\(n}} - n{{\) \+ 10;$}}
+  // FIXME: should be {{^  p = \(char \*\)malloc\(n}} - n + 10{{\);$}}
+}
+
+void bad_alloca(int n) {
+  char *p = (char *)alloca(n) + 10;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: arithmetic operation is applied to the result of alloca() instead of its size-like argument
+  // CHECK-FIXES: {{^  char \*p = \(char \*\)alloca\(n}} + 10{{\);$}}
+}
+
+void bad_realloc(char *s, int n) {
+  char *p = (char *)realloc(s, n) + 10;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: arithmetic operation is applied to the result of realloc() instead of its size-like argument
+  // CHECK-FIXES: {{^  char \*p = \(char \*\)realloc\(s, n}} + 10{{\);$}}
+}
+
+void bad_calloc(int n, int m) {
+  char *p = (char *)calloc(m, n) + 10;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: arithmetic operation is applied to the result of calloc() instead of its size-like argument
+  // CHECK-FIXES: {{^  char \*p = \(char \*\)calloc\(m, n}} + 10{{\);$}}
+}
+
+void (*(*const alloc_ptr)(size_t)) = malloc;
+
+void bad_indirect_alloc(int n) {
+  char *p = (char *)alloc_ptr(n) + 10;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: arithmetic operation is applied to the result of alloc_ptr() instead of its size-like argument
+  // CHECK-FIXES: {{^  char \*p = \(char \*\)alloc_ptr\(n}} + 10{{\);$}}
+}
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
@@ -65,6 +65,7 @@
    `bugprone-macro-parentheses <bugprone-macro-parentheses.html>`_, "Yes"
    `bugprone-macro-repeated-side-effects <bugprone-macro-repeated-side-effects.html>`_,
    `bugprone-misplaced-operator-in-strlen-in-alloc <bugprone-misplaced-operator-in-strlen-in-alloc.html>`_, "Yes"
+   `bugprone-misplaced-pointer-arithmetic-in-alloc <bugprone-misplaced-pointer-arithmetic-in-alloc.html>`_, "Yes"
    `bugprone-misplaced-widening-cast <bugprone-misplaced-widening-cast.html>`_,
    `bugprone-move-forwarding-reference <bugprone-move-forwarding-reference.html>`_, "Yes"
    `bugprone-multiple-statement-macro <bugprone-multiple-statement-macro.html>`_,
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone-misplaced-pointer-arithmetic-in-alloc.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone-misplaced-pointer-arithmetic-in-alloc.rst
@@ -0,0 +1,25 @@
+.. title:: clang-tidy - bugprone-misplaced-pointer-arithmetic-in-alloc
+
+bugprone-misplaced-pointer-artithmetic-in-alloc
+===============================================
+
+Finds cases where an integer expression is added to or subtracted from the
+result of a memory allocation function (``malloc()``, ``calloc()``,
+``realloc()``, ``alloca()``) instead of its argument. The check detects error
+cases even if one of these functions is called by a constant function pointer.
+
+Example code:
+
+.. code-block:: c
+
+    void bad_malloc(int n) {
+      char *p = (char*) malloc(n) + 10;
+    }
+
+
+The suggested fix is to add the integer expression to the argument of
+``malloc`` and not to its result. In the example above the fix would be
+
+.. code-block:: c
+
+      char *p = (char*) malloc(n + 10);
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -70,6 +70,12 @@
 New checks
 ^^^^^^^^^^
 
+- New :doc:`bugprone-misplaced-pointer-arithmetic-in-alloc
+  <clang-tidy/checks/bugprone-misplaced-pointer-arithmetic-in-alloc>` check.
+
+  Finds cases where an integer expression is added to or subtracted from the
+  result of a memory allocation function (``malloc()``, ``calloc()``,
+  ``realloc()``, ``alloca()``) instead of its argument.
 
 New aliases
 ^^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/bugprone/MisplacedPointerArithmeticInAllocCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/MisplacedPointerArithmeticInAllocCheck.h
@@ -0,0 +1,36 @@
+//===--- MisplacedPointerArithmeticInAllocCheck.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_MISPLACED_OPERATOR_IN_ALLOC_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_MISPLACED_OPERATOR_IN_ALLOC_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Finds cases where an integer is added to or subracted from the result of a
+/// memory allocation function instead of its argument.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-misplaced-operator-in-alloc.html
+class MisplacedPointerArithmeticInAllocCheck : public ClangTidyCheck {
+public:
+  MisplacedPointerArithmeticInAllocCheck(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_ALLOC_H
Index: clang-tools-extra/clang-tidy/bugprone/MisplacedPointerArithmeticInAllocCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/MisplacedPointerArithmeticInAllocCheck.cpp
@@ -0,0 +1,132 @@
+//===--- MisplacedPointerArithmeticInAllocCheck.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 "MisplacedPointerArithmeticInAllocCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+static FixItHint composeHint(const BinaryOperator *Outer, const Expr *Inner,
+                             char ParenChar, const SourceManager &SM,
+                             const LangOptions &LO) {
+  const StringRef LeftText = Lexer::getSourceText(
+      CharSourceRange::getTokenRange(Outer->getLHS()->getSourceRange()),
+      SM, LO);
+  const StringRef RightText = Lexer::getSourceText(
+      CharSourceRange::getTokenRange(Outer->getRHS()->getSourceRange()),
+      SM, LO);
+  const StringRef InnerText = Lexer::getSourceText(
+      CharSourceRange::getTokenRange(Inner->getSourceRange()), SM, LO);
+  const StringRef AheadOfClosingParen =
+    InnerText.substr(0, InnerText.rfind(ParenChar));
+
+  StringRef LeftAhead, LeftBehind;
+  std::tie(LeftAhead, LeftBehind) = LeftText.split(InnerText);
+
+  return FixItHint::CreateReplacement(
+      Outer->getSourceRange(),
+      (LeftAhead + AheadOfClosingParen +
+       ((Outer->getOpcode() == BO_Add) ? " + " : " - ") + RightText +
+       std::string(1, ParenChar) + LeftBehind)
+      .str());
+}
+
+void MisplacedPointerArithmeticInAllocCheck::registerMatchers(
+    MatchFinder *Finder) {
+  const auto AllocFunc = functionDecl(
+      anyOf(hasName("::malloc"), hasName("std::malloc"), hasName("::alloca"),
+            hasName("::calloc"), hasName("std::calloc"), hasName("::realloc"),
+            hasName("std::realloc")));
+
+  const auto AllocFuncPtr =
+      varDecl(hasType(isConstQualified()),
+              hasInitializer(ignoringParenImpCasts(
+                  declRefExpr(hasDeclaration(AllocFunc)))));
+
+  const auto AdditiveOperator =
+      binaryOperator(anyOf(hasOperatorName("+"), hasOperatorName("-")));
+
+  const auto IntExpr = expr(hasType(isInteger()));
+
+  const auto AllocCall =
+      callExpr(callee(decl(anyOf(AllocFunc, AllocFuncPtr)))).bind("AllocCall");
+
+  Finder->addMatcher(
+      binaryOperator(
+          AdditiveOperator,
+          hasLHS(anyOf(AllocCall, castExpr(hasSourceExpression(AllocCall)))),
+          hasRHS(IntExpr))
+          .bind("PtrArith"),
+      this);
+
+  const auto CXXConstructorWithSingleIntArg =
+    cxxConstructExpr(argumentCountIs(1), hasArgument(0, IntExpr));
+
+  const auto New =
+    cxxNewExpr(unless(isArray()),
+               has(CXXConstructorWithSingleIntArg)).bind("New");
+
+  Finder->addMatcher(
+      binaryOperator(
+          AdditiveOperator,
+          hasLHS(anyOf(New, castExpr(New))),
+          hasRHS(IntExpr))
+          .bind("PtrArith"),
+      this);
+
+  const auto ArrayNew = cxxNewExpr(isArray()).bind("ArrayNew");
+
+  Finder->addMatcher(
+      binaryOperator(
+          AdditiveOperator,
+          hasLHS(anyOf(ArrayNew, castExpr(ArrayNew))),
+          hasRHS(IntExpr))
+          .bind("PtrArith"),
+      this);
+}
+
+void MisplacedPointerArithmeticInAllocCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *PtrArith = Result.Nodes.getNodeAs<BinaryOperator>("PtrArith");
+  std::string CallName;
+
+  FixItHint Hint;
+  if (const auto *AllocCall =
+      Result.Nodes.getNodeAs<CallExpr>("AllocCall")) {
+    Hint = composeHint(PtrArith, AllocCall, ')', *Result.SourceManager,
+                       getLangOpts());
+    const NamedDecl *Func = AllocCall->getDirectCallee();
+    if (!Func) {
+      Func = cast<NamedDecl>(AllocCall->getCalleeDecl());
+    }
+    CallName = Func->getName().str();
+  } else if (const auto *New = Result.Nodes.getNodeAs<CXXNewExpr>("New")) {
+    Hint = composeHint(PtrArith, New, ')', *Result.SourceManager,
+                       getLangOpts());
+    CallName = "operator new";
+  } else {
+    const auto *ArrayNew = Result.Nodes.getNodeAs<CXXNewExpr>("ArrayNew");
+    Hint = composeHint(PtrArith, ArrayNew, ']', *Result.SourceManager,
+                       getLangOpts());
+    CallName = "operator new[]";
+  }
+
+  diag(PtrArith->getBeginLoc(),
+       "arithmetic operation is applied to the result of %0() instead of its "
+       "size-like argument")
+      << CallName << Hint;
+}
+
+} // 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
@@ -22,6 +22,7 @@
   MacroParenthesesCheck.cpp
   MacroRepeatedSideEffectsCheck.cpp
   MisplacedOperatorInStrlenInAllocCheck.cpp
+  MisplacedPointerArithmeticInAllocCheck.cpp
   MisplacedWideningCastCheck.cpp
   MoveForwardingReferenceCheck.cpp
   MultipleStatementMacroCheck.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
@@ -30,6 +30,7 @@
 #include "MacroParenthesesCheck.h"
 #include "MacroRepeatedSideEffectsCheck.h"
 #include "MisplacedOperatorInStrlenInAllocCheck.h"
+#include "MisplacedPointerArithmeticInAllocCheck.h"
 #include "MisplacedWideningCastCheck.h"
 #include "MoveForwardingReferenceCheck.h"
 #include "MultipleStatementMacroCheck.h"
@@ -106,6 +107,8 @@
         "bugprone-macro-repeated-side-effects");
     CheckFactories.registerCheck<MisplacedOperatorInStrlenInAllocCheck>(
         "bugprone-misplaced-operator-in-strlen-in-alloc");
+    CheckFactories.registerCheck<MisplacedPointerArithmeticInAllocCheck>(
+        "bugprone-misplaced-pointer-arithmetic-in-alloc");
     CheckFactories.registerCheck<MisplacedWideningCastCheck>(
         "bugprone-misplaced-widening-cast");
     CheckFactories.registerCheck<MoveForwardingReferenceCheck>(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to