[clang] [clang][analyzer]Add C++ polymorphic ptr arithmetic checker (PR #82977)

via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 26 02:06:12 PST 2024


https://github.com/Discookie updated https://github.com/llvm/llvm-project/pull/82977

>From 1454b5bb4fad03b043c25a64c8c4f4495e17a316 Mon Sep 17 00:00:00 2001
From: Viktor <viktor.cseh at ericsson.com>
Date: Mon, 26 Feb 2024 10:06:01 +0000
Subject: [PATCH] [clang][analyzer]Add C++ polymorphic ptr arithmetic checker

---
 clang/docs/analyzer/checkers.rst              |  33 ++++
 .../clang/StaticAnalyzer/Checkers/Checkers.td |   5 +
 .../StaticAnalyzer/Checkers/CMakeLists.txt    |   1 +
 .../Checkers/PolymorphicPtrArithmetic.cpp     | 178 ++++++++++++++++++
 .../Analysis/PolymorphicPtrArithmetic.cpp     | 141 ++++++++++++++
 5 files changed, 358 insertions(+)
 create mode 100644 clang/lib/StaticAnalyzer/Checkers/PolymorphicPtrArithmetic.cpp
 create mode 100644 clang/test/Analysis/PolymorphicPtrArithmetic.cpp

diff --git a/clang/docs/analyzer/checkers.rst b/clang/docs/analyzer/checkers.rst
index 510629d8a2d480..5d000dbc03181d 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -581,6 +581,39 @@ store combinations of flag values:
 
 Projects that use this pattern should not enable this optin checker.
 
+.. _optin-cplusplus-PolymorphicPtrArithmetic:
+
+optin.cplusplus.PolymorphicPtrArithmetic (C++)
+""""""""""""""""""""""""""""""""""""""""""""""
+
+This checker reports pointer arithmetic operations on arrays of polymorphic
+objects, where the array has the type of its base class.
+Deleting an array where the array's static type is different from its dynamic
+type is undefined.
+
+This checker corresponds to the 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>`_.
+
+.. code-block:: cpp
+
+ class Base {
+ public:
+   int member = 0;
+   virtual ~Base() {}
+ };
+ class Derived : public Base {}
+
+ Base *create() {
+   Base *x = new Derived[10]; // note: Casting from 'Derived' to 'Base' here
+   return x;
+ }
+
+ void foo() {
+   Base *x = create();
+   (x + 3)->member += 1; // warn: Doing pointer arithmetic with 'Derived' objects as their base class 'Base' is undefined
+   x[3].member += 1;  // warn: Doing pointer arithmetic with 'Derived' objects as their base class 'Base' is undefined
+   delete[] static_cast<Derived*>(x);
+ }
+
 .. _optin-cplusplus-UninitializedObject:
 
 optin.cplusplus.UninitializedObject (C++)
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index e7774e5a9392d2..85ffe6f42cf40e 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -706,6 +706,11 @@ def PureVirtualCallChecker : Checker<"PureVirtualCall">,
 
 let ParentPackage = CplusplusOptIn in {
 
+def PolymorphicPtrArithmeticChecker : Checker<"PolymorphicPtrArithmetic">,
+  HelpText<"Reports doing pointer arithmetic on arrays of polymorphic objects "
+           "with the type of their base class">,
+  Documentation<HasDocumentation>;
+
 def UninitializedObjectChecker: Checker<"UninitializedObject">,
   HelpText<"Reports uninitialized fields after object construction">,
   CheckerOptions<[
diff --git a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index 4443ffd0929388..a29b36a04cf064 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -31,6 +31,7 @@ add_clang_library(clangStaticAnalyzerCheckers
   ContainerModeling.cpp
   ConversionChecker.cpp
   CXXDeleteChecker.cpp
+  PolymorphicPtrArithmetic.cpp
   CXXSelfAssignmentChecker.cpp
   DeadStoresChecker.cpp
   DebugCheckers.cpp
diff --git a/clang/lib/StaticAnalyzer/Checkers/PolymorphicPtrArithmetic.cpp b/clang/lib/StaticAnalyzer/Checkers/PolymorphicPtrArithmetic.cpp
new file mode 100644
index 00000000000000..dfa71e1cfd2ef7
--- /dev/null
+++ b/clang/lib/StaticAnalyzer/Checkers/PolymorphicPtrArithmetic.cpp
@@ -0,0 +1,178 @@
+//=== PolymorphicPtrArithmetic.cpp ------------------------------*- 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
+//
+//===----------------------------------------------------------------------===//
+//
+// This checker reports pointer arithmetic operations on arrays of
+// polymorphic objects, where the array has the type of its base class.
+// Corresponds to the CTR56-CPP. Do not use pointer arithmetic on
+// polymorphic objects
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/CheckerManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicType.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+class PolymorphicPtrArithmeticChecker
+    : public Checker<check::PreStmt<BinaryOperator>,
+                     check::PreStmt<ArraySubscriptExpr>> {
+  const BugType BT{this,
+                   "Pointer arithmetic on polymorphic objects is undefined"};
+
+protected:
+  class PtrCastVisitor : public BugReporterVisitor {
+  public:
+    void Profile(llvm::FoldingSetNodeID &ID) const override {
+      static int X = 0;
+      ID.AddPointer(&X);
+    }
+    PathDiagnosticPieceRef VisitNode(const ExplodedNode *N,
+                                     BugReporterContext &BRC,
+                                     PathSensitiveBugReport &BR) override;
+  };
+
+  void checkTypedExpr(const Expr *E, CheckerContext &C) const;
+
+public:
+  void checkPreStmt(const BinaryOperator *B, CheckerContext &C) const;
+  void checkPreStmt(const ArraySubscriptExpr *B, CheckerContext &C) const;
+};
+} // namespace
+
+void PolymorphicPtrArithmeticChecker::checkPreStmt(const BinaryOperator *B,
+                                                   CheckerContext &C) const {
+  if (!B->isAdditiveOp())
+    return;
+
+  bool IsLHSPtr = B->getLHS()->getType()->isAnyPointerType();
+  bool IsRHSPtr = B->getRHS()->getType()->isAnyPointerType();
+  if (!IsLHSPtr && !IsRHSPtr)
+    return;
+
+  const Expr *E = IsLHSPtr ? B->getLHS() : B->getRHS();
+
+  checkTypedExpr(E, C);
+}
+
+void PolymorphicPtrArithmeticChecker::checkPreStmt(const ArraySubscriptExpr *B,
+                                                   CheckerContext &C) const {
+  bool IsLHSPtr = B->getLHS()->getType()->isAnyPointerType();
+  bool IsRHSPtr = B->getRHS()->getType()->isAnyPointerType();
+  if (!IsLHSPtr && !IsRHSPtr)
+    return;
+
+  const Expr *E = IsLHSPtr ? B->getLHS() : B->getRHS();
+
+  checkTypedExpr(E, C);
+}
+
+void PolymorphicPtrArithmeticChecker::checkTypedExpr(const Expr *E,
+                                                     CheckerContext &C) const {
+  const MemRegion *MR = C.getSVal(E).getAsRegion();
+  if (!MR)
+    return;
+
+  const auto *BaseClassRegion = MR->getAs<TypedValueRegion>();
+  const auto *DerivedClassRegion = MR->getBaseRegion()->getAs<SymbolicRegion>();
+  if (!BaseClassRegion || !DerivedClassRegion)
+    return;
+
+  const auto *BaseClass = BaseClassRegion->getValueType()->getAsCXXRecordDecl();
+  const auto *DerivedClass =
+      DerivedClassRegion->getSymbol()->getType()->getPointeeCXXRecordDecl();
+  if (!BaseClass || !DerivedClass)
+    return;
+
+  if (!BaseClass->hasDefinition() || !DerivedClass->hasDefinition())
+    return;
+
+  if (!DerivedClass->isDerivedFrom(BaseClass))
+    return;
+
+  ExplodedNode *N = C.generateNonFatalErrorNode();
+  if (!N)
+    return;
+
+  SmallString<256> Buf;
+  llvm::raw_svector_ostream OS(Buf);
+
+  QualType SourceType = BaseClassRegion->getValueType();
+  QualType TargetType =
+      DerivedClassRegion->getSymbol()->getType()->getPointeeType();
+
+  OS << "Doing pointer arithmetic with '" << TargetType.getAsString()
+     << "' objects as their base class '"
+     << SourceType.getAsString(C.getASTContext().getPrintingPolicy())
+     << "' is undefined";
+
+  auto R = std::make_unique<PathSensitiveBugReport>(BT, OS.str(), N);
+
+  // Mark region of problematic base class for later use in the BugVisitor.
+  R->markInteresting(BaseClassRegion);
+  R->addVisitor<PtrCastVisitor>();
+  C.emitReport(std::move(R));
+}
+
+PathDiagnosticPieceRef
+PolymorphicPtrArithmeticChecker::PtrCastVisitor::VisitNode(
+    const ExplodedNode *N, BugReporterContext &BRC,
+    PathSensitiveBugReport &BR) {
+  const Stmt *S = N->getStmtForDiagnostics();
+  if (!S)
+    return nullptr;
+
+  const auto *CastE = dyn_cast<CastExpr>(S);
+  if (!CastE)
+    return nullptr;
+
+  // FIXME: This way of getting base types does not support reference types.
+  QualType SourceType = CastE->getSubExpr()->getType()->getPointeeType();
+  QualType TargetType = CastE->getType()->getPointeeType();
+
+  if (SourceType.isNull() || TargetType.isNull() || SourceType == TargetType)
+    return nullptr;
+
+  // Region associated with the current cast expression.
+  const MemRegion *M = N->getSVal(CastE).getAsRegion();
+  if (!M)
+    return nullptr;
+
+  // Check if target region was marked as problematic previously.
+  if (!BR.isInteresting(M))
+    return nullptr;
+
+  SmallString<256> Buf;
+  llvm::raw_svector_ostream OS(Buf);
+
+  OS << "Casting from '" << SourceType.getAsString() << "' to '"
+     << TargetType.getAsString() << "' here";
+
+  PathDiagnosticLocation Pos(S, BRC.getSourceManager(),
+                             N->getLocationContext());
+  return std::make_shared<PathDiagnosticEventPiece>(Pos, OS.str(),
+                                                    /*addPosRange=*/true);
+}
+
+void ento::registerPolymorphicPtrArithmeticChecker(CheckerManager &mgr) {
+  mgr.registerChecker<PolymorphicPtrArithmeticChecker>();
+}
+
+bool ento::shouldRegisterPolymorphicPtrArithmeticChecker(
+    const CheckerManager &mgr) {
+  return true;
+}
diff --git a/clang/test/Analysis/PolymorphicPtrArithmetic.cpp b/clang/test/Analysis/PolymorphicPtrArithmetic.cpp
new file mode 100644
index 00000000000000..77dd6ff3853e44
--- /dev/null
+++ b/clang/test/Analysis/PolymorphicPtrArithmetic.cpp
@@ -0,0 +1,141 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=optin.cplusplus.PolymorphicPtrArithmetic -std=c++11 -verify -analyzer-output=text %s
+
+struct Base {
+    int x = 0;
+    virtual ~Base() = default;
+};
+
+struct Derived : public Base {};
+
+struct DoubleDerived : public Derived {};
+
+Derived *get();
+
+Base *create() {
+    Base *b = new Derived[3]; // expected-note{{Casting from 'Derived' to 'Base' here}}
+    return b;
+}
+
+void sink(Base *b) {
+    b[1].x++; // expected-warning{{Doing pointer arithmetic with 'Derived' objects as their base class 'Base' is undefined}}
+    // expected-note at -1{{Doing pointer arithmetic with 'Derived' objects as their base class 'Base' is undefined}}
+    delete[] static_cast<Derived*>(b);
+}
+
+void sink_cast(Base *b) {
+    static_cast<Derived*>(b)[1].x++; // no-warning
+    delete[] static_cast<Derived*>(b);
+}
+
+void sink_derived(Derived *d) {
+    d[1].x++; // no-warning
+    delete[] static_cast<Derived*>(d);
+}
+
+void same_function() {
+    Base *sd = new Derived[10]; // expected-note{{Casting from 'Derived' to 'Base' here}}
+    // expected-note at -1{{Casting from 'Derived' to 'Base' here}}
+    sd[1].x++; // expected-warning{{Doing pointer arithmetic with 'Derived' objects as their base class 'Base' is undefined}}
+    // expected-note at -1{{Doing pointer arithmetic with 'Derived' objects as their base class 'Base' is undefined}}
+    (sd + 1)->x++; // expected-warning{{Doing pointer arithmetic with 'Derived' objects as their base class 'Base' is undefined}}
+    // expected-note at -1{{Doing pointer arithmetic with 'Derived' objects as their base class 'Base' is undefined}}
+    delete[] static_cast<Derived*>(sd);
+    
+    Base *dd = new DoubleDerived[10]; // expected-note{{Casting from 'DoubleDerived' to 'Base' here}}
+    dd[1].x++; // expected-warning{{Doing pointer arithmetic with 'DoubleDerived' objects as their base class 'Base' is undefined}}
+    // expected-note at -1{{Doing pointer arithmetic with 'DoubleDerived' objects as their base class 'Base' is undefined}}
+    delete[] static_cast<DoubleDerived*>(dd);
+
+}
+
+void different_function() {
+    Base *assigned = get(); // expected-note{{Casting from 'Derived' to 'Base' here}}
+    assigned[1].x++; // expected-warning{{Doing pointer arithmetic with 'Derived' objects as their base class 'Base' is undefined}}
+    // expected-note at -1{{Doing pointer arithmetic with 'Derived' objects as their base class 'Base' is undefined}}
+    delete[] static_cast<Derived*>(assigned);
+
+    Base *indirect;
+    indirect = get(); // expected-note{{Casting from 'Derived' to 'Base' here}}
+    indirect[1].x++; // expected-warning{{Doing pointer arithmetic with 'Derived' objects as their base class 'Base' is undefined}}
+    // expected-note at -1{{Doing pointer arithmetic with 'Derived' objects as their base class 'Base' is undefined}}
+    delete[] static_cast<Derived*>(indirect);
+
+    Base *created = create(); // expected-note{{Calling 'create'}}
+    // expected-note at -1{{Returning from 'create'}}
+    created[1].x++; // expected-warning{{Doing pointer arithmetic with 'Derived' objects as their base class 'Base' is undefined}}
+    // expected-note at -1{{Doing pointer arithmetic with 'Derived' objects as their base class 'Base' is undefined}}
+    delete[] static_cast<Derived*>(created);
+
+    Base *sb = new Derived[10]; // expected-note{{Casting from 'Derived' to 'Base' here}}
+    sink(sb); // expected-note{{Calling 'sink'}}
+}
+
+void safe_function() {
+    Derived *d = new Derived[10];
+    d[1].x++; // no-warning
+    delete[] static_cast<Derived*>(d);
+
+    Base *b = new Derived[10];
+    static_cast<Derived*>(b)[1].x++; // no-warning
+    delete[] static_cast<Derived*>(b);
+
+    Base *sb = new Derived[10];
+    sink_cast(sb); // no-warning
+
+    Derived *sd = new Derived[10];
+    sink_derived(sd); // no-warning
+}
+
+void multiple_derived() {
+    Base *b = new DoubleDerived[10]; // expected-note{{Casting from 'DoubleDerived' to 'Base' here}}
+    b[1].x++; // expected-warning{{Doing pointer arithmetic with 'DoubleDerived' objects as their base class 'Base' is undefined}}
+    // expected-note at -1{{Doing pointer arithmetic with 'DoubleDerived' objects as their base class 'Base' is undefined}}
+    delete[] static_cast<DoubleDerived*>(b);
+
+    Base *b2 = new DoubleDerived[10]; // expected-note{{Casting from 'DoubleDerived' to 'Base' here}}
+    Derived *d2 = static_cast<Derived*>(b2); // expected-note{{Casting from 'Base' to 'Derived' here}}
+    d2[1].x++; // expected-warning{{Doing pointer arithmetic with 'DoubleDerived' objects as their base class 'Derived' is undefined}}
+    // expected-note at -1{{Doing pointer arithmetic with 'DoubleDerived' objects as their base class 'Derived' is undefined}}
+    delete[] static_cast<DoubleDerived*>(d2);
+
+    Derived *d3 = new DoubleDerived[10]; // expected-note{{Casting from 'DoubleDerived' to 'Derived' here}}
+    Base *b3 = d3; // expected-note{{Casting from 'Derived' to 'Base' here}}
+    b3[1].x++; // expected-warning{{Doing pointer arithmetic with 'DoubleDerived' objects as their base class 'Base' is undefined}}
+    // expected-note at -1{{Doing pointer arithmetic with 'DoubleDerived' objects as their base class 'Base' is undefined}}
+    delete[] static_cast<DoubleDerived*>(b3);
+
+    Base *b4 = new DoubleDerived[10];
+    Derived *d4 = static_cast<Derived*>(b4);
+    DoubleDerived *dd4 = static_cast<DoubleDerived*>(d4);
+    dd4[1].x++; // no-warning
+    delete[] static_cast<DoubleDerived*>(dd4);
+
+    Base *b5 = new DoubleDerived[10]; // expected-note{{Casting from 'DoubleDerived' to 'Base' here}}
+    DoubleDerived *dd5 = static_cast<DoubleDerived*>(b5); // expected-note{{Casting from 'Base' to 'DoubleDerived' here}}
+    Derived *d5 = dd5; // expected-note{{Casting from 'DoubleDerived' to 'Derived' here}}
+    d5[1].x++; // expected-warning{{Doing pointer arithmetic with 'DoubleDerived' objects as their base class 'Derived' is undefined}}
+    // expected-note at -1{{Doing pointer arithmetic with 'DoubleDerived' objects as their base class 'Derived' is undefined}}
+    delete[] static_cast<DoubleDerived*>(d5);
+}
+
+void other_operators() {
+    Derived *d = new Derived[10];
+    Base *b1 = d;
+    Base *b2 = d + 1;
+    Base *b3 = new Derived[10];
+    if (b1 < b2) return; // no-warning
+    if (b1 == b3) return; // no-warning
+}
+
+void unrelated_casts() {
+    Base *b = new DoubleDerived[10]; // expected-note{{Casting from 'DoubleDerived' to 'Base' here}}
+    Base &b2 = *b; // no-note: See the FIXME.
+
+    // FIXME: Displaying casts of reference types is not supported.
+    Derived &d2 = static_cast<Derived&>(b2); // no-note: See the FIXME.
+
+    Derived *d = &d2; // no-note: See the FIXME.
+    d[1].x++; // expected-warning{{Doing pointer arithmetic with 'DoubleDerived' objects as their base class 'Derived' is undefined}}
+    // expected-note at -1{{Doing pointer arithmetic with 'DoubleDerived' objects as their base class 'Derived' is undefined}}
+    delete[] static_cast<DoubleDerived*>(d);
+}



More information about the cfe-commits mailing list