[clang] [clang][analyzer]Add C++ polymorphic ptr arithmetic checker (PR #82977)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 26 02:04:50 PST 2024
https://github.com/Discookie created https://github.com/llvm/llvm-project/pull/82977
This checker reports cases where an array of polymorphic objects are deleted as their base class.
Deleting an array where the array's static type is different from its dynamic type is undefined.
Similar in structure to `alpha.cplusplus.ArrayDelete`.
This checker 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).
>From 0318d05d4c6f4792287084fe90c23cd064a09d17 Mon Sep 17 00:00:00 2001
From: Viktor <viktor.cseh at ericsson.com>
Date: Thu, 22 Feb 2024 09:28:04 +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 | 180 ++++++++++++++++++
.../Analysis/PolymorphicPtrArithmetic.cpp | 141 ++++++++++++++
5 files changed, 360 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..1e8dfff74738d5
--- /dev/null
+++ b/clang/lib/StaticAnalyzer/Checkers/PolymorphicPtrArithmetic.cpp
@@ -0,0 +1,180 @@
+//=== 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