[clang-tidy] Introduce misc No Integer To Pointer Cast check

While casting an (integral) pointer to an integer is obvious - you just get
the integral value of the pointer, casting an integer to an (integral) pointer
is deceivingly different. While you will get a pointer with that integral value,
if you got that integral value via a pointer-to-integer cast originally,
the new pointer will lack the provenance information from the original pointer.

So while (integral) pointer to integer casts are effectively no-ops,
and are transparent to the optimizer, integer to (integral) pointer casts
are *NOT* transparent, and may conceal information from optimizer.

While that may be the intention, it is not always so. For example,
let's take a look at a routine to align the pointer up to the multiple of 16:
The obvious, naive implementation for that is:

```
  char* src(char* maybe_underbiased_ptr) {
    uintptr_t maybe_underbiased_intptr = (uintptr_t)maybe_underbiased_ptr;
    uintptr_t aligned_biased_intptr = maybe_underbiased_intptr + 15;
    uintptr_t aligned_intptr = aligned_biased_intptr & (~15);
    return (char*)aligned_intptr; // warning: avoid integer to pointer casts [misc-no-inttoptr]
  }
```

The check will rightfully diagnose that cast.

But when provenance concealment is not the goal of the code, but an accident,
this example can be rewritten as follows, without using integer to pointer cast:

```
  char*
  tgt(char* maybe_underbiased_ptr) {
      uintptr_t maybe_underbiased_intptr = (uintptr_t)maybe_underbiased_ptr;
      uintptr_t aligned_biased_intptr = maybe_underbiased_intptr + 15;
      uintptr_t aligned_intptr = aligned_biased_intptr & (~15);
      uintptr_t bias = aligned_intptr - maybe_underbiased_intptr;
      return maybe_underbiased_ptr + bias;
  }
```

See also:
* D71499
* [[ https://www.cs.utah.edu/~regehr/oopsla18.pdf | Juneyoung Lee, Chung-Kil Hur, Ralf Jung, Zhengyang Liu, John Regehr, and Nuno P. Lopes. 2018. Reconciling High-Level Optimizations and Low-Level Code in LLVM. Proc. ACM Program. Lang. 2, OOPSLA, Article 125 (November 2018), 28 pages. ]]

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D91055

diff --git a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
index 0e38e9f..3153646 100644 (file)
--- a/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/performance/CMakeLists.txt
@@ -13,6 +13,7 @@ add_clang_library(clangTidyPerformanceModule
   MoveConstArgCheck.cpp
   MoveConstructorInitCheck.cpp
   NoAutomaticMoveCheck.cpp
+  NoIntToPtrCheck.cpp
   NoexceptMoveConstructorCheck.cpp
   PerformanceTidyModule.cpp
   TriviallyDestructibleCheck.cpp

diff --git a/clang-tools-extra/clang-tidy/performance/NoIntToPtrCheck.cpp b/clang-tools-extra/clang-tidy/performance/NoIntToPtrCheck.cpp
new file mode 100644 (file)
index 0000000..bc82a21
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/performance/NoIntToPtrCheck.cpp
@@ -0,0 +1,34 @@
+//===--- NoIntToPtrCheck.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 "NoIntToPtrCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+void NoIntToPtrCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(castExpr(hasCastKind(CK_IntegralToPointer),
+                              unless(hasSourceExpression(integerLiteral())))
+                         .bind("x"),
+                     this);
+}
+
+void NoIntToPtrCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedCast = Result.Nodes.getNodeAs<CastExpr>("x");
+  diag(MatchedCast->getBeginLoc(),
+       "integer to pointer cast pessimizes optimization opportunities");
+}
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang

diff --git a/clang-tools-extra/clang-tidy/performance/NoIntToPtrCheck.h b/clang-tools-extra/clang-tidy/performance/NoIntToPtrCheck.h
new file mode 100644 (file)
index 0000000..f073431
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/performance/NoIntToPtrCheck.h
@@ -0,0 +1,34 @@
+//===--- NoIntToPtrCheck.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_PERFORMANCE_NOINTTOPTRCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOINTTOPTRCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace performance {
+
+/// Diagnoses every integer to pointer cast.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/performance-no-int-to-ptr.html
+class NoIntToPtrCheck : public ClangTidyCheck {
+public:
+  NoIntToPtrCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace performance
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_PERFORMANCE_NOINTTOPTRCHECK_H

diff --git a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
index 87673a2..ba9d7ee 100644 (file)
--- a/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
@@ -18,6 +18,7 @@
 #include "MoveConstArgCheck.h"
 #include "MoveConstructorInitCheck.h"
 #include "NoAutomaticMoveCheck.h"
+#include "NoIntToPtrCheck.h"
 #include "NoexceptMoveConstructorCheck.h"
 #include "TriviallyDestructibleCheck.h"
 #include "TypePromotionInMathFnCheck.h"
@@ -49,6 +50,7 @@ public:
         "performance-move-constructor-init");
     CheckFactories.registerCheck<NoAutomaticMoveCheck>(
         "performance-no-automatic-move");
+    CheckFactories.registerCheck<NoIntToPtrCheck>("performance-no-int-to-ptr");
     CheckFactories.registerCheck<NoexceptMoveConstructorCheck>(
         "performance-noexcept-move-constructor");
     CheckFactories.registerCheck<TriviallyDestructibleCheck>(

diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 3df5b1c..0622166 100644 (file)
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -135,6 +135,11 @@ New checks
   Alias to the :doc:`bugprone-signal-handler
   <clang-tidy/checks/bugprone-signal-handler>` check.
 
+- New :doc:`performance-no-int-to-ptr
+  <clang-tidy/checks/performance-no-int-to-ptr>` check.
+
+  Diagnoses every integer to pointer cast.
+
 - New :doc:`readability-function-cognitive-complexity
   <clang-tidy/checks/readability-function-cognitive-complexity>` check.
 

diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 9c7074e..6c882bb 100644 (file)
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -265,6 +265,7 @@ Clang-Tidy Checks
    `performance-move-const-arg <performance-move-const-arg.html>`_, "Yes"
    `performance-move-constructor-init <performance-move-constructor-init.html>`_, "Yes"
    `performance-no-automatic-move <performance-no-automatic-move.html>`_,
+   `performance-no-int-to-ptr <performance-no-int-to-ptr.html>`_,
    `performance-noexcept-move-constructor <performance-noexcept-move-constructor.html>`_, "Yes"
    `performance-trivially-destructible <performance-trivially-destructible.html>`_, "Yes"
    `performance-type-promotion-in-math-fn <performance-type-promotion-in-math-fn.html>`_, "Yes"

diff --git a/clang-tools-extra/docs/clang-tidy/checks/performance-no-int-to-ptr.rst b/clang-tools-extra/docs/clang-tidy/checks/performance-no-int-to-ptr.rst
new file mode 100644 (file)
index 0000000..8233ea1
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/performance-no-int-to-ptr.rst
@@ -0,0 +1,45 @@
+.. title:: clang-tidy - performance-no-int-to-ptr
+
+performance-no-int-to-ptr
+=========================
+
+Diagnoses every integer to pointer cast.
+
+While casting an (integral) pointer to an integer is obvious - you just get
+the integral value of the pointer, casting an integer to an (integral) pointer
+is deceivingly different. While you will get a pointer with that integral value,
+if you got that integral value via a pointer-to-integer cast originally,
+the new pointer will lack the provenance information from the original pointer.
+
+So while (integral) pointer to integer casts are effectively no-ops,
+and are transparent to the optimizer, integer to (integral) pointer casts
+are *NOT* transparent, and may conceal information from optimizer.
+
+While that may be the intention, it is not always so. For example,
+let's take a look at a routine to align the pointer up to the multiple of 16:
+The obvious, naive implementation for that is:
+
+.. code-block:: c++
+
+  char* src(char* maybe_underbiased_ptr) {
+    uintptr_t maybe_underbiased_intptr = (uintptr_t)maybe_underbiased_ptr;
+    uintptr_t aligned_biased_intptr = maybe_underbiased_intptr + 15;
+    uintptr_t aligned_intptr = aligned_biased_intptr & (~15);
+    return (char*)aligned_intptr; // warning: avoid integer to pointer casts [performance-no-int-to-ptr]
+  }
+
+The check will rightfully diagnose that cast.
+
+But when provenance concealment is not the goal of the code, but an accident,
+this example can be rewritten as follows, without using integer to pointer cast:
+
+.. code-block:: c++
+
+  char*
+  tgt(char* maybe_underbiased_ptr) {
+      uintptr_t maybe_underbiased_intptr = (uintptr_t)maybe_underbiased_ptr;
+      uintptr_t aligned_biased_intptr = maybe_underbiased_intptr + 15;
+      uintptr_t aligned_intptr = aligned_biased_intptr & (~15);
+      uintptr_t bias = aligned_intptr - maybe_underbiased_intptr;
+      return maybe_underbiased_ptr + bias;
+  }

diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance-no-int-to-ptr.c b/clang-tools-extra/test/clang-tidy/checkers/performance-no-int-to-ptr.c
new file mode 100644 (file)
index 0000000..90f51bc
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance-no-int-to-ptr.c
@@ -0,0 +1,66 @@
+// RUN: %check_clang_tidy %s performance-no-int-to-ptr %t
+
+void *t0(char x) {
+  return x;
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr]
+}
+void *t1(signed char x) {
+  return x;
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr]
+}
+void *t2(unsigned char x) {
+  return x;
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr]
+}
+
+void *t3(short x) {
+  return x;
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr]
+}
+void *t4(unsigned short x) {
+  return x;
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr]
+}
+void *t5(signed short x) {
+  return x;
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr]
+}
+
+void *t6(int x) {
+  return x;
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr]
+}
+void *t7(unsigned int x) {
+  return x;
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr]
+}
+void *t8(signed int x) {
+  return x;
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr]
+}
+
+void *t9(long x) {
+  return x;
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr]
+}
+void *t10(unsigned long x) {
+  return x;
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr]
+}
+void *t11(signed long x) {
+  return x;
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr]
+}
+
+void *t12(long long x) {
+  return x;
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr]
+}
+void *t13(unsigned long long x) {
+  return x;
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr]
+}
+void *t14(signed long long x) {
+  return x;
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr]
+}

diff --git a/clang-tools-extra/test/clang-tidy/checkers/performance-no-int-to-ptr.cpp b/clang-tools-extra/test/clang-tidy/checkers/performance-no-int-to-ptr.cpp
new file mode 100644 (file)
index 0000000..adda32e
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/performance-no-int-to-ptr.cpp
@@ -0,0 +1,22 @@
+// RUN: %check_clang_tidy %s performance-no-int-to-ptr %t
+
+// can't implicitly cast int to a pointer.
+// can't use static_cast<>() to cast integer to a pointer.
+// can't use dynamic_cast<>() to cast integer to a pointer.
+// can't use const_cast<>() to cast integer to a pointer.
+
+void *t0(long long int x) {
+  return (void *)x;
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr]
+}
+
+void *t1(int x) {
+  return reinterpret_cast<void *>(x);
+  // CHECK-MESSAGES: [[@LINE-1]]:10: warning: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr]
+}
+
+// Don't diagnose casts from integer literals.
+// It's a widely-used technique in embedded/microcontroller/hardware interfacing.
+void *t3(long long int x) {
+  return (void *)0xFEEDFACE;
+}