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

[etienneb]

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,'}');
    }

[etienneb]

nits

[alexfh]

LG. Nice check!

test/clang-tidy/misc-pointer-and-integral-operation-cxx98.cpp

10

Specify the whole message including the check name once.

[Eugene.Zelenko]

docs/clang-tidy/checks/misc-pointer-and-integral-operation.rst

10

Looks like excessive line.

23

Process?

[etienneb]

comments.

docs/clang-tidy/checks/misc-pointer-and-integral-operation.rst

23

tpo. oups.. typo :)

clang-tidy/misc/CMakeLists.txt

add_clang_library(clangTidyMiscModule
   MiscTidyModule.cpp
   MisplacedWideningCastCheck.cpp
   MoveConstantArgumentCheck.cpp
   MoveConstructorInitCheck.cpp
   MultipleStatementMacroCheck.cpp
   NewDeleteOverloadsCheck.cpp
   NoexceptMoveConstructorCheck.cpp
   NonCopyableObjects.cpp
+  PointerAndIntegralOperationCheck.cpp
   SizeofContainerCheck.cpp
   StaticAssertCheck.cpp
   StringIntegerAssignmentCheck.cpp
   StringLiteralWithEmbeddedNulCheck.cpp
   SuspiciousMissingCommaCheck.cpp
   SuspiciousSemicolonCheck.cpp
   SwappedArgumentsCheck.cpp
   ThrowByValueCatchByReferenceCheck.cpp

clang-tidy/misc/MiscTidyModule.cpp

 #include "MacroRepeatedSideEffectsCheck.h"
 #include "MisplacedWideningCastCheck.h"
 #include "MoveConstantArgumentCheck.h"
 #include "MoveConstructorInitCheck.h"
 #include "MultipleStatementMacroCheck.h"
 #include "NewDeleteOverloadsCheck.h"
 #include "NoexceptMoveConstructorCheck.h"
 #include "NonCopyableObjects.h"
+#include "PointerAndIntegralOperationCheck.h"
 #include "SizeofContainerCheck.h"
 #include "StaticAssertCheck.h"
 #include "StringIntegerAssignmentCheck.h"
 #include "StringLiteralWithEmbeddedNulCheck.h"
 #include "SuspiciousMissingCommaCheck.h"
 #include "SuspiciousSemicolonCheck.h"
 #include "SwappedArgumentsCheck.h"
 #include "ThrowByValueCatchByReferenceCheck.h"
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<MultipleStatementMacroCheck>(
         "misc-multiple-statement-macro");
     CheckFactories.registerCheck<NewDeleteOverloadsCheck>(
         "misc-new-delete-overloads");
     CheckFactories.registerCheck<NoexceptMoveConstructorCheck>(
         "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");
     CheckFactories.registerCheck<StringIntegerAssignmentCheck>(
         "misc-string-integer-assignment");
     CheckFactories.registerCheck<StringLiteralWithEmbeddedNulCheck>(
         "misc-string-literal-with-embedded-nul");
     CheckFactories.registerCheck<SuspiciousMissingCommaCheck>(

clang-tidy/misc/PointerAndIntegralOperationCheck.h

+//===--- 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

clang-tidy/misc/PointerAndIntegralOperationCheck.cpp

+//===--- 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

docs/ReleaseNotes.rst

- New `misc-forward-declaration-namespace
   Checks if an unused forward declaration is in a wrong namespace.
 
 - New `misc-misplaced-widening-cast
   <http://clang.llvm.org/extra/clang-tidy/checks/misc-misplaced-widening-cast.html>`_ check
 
   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
 
   Warns about suspicious NUL character in string literals which may lead to
   truncation or invalid character escaping.
 
 - New `misc-suspicious-missing-comma
   <http://clang.llvm.org/extra/clang-tidy/checks/misc-suspicious-missing-comma.html>`_ check

docs/clang-tidy/checks/list.rst

.. toctree::
    misc-macro-parentheses
    misc-macro-repeated-side-effects
    misc-misplaced-widening-cast
    misc-move-constructor-init
    misc-multiple-statement-macro
    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
    misc-string-literal-with-embedded-nul
    misc-suspicious-missing-comma
    misc-suspicious-semicolon
    misc-swapped-arguments
    misc-throw-by-value-catch-by-reference

docs/clang-tidy/checks/misc-pointer-and-integral-operation.rst

+.. 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
+      ...

test/clang-tidy/misc-pointer-and-integral-operation-cxx98.cpp

+// 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);
+}

test/clang-tidy/misc-pointer-and-integral-operation.cpp

+// 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);
+}