Index: cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td =================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td +++ cfe/trunk/include/clang/StaticAnalyzer/Checkers/Checkers.td @@ -284,6 +284,11 @@ let ParentPackage = CplusplusAlpha in { +def DeleteWithNonVirtualDtorChecker : Checker<"DeleteWithNonVirtualDtor">, + HelpText<"Reports destructions of polymorphic objects with a non-virtual " + "destructor in their base class">, + DescFile<"DeleteWithNonVirtualDtorChecker.cpp">; + def IteratorRangeChecker : Checker<"IteratorRange">, HelpText<"Check for iterators used outside their valid ranges">, DescFile<"IteratorChecker.cpp">; Index: cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt +++ cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt @@ -29,6 +29,7 @@ CXXSelfAssignmentChecker.cpp DeadStoresChecker.cpp DebugCheckers.cpp + DeleteWithNonVirtualDtorChecker.cpp DereferenceChecker.cpp DirectIvarAssignment.cpp DivZeroChecker.cpp Index: cfe/trunk/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp @@ -0,0 +1,153 @@ +//===-- DeleteWithNonVirtualDtorChecker.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. +// +// Diagnostic flags -Wnon-virtual-dtor and -Wdelete-non-virtual-dtor report if +// an object with a virtual function but a non-virtual destructor exists or is +// deleted, respectively. +// +// This check exceeds them by comparing the dynamic and static types of the +// object at the point of destruction and only warns if it happens through a +// pointer to a base type without a virtual destructor. The check places a note +// at the last point where the conversion from derived to base 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 DeleteWithNonVirtualDtorChecker + : public Checker> { + mutable std::unique_ptr BT; + + class DeleteBugVisitor : public BugReporterVisitorImpl { + public: + DeleteBugVisitor() : Satisfied(false) {} + void Profile(llvm::FoldingSetNodeID &ID) const override { + static int X = 0; + ID.AddPointer(&X); + } + std::shared_ptr 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 DeleteWithNonVirtualDtorChecker::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(); + const auto *DerivedClassRegion = MR->getBaseRegion()->getAs(); + 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 (BaseClass->getDestructor()->isVirtual()) + return; + + if (!DerivedClass->isDerivedFrom(BaseClass)) + return; + + if (!BT) + BT.reset(new BugType(this, + "Destruction of a polymorphic object with no " + "virtual destructor", + "Logic error")); + + ExplodedNode *N = C.generateNonFatalErrorNode(); + auto R = llvm::make_unique(*BT, BT->getName(), N); + + // Mark region of problematic base class for later use in the BugVisitor. + R->markInteresting(BaseClassRegion); + R->addVisitor(llvm::make_unique()); + C.emitReport(std::move(R)); +} + +std::shared_ptr +DeleteWithNonVirtualDtorChecker::DeleteBugVisitor::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(S); + if (!CastE) + return nullptr; + + // Only interested in DerivedToBase implicit casts. + // Explicit casts can have different CastKinds. + if (const auto *ImplCastE = dyn_cast(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 << "Conversion from derived to base happened here"; + PathDiagnosticLocation Pos(S, BRC.getSourceManager(), + N->getLocationContext()); + return std::make_shared(Pos, OS.str(), true, + nullptr); +} + +void ento::registerDeleteWithNonVirtualDtorChecker(CheckerManager &mgr) { + mgr.registerChecker(); +} Index: cfe/trunk/test/Analysis/DeleteWithNonVirtualDtor.cpp =================================================================== --- cfe/trunk/test/Analysis/DeleteWithNonVirtualDtor.cpp +++ cfe/trunk/test/Analysis/DeleteWithNonVirtualDtor.cpp @@ -0,0 +1,187 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.cplusplus.DeleteWithNonVirtualDtor -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 Base { + virtual void destroy() = 0; +}; + +class PrivateDtor final : public Base { +public: + void destroy() { delete this; } +private: + ~PrivateDtor() {} +}; + +struct ImplicitNV { + virtual void f(); +}; + +struct ImplicitNVDerived : public ImplicitNV {}; + +NVDerived *get(); + +NonVirtual *create() { + NonVirtual *x = new NVDerived(); // expected-note{{Conversion from derived to base happened here}} + return x; +} + +void sink(NonVirtual *x) { + delete x; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}} + // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}} +} + +void sinkCast(NonVirtual *y) { + delete reinterpret_cast(y); +} + +void sinkParamCast(NVDerived *z) { + delete z; +} + +void singleDerived() { + NonVirtual *sd; + sd = new NVDerived(); // expected-note{{Conversion from derived to base happened here}} + delete sd; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}} + // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}} +} + +void singleDerivedArr() { + NonVirtual *sda = new NVDerived[5]; // expected-note{{Conversion from derived to base happened here}} + delete[] sda; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}} + // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}} +} + +void doubleDerived() { + NonVirtual *dd = new NVDoubleDerived(); // expected-note{{Conversion from derived to base happened here}} + delete (dd); // expected-warning{{Destruction of a polymorphic object with no virtual destructor}} + // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}} +} + +void assignThroughFunction() { + NonVirtual *atf = get(); // expected-note{{Conversion from derived to base happened here}} + delete atf; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}} + // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}} +} + +void assignThroughFunction2() { + NonVirtual *atf2; + atf2 = get(); // expected-note{{Conversion from derived to base happened here}} + delete atf2; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}} + // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}} +} + +void createThroughFunction() { + NonVirtual *ctf = create(); // expected-note{{Calling 'create'}} + // expected-note@-1{{Returning from 'create'}} + delete ctf; // expected-warning {{Destruction of a polymorphic object with no virtual destructor}} + // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}} +} + +void deleteThroughFunction() { + NonVirtual *dtf = new NVDerived(); // expected-note{{Conversion from derived to base happened here}} + sink(dtf); // expected-note{{Calling 'sink'}} +} + +void singleCastCStyle() { + NVDerived *sccs = new NVDerived(); + NonVirtual *sccs2 = (NonVirtual*)sccs; // expected-note{{Conversion from derived to base happened here}} + delete sccs2; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}} + // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}} +} + +void doubleCastCStyle() { + NonVirtual *dccs = new NVDerived(); + NVDerived *dccs2 = (NVDerived*)dccs; + dccs = (NonVirtual*)dccs2; // expected-note{{Conversion from derived to base happened here}} + delete dccs; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}} + // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}} +} + +void singleCast() { + NVDerived *sc = new NVDerived(); + NonVirtual *sc2 = reinterpret_cast(sc); // expected-note{{Conversion from derived to base happened here}} + delete sc2; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}} + // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}} +} + +void doubleCast() { + NonVirtual *dd = new NVDerived(); + NVDerived *dd2 = reinterpret_cast(dd); + dd = reinterpret_cast(dd2); // expected-note {{Conversion from derived to base happened here}} + delete dd; // expected-warning {{Destruction of a polymorphic object with no virtual destructor}} + // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}} +} + +void implicitNV() { + ImplicitNV *invd = new ImplicitNVDerived(); // expected-note{{Conversion from derived to base happened here}} + delete invd; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}} + // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}} +} + +void doubleDecl() { + ImplicitNV *dd1, *dd2; + dd1 = new ImplicitNVDerived(); // expected-note{{Conversion from derived to base happened here}} + delete dd1; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}} + // expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}} +} + +void virtualBase() { + Virtual *vb = new VDerived(); + delete vb; // 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(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 +} + +// Deleting a polymorphic object with a non-virtual dtor +// is not a problem if it is referenced by its precise type. + +void preciseType() { + NVDerived *pt = new NVDerived(); + delete pt; // no-warning +} + +void privateDtor() { + Base *pd = new PrivateDtor(); + pd->destroy(); // no-warning +}