[clang-tidy] Add bugprone-pointer-arithmetic-on-polymorphic-object …

…check (llvm#91951)

Finds pointer arithmetic on classes that declare a virtual function.

This check corresponds to the SEI Cert rule [CTR56-CPP: Do not use
pointer arithmetic on polymorphic
objects](https://wiki.sei.cmu.edu/confluence/display/cplusplus/CTR56-CPP.+Do+not+use+pointer+arithmetic+on+polymorphic+objects).

```cpp
struct Base {
  virtual void ~Base();
};

struct Derived : public Base {};

void foo(Base *b) {
  b += 1; // passing `Derived` to `foo()` results in UB
}
```

[Results on open-source
projects](https://codechecker-demo.eastus.cloudapp.azure.com/Default/runs?run=Discookie-ctr56-with-classnames).
Most of the Qtbase reports are from having a `virtual override`
declaration, and the LLVM reports are true positives, as far as I can
tell.

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

@@ -51,6 +51,7 @@
 #include "NotNullTerminatedResultCheck.h"
 #include "OptionalValueConversionCheck.h"
 #include "ParentVirtualCallCheck.h"
+#include "PointerArithmeticOnPolymorphicObjectCheck.h"
 #include "PosixReturnCheck.h"
 #include "RedundantBranchConditionCheck.h"
 #include "ReservedIdentifierCheck.h"
@@ -171,6 +172,8 @@ class BugproneModule : public ClangTidyModule {
         "bugprone-multiple-statement-macro");
     CheckFactories.registerCheck<OptionalValueConversionCheck>(
         "bugprone-optional-value-conversion");
+    CheckFactories.registerCheck<PointerArithmeticOnPolymorphicObjectCheck>(
+        "bugprone-pointer-arithmetic-on-polymorphic-object");
     CheckFactories.registerCheck<RedundantBranchConditionCheck>(
         "bugprone-redundant-branch-condition");
     CheckFactories.registerCheck<cppcoreguidelines::NarrowingConversionsCheck>(

clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

@@ -48,6 +48,7 @@ add_clang_library(clangTidyBugproneModule
   NotNullTerminatedResultCheck.cpp
   OptionalValueConversionCheck.cpp
   ParentVirtualCallCheck.cpp
+  PointerArithmeticOnPolymorphicObjectCheck.cpp
   PosixReturnCheck.cpp
   RedundantBranchConditionCheck.cpp
   ReservedIdentifierCheck.cpp

clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.cpp

@@ -0,0 +1,81 @@
+//===--- PointerArithmeticOnPolymorphicObjectCheck.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 "PointerArithmeticOnPolymorphicObjectCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+namespace {
+AST_MATCHER(CXXRecordDecl, isAbstract) { return Node.isAbstract(); }
+AST_MATCHER(CXXRecordDecl, isPolymorphic) { return Node.isPolymorphic(); }
+} // namespace
+
+PointerArithmeticOnPolymorphicObjectCheck::
+    PointerArithmeticOnPolymorphicObjectCheck(StringRef Name,
+                                              ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      IgnoreInheritedVirtualFunctions(
+          Options.get("IgnoreInheritedVirtualFunctions", false)) {}
+
+void PointerArithmeticOnPolymorphicObjectCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnoreInheritedVirtualFunctions",
+                IgnoreInheritedVirtualFunctions);
+}
+
+void PointerArithmeticOnPolymorphicObjectCheck::registerMatchers(
+    MatchFinder *Finder) {
+  const auto PolymorphicPointerExpr =
+      expr(hasType(hasCanonicalType(pointerType(pointee(hasCanonicalType(
+               hasDeclaration(cxxRecordDecl(unless(isFinal()), isPolymorphic())
+                                  .bind("pointee"))))))))
+          .bind("pointer");
+
+  const auto PointerExprWithVirtualMethod =
+      expr(hasType(hasCanonicalType(
+               pointerType(pointee(hasCanonicalType(hasDeclaration(
+                   cxxRecordDecl(
+                       unless(isFinal()),
+                       anyOf(hasMethod(isVirtualAsWritten()), isAbstract()))
+                       .bind("pointee"))))))))
+          .bind("pointer");
+
+  const auto SelectedPointerExpr = IgnoreInheritedVirtualFunctions
+                                       ? PointerExprWithVirtualMethod
+                                       : PolymorphicPointerExpr;
+
+  const auto ArraySubscript = arraySubscriptExpr(hasBase(SelectedPointerExpr));
+
+  const auto BinaryOperators =
+      binaryOperator(hasAnyOperatorName("+", "-", "+=", "-="),
+                     hasEitherOperand(SelectedPointerExpr));
+
+  const auto UnaryOperators = unaryOperator(
+      hasAnyOperatorName("++", "--"), hasUnaryOperand(SelectedPointerExpr));
+
+  Finder->addMatcher(ArraySubscript, this);
+  Finder->addMatcher(BinaryOperators, this);
+  Finder->addMatcher(UnaryOperators, this);
+}
+
+void PointerArithmeticOnPolymorphicObjectCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *PointerExpr = Result.Nodes.getNodeAs<Expr>("pointer");
+  const auto *PointeeDecl = Result.Nodes.getNodeAs<CXXRecordDecl>("pointee");
+
+  diag(PointerExpr->getBeginLoc(),
+       "pointer arithmetic on polymorphic object of type %0 can result in "
+       "undefined behavior if the dynamic type differs from the pointer type")
+      << PointeeDecl << PointerExpr->getSourceRange();
+}
+
+} // namespace clang::tidy::bugprone

clang-tools-extra/clang-tidy/bugprone/PointerArithmeticOnPolymorphicObjectCheck.h

@@ -0,0 +1,41 @@
+//===--- PointerArithmeticOnPolymorphicObjectCheck.h ------------*- 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_POINTERARITHMETICONPOLYMORPHICOBJECTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_POINTERARITHMETICONPOLYMORPHICOBJECTCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Finds pointer arithmetic performed on classes that contain a
+/// virtual function.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/pointer-arithmetic-on-polymorphic-object.html
+class PointerArithmeticOnPolymorphicObjectCheck : public ClangTidyCheck {
+public:
+  PointerArithmeticOnPolymorphicObjectCheck(StringRef Name,
+                                            ClangTidyContext *Context);
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus;
+  }
+  std::optional<TraversalKind> getCheckTraversalKind() const override {
+    return TK_IgnoreUnlessSpelledInSource;
+  }
+
+private:
+  const bool IgnoreInheritedVirtualFunctions;
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_POINTERARITHMETICONPOLYMORPHICOBJECTCHECK_H

clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp

@@ -10,6 +10,7 @@
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "../bugprone/BadSignalToKillThreadCheck.h"
+#include "../bugprone/PointerArithmeticOnPolymorphicObjectCheck.h"
 #include "../bugprone/ReservedIdentifierCheck.h"
 #include "../bugprone/SignalHandlerCheck.h"
 #include "../bugprone/SignedCharMisuseCheck.h"
@@ -238,6 +239,10 @@ class CERTModule : public ClangTidyModule {
     // CON
     CheckFactories.registerCheck<bugprone::SpuriouslyWakeUpFunctionsCheck>(
         "cert-con54-cpp");
+    // CTR
+    CheckFactories
+        .registerCheck<bugprone::PointerArithmeticOnPolymorphicObjectCheck>(
+            "cert-ctr56-cpp");
     // DCL
     CheckFactories.registerCheck<VariadicFunctionDefCheck>("cert-dcl50-cpp");
     CheckFactories.registerCheck<bugprone::ReservedIdentifierCheck>(

clang-tools-extra/docs/ReleaseNotes.rst

@@ -137,6 +137,11 @@ New checks
   Detects error-prone Curiously Recurring Template Pattern usage, when the CRTP
   can be constructed outside itself and the derived class.
 
+- New :doc:`bugprone-pointer-arithmetic-on-polymorphic-object
+  <clang-tidy/checks/bugprone/pointer-arithmetic-on-polymorphic-object>` check.
+
+  Finds pointer arithmetic performed on classes that contain a virtual function.
+
 - New :doc:`bugprone-return-const-ref-from-parameter
   <clang-tidy/checks/bugprone/return-const-ref-from-parameter>` check.
 
@@ -199,6 +204,11 @@ New checks
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
+- New alias :doc:`cert-ctr56-cpp <clang-tidy/checks/cert/ctr56-cpp>` to
+  :doc:`bugprone-pointer-arithmetic-on-polymorphic-object
+  <clang-tidy/checks/bugprone/pointer-arithmetic-on-polymorphic-object>`
+  was added.
+
 - New alias :doc:`cert-int09-c <clang-tidy/checks/cert/int09-c>` to
   :doc:`readability-enum-initial-value <clang-tidy/checks/readability/enum-initial-value>`
   was added.

clang-tools-extra/docs/clang-tidy/checks/bugprone/pointer-arithmetic-on-polymorphic-object.rst

@@ -0,0 +1,68 @@
+.. title:: clang-tidy - bugprone-pointer-arithmetic-on-polymorphic-object
+
+bugprone-pointer-arithmetic-on-polymorphic-object
+=================================================
+
+Finds pointer arithmetic performed on classes that contain a virtual function.
+
+Pointer arithmetic on polymorphic objects where the pointer's static type is
+different from its dynamic type is undefined behavior, as the two types could
+have different sizes, and thus the vtable pointer could point to an
+invalid address.
+
+Finding pointers where the static type contains a virtual member function is a
+good heuristic, as the pointer is likely to point to a different,
+derived object.
+
+Example:
+
+.. code-block:: c++
+
+  struct Base {
+    virtual void ~Base();
+  };
+
+  struct Derived : public Base {};
+
+  void foo() {
+    Base *b = new Derived[10];
+
+    b += 1;
+    // warning: pointer arithmetic on class that declares a virtual function can
+    // result in undefined behavior if the dynamic type differs from the
+    // pointer type
+
+    delete[] static_cast<Derived*>(b);
+  }
+
+Options
+-------
+
+.. option:: IgnoreInheritedVirtualFunctions
+
+  When `true`, objects that only inherit a virtual function are not checked.
+  Classes that do not declare a new virtual function are excluded
+  by default, as they make up the majority of false positives.
+  Default: `false`.
+
+  .. code-block:: c++
+
+    void bar() {
+      Base *b = new Base[10];
+      b += 1; // warning, as Base declares a virtual destructor
+  
+      delete[] b;
+
+      Derived *d = new Derived[10]; // Derived overrides the destructor, and
+                                    // declares no other virtual functions
+      d += 1; // warning only if IgnoreVirtualDeclarationsOnly is set to false
+  
+      delete[] d;
+    }
+
+References
+----------
+
+This check corresponds to the SEI Cert rule
+`CTR56-CPP. Do not use pointer arithmetic on polymorphic objects
+<https://wiki.sei.cmu.edu/confluence/display/cplusplus/CTR56-CPP.+Do+not+use+pointer+arithmetic+on+polymorphic+objects>`_.

clang-tools-extra/docs/clang-tidy/checks/cert/ctr56-cpp.rst

@@ -0,0 +1,10 @@
+.. title:: clang-tidy - cert-ctr56-cpp
+.. meta::
+    :http-equiv=refresh: 5;URL=../bugprone/pointer-arithmetic-on-polymorphic-object.html
+
+cert-ctr56-cpp
+==============
+
+The `cert-ctr56-cpp` check is an alias, please see
+:doc:`bugprone-pointer-arithmetic-on-polymorphic-object
+<../bugprone/pointer-arithmetic-on-polymorphic-object>` for more information.

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -157,6 +157,7 @@ Clang-Tidy Checks
    :doc:`bugprone-unused-raii <bugprone/unused-raii>`, "Yes"
    :doc:`bugprone-unused-return-value <bugprone/unused-return-value>`,
    :doc:`bugprone-use-after-move <bugprone/use-after-move>`,
+   :doc:`bugprone-pointer-arithmetic-on-polymorphic-object <bugprone/pointer-arithmetic-on-polymorphic-object>`,
    :doc:`bugprone-virtual-near-miss <bugprone/virtual-near-miss>`, "Yes"
    :doc:`cert-dcl50-cpp <cert/dcl50-cpp>`,
    :doc:`cert-dcl58-cpp <cert/dcl58-cpp>`,