[clang-tidy] Add checker for operations between integrals and pointers

Summary:
This check is finding suspicious operations involving pointers and integral types; which are most likely bugs.

Examples:
subversion/v1_6/subversion/libsvn_subr/utf.c
```
static const char *
fuzzy_escape(const char *src, apr_size_t len, apr_pool_t *pool)
{
  [...]
   while (src_orig < src_end)
    {
      if (! svn_ctype_isascii(*src_orig) || src_orig == '\0')   // Should be *src_orig
        {
```

apache2/v2_2_23/modules/metadata/mod_headers.c
```
static char *parse_format_tag(apr_pool_t *p, format_tag *tag, const char **sa)
{
  [...]
    tag->arg = '\0';   // ERROR: tag->arg has type char*

    /* grab the argument if there is one */
    if (*s == '{') {
        ++s;
        tag->arg = ap_getword(p,&s,'}');
    }
```

Reviewers: alexfh

Subscribers: Eugene.Zelenko, cfe-commits

Differential Revision: http://reviews.llvm.org/D19118

llvm-svn: 266450

9 files changed, 248 insertions, 0 deletions

diff --git a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
index c3ee5c3a6b2..ae0b0dfdb78 100644
--- a/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -21,6 +21,7 @@ add_clang_library(clangTidyMiscModule
   NewDeleteOverloadsCheck.cpp
   NoexceptMoveConstructorCheck.cpp
   NonCopyableObjects.cpp
+  PointerAndIntegralOperationCheck.cpp
   SizeofContainerCheck.cpp
   StaticAssertCheck.cpp
   StringIntegerAssignmentCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
index df335fa6e51..bbc0eb714e8 100644
--- a/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
@@ -29,6 +29,7 @@
 #include "NewDeleteOverloadsCheck.h"
 #include "NoexceptMoveConstructorCheck.h"
 #include "NonCopyableObjects.h"
+#include "PointerAndIntegralOperationCheck.h"
 #include "SizeofContainerCheck.h"
 #include "StaticAssertCheck.h"
 #include "StringIntegerAssignmentCheck.h"
@@ -88,6 +89,8 @@ public:
         "misc-noexcept-move-constructor");
     CheckFactories.registerCheck<NonCopyableObjectsCheck>(
         "misc-non-copyable-objects");
+    CheckFactories.registerCheck<PointerAndIntegralOperationCheck>(
+        "misc-pointer-and-integral-operation");
     CheckFactories.registerCheck<SizeofContainerCheck>("misc-sizeof-container");
     CheckFactories.registerCheck<StaticAssertCheck>(
         "misc-static-assert");
diff --git a/clang-tools-extra/clang-tidy/misc/PointerAndIntegralOperationCheck.cpp b/clang-tools-extra/clang-tidy/misc/PointerAndIntegralOperationCheck.cpp
new file mode 100644
index 00000000000..e154e8dac81
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/misc/PointerAndIntegralOperationCheck.cpp
@@ -0,0 +1,104 @@
+//===--- PointerAndIntegralOperationCheck.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 "PointerAndIntegralOperationCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+void PointerAndIntegralOperationCheck::registerMatchers(MatchFinder *Finder) {
+  const auto PointerExpr = expr(hasType(pointerType()));
+  const auto BoolExpr = ignoringParenImpCasts(hasType(booleanType()));
+  const auto CharExpr = ignoringParenImpCasts(hasType(isAnyCharacter()));
+
+  const auto BinOpWithPointerExpr =
+      binaryOperator(unless(anyOf(hasOperatorName(","), hasOperatorName("="))),
+                     hasEitherOperand(PointerExpr));
+
+  const auto AssignToPointerExpr =
+      binaryOperator(hasOperatorName("="), hasLHS(PointerExpr));
+
+  const auto CompareToPointerExpr =
+      binaryOperator(anyOf(hasOperatorName("<"), hasOperatorName("<="),
+                           hasOperatorName(">"), hasOperatorName(">=")),
+                     hasEitherOperand(PointerExpr));
+
+  // Detect expression like: ptr = (x != y);
+  Finder->addMatcher(binaryOperator(AssignToPointerExpr, hasRHS(BoolExpr))
+                         .bind("assign-bool-to-pointer"),
+                     this);
+
+  // Detect expression like: ptr = A[i]; where A is char*.
+  Finder->addMatcher(binaryOperator(AssignToPointerExpr, hasRHS(CharExpr))
+                         .bind("assign-char-to-pointer"),
+                     this);
+
+  // Detect expression like: ptr < false;
+  Finder->addMatcher(
+      binaryOperator(BinOpWithPointerExpr,
+                     hasEitherOperand(ignoringParenImpCasts(cxxBoolLiteral())))
+          .bind("pointer-and-bool-literal"),
+      this);
+
+  // Detect expression like: ptr < 'a';
+  Finder->addMatcher(binaryOperator(BinOpWithPointerExpr,
+                                    hasEitherOperand(ignoringParenImpCasts(
+                                        characterLiteral())))
+                         .bind("pointer-and-char-literal"),
+                     this);
+
+  // Detect expression like: ptr < 0;
+  Finder->addMatcher(binaryOperator(CompareToPointerExpr,
+                                    hasEitherOperand(ignoringParenImpCasts(
+                                        integerLiteral(equals(0)))))
+                         .bind("compare-pointer-to-zero"),
+                     this);
+
+  // Detect expression like: ptr < nullptr;
+  Finder->addMatcher(binaryOperator(CompareToPointerExpr,
+                                    hasEitherOperand(ignoringParenImpCasts(
+                                        cxxNullPtrLiteralExpr())))
+                         .bind("compare-pointer-to-null"),
+                     this);
+}
+
+void PointerAndIntegralOperationCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  if (const auto *E =
+          Result.Nodes.getNodeAs<BinaryOperator>("assign-bool-to-pointer")) {
+    diag(E->getOperatorLoc(), "suspicious assignment from bool to pointer");
+  } else if (const auto *E = Result.Nodes.getNodeAs<BinaryOperator>(
+                 "assign-char-to-pointer")) {
+    diag(E->getOperatorLoc(), "suspicious assignment from char to pointer");
+  } else if (const auto *E = Result.Nodes.getNodeAs<BinaryOperator>(
+                 "pointer-and-bool-literal")) {
+    diag(E->getOperatorLoc(),
+         "suspicious operation between pointer and bool literal");
+  } else if (const auto *E = Result.Nodes.getNodeAs<BinaryOperator>(
+                 "pointer-and-char-literal")) {
+    diag(E->getOperatorLoc(),
+         "suspicious operation between pointer and character literal");
+  } else if (const auto *E = Result.Nodes.getNodeAs<BinaryOperator>(
+                 "compare-pointer-to-zero")) {
+    diag(E->getOperatorLoc(), "suspicious comparison of pointer with zero");
+  } else if (const auto *E = Result.Nodes.getNodeAs<BinaryOperator>(
+                 "compare-pointer-to-null")) {
+    diag(E->getOperatorLoc(),
+         "suspicious comparison of pointer with null expression");
+  }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
diff --git a/clang-tools-extra/clang-tidy/misc/PointerAndIntegralOperationCheck.h b/clang-tools-extra/clang-tidy/misc/PointerAndIntegralOperationCheck.h
new file mode 100644
index 00000000000..996253e8e69
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/misc/PointerAndIntegralOperationCheck.h
@@ -0,0 +1,35 @@
+//===--- PointerAndIntegralOperationCheck.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_MISC_POINTER_AND_INTEGRAL_OPERATION_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_POINTER_AND_INTEGRAL_OPERATION_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Find suspicious expressions involving pointer and integral types.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-pointer-and-integral-operation.html
+class PointerAndIntegralOperationCheck : public ClangTidyCheck {
+public:
+  PointerAndIntegralOperationCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_POINTER_AND_INTEGRAL_OPERATION_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index c61121988cb..578551e4da4 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -103,6 +103,11 @@ identified.  The improvements since the 3.8 release include:
   Warns when there is a explicit redundant cast of a calculation result to a
   bigger type.
 
+- New `misc-pointer-and-integral-operation
+  <http://clang.llvm.org/extra/clang-tidy/checks/misc-misc-pointer-and-integral-operation.html>`_ check
+
+  Warns about suspicious operations involving pointers and integral types.
+
 - New `misc-string-literal-with-embedded-nul
   <http://clang.llvm.org/extra/clang-tidy/checks/misc-string-literal-with-embedded-nul.html>`_ check
 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 5fab245a14c..8af60124e02 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -65,6 +65,7 @@ Clang-Tidy Checks
    misc-new-delete-overloads
    misc-noexcept-move-constructor
    misc-non-copyable-objects
+   misc-pointer-and-integral-operation
    misc-sizeof-container
    misc-static-assert
    misc-string-integer-assignment
diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc-pointer-and-integral-operation.rst b/clang-tools-extra/docs/clang-tidy/checks/misc-pointer-and-integral-operation.rst
new file mode 100644
index 00000000000..d32d8245da6
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/misc-pointer-and-integral-operation.rst
@@ -0,0 +1,24 @@
+.. title:: clang-tidy - misc-pointer-and-integral-operation
+
+misc-pointer-and-integral-operation
+===================================
+
+Looks for operation involving pointers and integral types. A common mistake is
+to forget to dereference a pointer. These errors may be detected when a pointer
+object is compare to an object with integral type.
+
+Examples:
+
+.. code:: c++
+
+  char* ptr;
+  if ((ptr = malloc(...)) < nullptr)   // Pointer comparison with operator '<'
+    ...                                // Should probably be '!='
+
+  if (ptr == '\0')   // Should probably be *ptr
+    ... 
+
+  void Process(std::string path, bool* error) {
+    [...]
+    if (error != false)  // Should probably be *error
+      ...
diff --git a/clang-tools-extra/test/clang-tidy/misc-pointer-and-integral-operation-cxx98.cpp b/clang-tools-extra/test/clang-tidy/misc-pointer-and-integral-operation-cxx98.cpp
new file mode 100644
index 00000000000..4834a469e76
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/misc-pointer-and-integral-operation-cxx98.cpp
@@ -0,0 +1,45 @@
+// RUN: %check_clang_tidy %s misc-pointer-and-integral-operation %t -- -- -std=c++98
+
+bool* pb;
+char* pc;
+int* pi;
+
+int Test() {
+  pb = false;
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: suspicious assignment from bool to pointer [misc-pointer-and-integral-operation]
+  pc = '\0';
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: suspicious assignment from char to pointer
+
+  pb = (false?false:false);
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: suspicious assignment from bool to pointer
+  pb = (4 != 5?false:false);
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: suspicious assignment from bool to pointer
+
+  if (pb < false) return 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious operation between pointer and bool literal
+  if (pb != false) return 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious operation between pointer and bool literal
+  if (pc < '\0') return 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious operation between pointer and character literal
+  if (pc != '\0') return 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious operation between pointer and character literal
+  if (pi < '\0') return 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious operation between pointer and character literal
+  if (pi != '\0') return 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious operation between pointer and character literal
+
+  return 1;
+}
+
+int Valid() {
+  *pb = false;
+  *pc = '\0';
+
+  pb += 0;
+  pc += 0;
+  pi += 0;
+
+  pb += (pb != 0);
+  pc += (pc != 0);
+  pi += (pi != 0);
+}
diff --git a/clang-tools-extra/test/clang-tidy/misc-pointer-and-integral-operation.cpp b/clang-tools-extra/test/clang-tidy/misc-pointer-and-integral-operation.cpp
new file mode 100644
index 00000000000..6591f0af360
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/misc-pointer-and-integral-operation.cpp
@@ -0,0 +1,30 @@
+// RUN: %check_clang_tidy %s misc-pointer-and-integral-operation %t
+
+bool* pb;
+char* pc;
+int* pi;
+
+int Test() {
+  if (pi < 0) return 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious comparison of pointer with zero [misc-pointer-and-integral-operation]
+  if (pi <= 0) return 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious comparison of pointer with zero
+
+  if (nullptr <= pb) return 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: suspicious comparison of pointer with null
+  if (pc < nullptr) return 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious comparison of pointer with null
+  if (pi > nullptr) return 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious comparison of pointer with null
+
+  return 1;
+}
+
+int Valid() {
+  *pb = false;
+  *pc = '\0';
+
+  pi += (pi != nullptr);
+  pi -= (pi == nullptr);
+  pc += (pb != nullptr);
+}