rnkovacs created this revision.
Herald added subscribers: baloghadamsoftware, xazax.hun, whisperity, mgorny.
This check warns if a derived type object is deleted through a base pointer
with a non-virtual destructor in its base class. It also places a note at the
last point where the derived-to-base conversion happened.
Corresponding CERT rule: OOP52-CPP: Do not delete a polymorphic object without
a virtual destructor.
<https://www.securecoding.cert.org/confluence/display/cplusplus/OOP52-CPP.+Do+not+delete+a+polymorphic+object+without+a+virtual+destructor>
https://reviews.llvm.org/D35796
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/CMakeLists.txt
lib/StaticAnalyzer/Checkers/MisusedPolymorphicObjectChecker.cpp
test/Analysis/MisusedPolymorphicObject.cpp
Index: test/Analysis/MisusedPolymorphicObject.cpp
===================================================================
--- /dev/null
+++ test/Analysis/MisusedPolymorphicObject.cpp
@@ -0,0 +1,168 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.cplusplus.PolymorphicDtorNotCalled -std=c++11 -verify -analyzer-output=text %s
+
+struct Virtual {
+ virtual ~Virtual() {}
+};
+
+struct VDerived : public Virtual {};
+
+struct NonVirtual {
+ ~NonVirtual() {}
+};
+
+struct NVDerived : public NonVirtual {};
+struct NVDoubleDerived : public NVDerived {};
+
+struct ImplicitNV {
+ virtual void f();
+};
+
+struct ImplicitNVDerived : public ImplicitNV {};
+
+NVDerived *get();
+
+NonVirtual *create() {
+ NonVirtual *x = new NVDerived(); // expected-note{{Derived-to-base conversion happened here}}
+ return x;
+}
+
+void sink(NonVirtual *x) {
+ delete x; // expected-warning{{Polymorphic object without virtual destructor deleted through base pointer}}
+ // expected-note@-1{{Polymorphic object without virtual destructor deleted through base pointer}}
+}
+
+void sinkCast(NonVirtual *y) {
+ delete reinterpret_cast<NVDerived*>(y);
+}
+
+void sinkParamCast(NVDerived *z) {
+ delete z;
+}
+
+void singleDerived() {
+ NonVirtual *sd;
+ sd = new NVDerived(); // expected-note{{Derived-to-base conversion happened here}}
+ delete sd; // expected-warning{{Polymorphic object without virtual destructor deleted through base pointer}}
+ // expected-note@-1{{Polymorphic object without virtual destructor deleted through base pointer}}
+}
+
+void singleDerivedArr() {
+ NonVirtual *sda = new NVDerived[5]; // expected-note{{Derived-to-base conversion happened here}}
+ delete[] sda; // expected-warning{{Polymorphic object without virtual destructor deleted through base pointer}}
+ // expected-note@-1{{Polymorphic object without virtual destructor deleted through base pointer}}
+}
+
+void doubleDerived() {
+ NonVirtual *dd = new NVDoubleDerived(); // expected-note{{Derived-to-base conversion happened here}}
+ delete (dd); // expected-warning{{Polymorphic object without virtual destructor deleted through base pointer}}
+ // expected-note@-1{{Polymorphic object without virtual destructor deleted through base pointer}}
+}
+
+void assignThroughFunction() {
+ NonVirtual *atf = get(); // expected-note{{Derived-to-base conversion happened here}}
+ delete atf; // expected-warning{{Polymorphic object without virtual destructor deleted through base pointer}}
+ // expected-note@-1{{Polymorphic object without virtual destructor deleted through base pointer}}
+}
+
+void assignThroughFunction2() {
+ NonVirtual *atf2;
+ atf2 = get(); // expected-note{{Derived-to-base conversion happened here}}
+ delete atf2; // expected-warning{{Polymorphic object without virtual destructor deleted through base pointer}}
+ // expected-note@-1{{Polymorphic object without virtual destructor deleted through base pointer}}
+}
+
+void createThroughFunction() {
+ NonVirtual *ctf = create(); // expected-note{{Calling 'create'}}
+ // expected-note@-1{{Returning from 'create'}}
+ delete ctf; // expected-warning {{Polymorphic object without virtual destructor deleted through base pointer}}
+ // expected-note@-1{{Polymorphic object without virtual destructor deleted through base pointer}}
+}
+
+void deleteThroughFunction() {
+ NonVirtual *dtf = new NVDerived(); // expected-note{{Derived-to-base conversion happened here}}
+ sink(dtf); // expected-note{{Calling 'sink'}}
+}
+
+void singleCastCStyle() {
+ NVDerived *sccs = new NVDerived();
+ NonVirtual *sccs2 = (NonVirtual*)sccs; // expected-note{{Derived-to-base conversion happened here}}
+ delete sccs2; // expected-warning{{Polymorphic object without virtual destructor deleted through base pointer}}
+ // expected-note@-1{{Polymorphic object without virtual destructor deleted through base pointer}}
+}
+
+void doubleCastCStyle() {
+ NonVirtual *dccs = new NVDerived();
+ NVDerived *dccs2 = (NVDerived*)dccs;
+ dccs = (NonVirtual*)dccs2; // expected-note{{Derived-to-base conversion happened here}}
+ delete dccs; // expected-warning{{Polymorphic object without virtual destructor deleted through base pointer}}
+ // expected-note@-1{{Polymorphic object without virtual destructor deleted through base pointer}}
+}
+
+void singleCast() {
+ NVDerived *sc = new NVDerived();
+ NonVirtual *sc2 = reinterpret_cast<NonVirtual*>(sc); // expected-note{{Derived-to-base conversion happened here}}
+ delete sc2; // expected-warning{{Polymorphic object without virtual destructor deleted through base pointer}}
+ // expected-note@-1{{Polymorphic object without virtual destructor deleted through base pointer}}
+}
+
+void doubleCast() {
+ NonVirtual *dd = new NVDerived();
+ NVDerived *dd2 = reinterpret_cast<NVDerived*>(dd);
+ dd = reinterpret_cast<NonVirtual*>(dd2); // expected-note {{Derived-to-base conversion happened here}}
+ delete dd; // expected-warning {{Polymorphic object without virtual destructor deleted through base pointer}}
+ // expected-note@-1{{Polymorphic object without virtual destructor deleted through base pointer}}
+}
+
+void implicitNV() {
+ ImplicitNV *invd = new ImplicitNVDerived(); // expected-note{{Derived-to-base conversion happened here}}
+ delete invd; // expected-warning{{Polymorphic object without virtual destructor deleted through base pointer}}
+ // expected-note@-1{{Polymorphic object without virtual destructor deleted through base pointer}}
+}
+
+void doubleDecl() {
+ ImplicitNV *dd1, *dd2;
+ dd1 = new ImplicitNVDerived(); // expected-note{{Derived-to-base conversion happened here}}
+ delete dd1; // expected-warning{{Polymorphic object without virtual destructor deleted through base pointer}}
+ // expected-note@-1{{Polymorphic object without virtual destructor deleted through base pointer}}
+}
+
+void virtualBase() {
+ Virtual *vb = new VDerived();
+ delete vb; // no-warning
+}
+
+void preciseType() {
+ NVDerived *pt = new NVDerived();
+ delete pt; // no-warning
+}
+
+void notDerived() {
+ NonVirtual *nd = new NonVirtual();
+ delete nd; // no-warning
+}
+
+void notDerivedArr() {
+ NonVirtual *nda = new NonVirtual[3];
+ delete[] nda; // no-warning
+}
+
+void cast() {
+ NonVirtual *c = new NVDerived();
+ delete reinterpret_cast<NVDerived*>(c); // no-warning
+}
+
+void deleteThroughFunction2() {
+ NonVirtual *dtf2 = new NVDerived();
+ sinkCast(dtf2); // no-warning
+}
+
+void deleteThroughFunction3() {
+ NVDerived *dtf3;
+ dtf3 = new NVDerived();
+ sinkParamCast(dtf3); // no-warning
+}
+
+void stackVar() {
+ NonVirtual sv2;
+ delete &sv2; // no-warning
+}
Index: lib/StaticAnalyzer/Checkers/MisusedPolymorphicObjectChecker.cpp
===================================================================
--- /dev/null
+++ lib/StaticAnalyzer/Checkers/MisusedPolymorphicObjectChecker.cpp
@@ -0,0 +1,146 @@
+//===-- MisusedPolymorphicObjectChecker.cpp -----------------------*- C++ -*--//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// Defines a checker for the OOP52-CPP CERT rule: Do not delete a polymorphic
+// object without a virtual destructor.
+//
+// This check warns if a derived type object is deleted through a base pointer
+// with a non-virtual destructor in its base class. It also places a note at
+// the last point where the derived-to-base conversion happened.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ClangSACheckers.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.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/DynamicTypeMap.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+
+using namespace clang;
+using namespace ento;
+
+namespace {
+class MisusedPolymorphicObjectChecker
+ : public Checker<check::PreStmt<CXXDeleteExpr>> {
+ mutable std::unique_ptr<BugType> BT;
+
+ class PolymorphicBugVisitor
+ : public BugReporterVisitorImpl<PolymorphicBugVisitor> {
+ public:
+ PolymorphicBugVisitor() : Satisfied(false) {}
+ void Profile(llvm::FoldingSetNodeID &ID) const override {
+ static int X = 0;
+ ID.AddPointer(&X);
+ }
+ std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N,
+ const ExplodedNode *PrevN,
+ BugReporterContext &BRC,
+ BugReport &BR) override;
+
+ private:
+ bool Satisfied;
+ };
+
+public:
+ void checkPreStmt(const CXXDeleteExpr *DE, CheckerContext &C) const;
+};
+} // end anonymous namespace
+
+void MisusedPolymorphicObjectChecker::checkPreStmt(const CXXDeleteExpr *DE,
+ CheckerContext &C) const {
+ const Expr *DeletedObj = DE->getArgument();
+ const MemRegion *MR = C.getSVal(DeletedObj).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->getDestructor()->isVirtual())
+ return;
+
+ if (!DerivedClass->isDerivedFrom(BaseClass))
+ return;
+
+ if (!BT)
+ BT.reset(new BugType(this,
+ "Polymorphic object without virtual destructor "
+ "deleted through base pointer",
+ "Undefined behavior (CERT OOP52-CPP)"));
+
+ ExplodedNode *N = C.generateNonFatalErrorNode();
+ auto R = llvm::make_unique<BugReport>(*BT, BT->getName(), N);
+
+ // Mark region of problematic base class for later use in the BugVisitor.
+ R->markInteresting(BaseClassRegion);
+ R->addVisitor(llvm::make_unique<PolymorphicBugVisitor>());
+ C.emitReport(std::move(R));
+}
+
+std::shared_ptr<PathDiagnosticPiece>
+MisusedPolymorphicObjectChecker::PolymorphicBugVisitor::VisitNode(
+ const ExplodedNode *N, const ExplodedNode *PrevN, BugReporterContext &BRC,
+ BugReport &BR) {
+ // Stop traversal after the first conversion was found on a path.
+ if (Satisfied)
+ return nullptr;
+
+ ProgramStateRef State = N->getState();
+ const LocationContext *LC = N->getLocationContext();
+ const Stmt *S = PathDiagnosticLocation::getStmt(N);
+ if (!S)
+ return nullptr;
+
+ const auto *CastE = dyn_cast<CastExpr>(S);
+ if (!CastE)
+ return nullptr;
+
+ // Only interested in DerivedToBase implicit casts.
+ // Explicit casts can have different CastKinds.
+ if (const auto *ImplCastE = dyn_cast<ImplicitCastExpr>(CastE)) {
+ if (ImplCastE->getCastKind() != CK_DerivedToBase)
+ return nullptr;
+ }
+
+ // Region associated with the current cast expression.
+ const MemRegion *M = State->getSVal(CastE, LC).getAsRegion();
+ if (!M)
+ return nullptr;
+
+ // Check if target region was marked as problematic previously.
+ if (!BR.isInteresting(M))
+ return nullptr;
+
+ // Stop traversal on this path.
+ Satisfied = true;
+
+ SmallString<256> Buf;
+ llvm::raw_svector_ostream OS(Buf);
+ OS << "Derived-to-base conversion happened here";
+ PathDiagnosticLocation Pos(S, BRC.getSourceManager(),
+ N->getLocationContext());
+ return std::make_shared<PathDiagnosticEventPiece>(Pos, OS.str(), true,
+ nullptr);
+}
+
+void ento::registerMisusedPolymorphicObjectChecker(CheckerManager &mgr) {
+ mgr.registerChecker<MisusedPolymorphicObjectChecker>();
+}
Index: lib/StaticAnalyzer/Checkers/CMakeLists.txt
===================================================================
--- lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -49,6 +49,7 @@
MallocOverflowSecurityChecker.cpp
MallocSizeofChecker.cpp
MisusedMovedObjectChecker.cpp
+ MisusedPolymorphicObjectChecker.cpp
MPI-Checker/MPIBugReporter.cpp
MPI-Checker/MPIChecker.cpp
MPI-Checker/MPIFunctionClassifier.cpp
Index: include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -293,6 +293,11 @@
"object will be reported">,
DescFile<"MisusedMovedObjectChecker.cpp">;
+def MisusedPolymorphicObjectChecker: Checker<"MisusedPolymorphicObject">,
+ HelpText<"Reports deletions of polymorphic objects with a non-virtual "
+ "destructor in their base class.">,
+ DescFile<"MisusedPolymorphicObjectChecker.cpp">;
+
} // end: "alpha.cplusplus"
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
