Skip to content

Commit 61e7ade

Browse files
Xazax-hunXazax-hun
committedSep 22, 2017
[analyzer] Add new delete with non-virtual destructor check
Patch by: Reka Nikolett Kovacs Differential Revision: https://reviews.llvm.org/D35796 llvm-svn: 313973
1 parent 2b1c3bb commit 61e7ade

File tree

4 files changed

+346
-0
lines changed

4 files changed

+346
-0
lines changed
 

‎clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

+5
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,11 @@ def VirtualCallChecker : Checker<"VirtualCall">,
284284

285285
let ParentPackage = CplusplusAlpha in {
286286

287+
def DeleteWithNonVirtualDtorChecker : Checker<"DeleteWithNonVirtualDtor">,
288+
HelpText<"Reports destructions of polymorphic objects with a non-virtual "
289+
"destructor in their base class">,
290+
DescFile<"DeleteWithNonVirtualDtorChecker.cpp">;
291+
287292
def IteratorRangeChecker : Checker<"IteratorRange">,
288293
HelpText<"Check for iterators used outside their valid ranges">,
289294
DescFile<"IteratorChecker.cpp">;

‎clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt

+1
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ add_clang_library(clangStaticAnalyzerCheckers
2929
CXXSelfAssignmentChecker.cpp
3030
DeadStoresChecker.cpp
3131
DebugCheckers.cpp
32+
DeleteWithNonVirtualDtorChecker.cpp
3233
DereferenceChecker.cpp
3334
DirectIvarAssignment.cpp
3435
DivZeroChecker.cpp

‎clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp

+153
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
//===-- DeleteWithNonVirtualDtorChecker.cpp -----------------------*- C++ -*--//
2+
//
3+
// The LLVM Compiler Infrastructure
4+
//
5+
// This file is distributed under the University of Illinois Open Source
6+
// License. See LICENSE.TXT for details.
7+
//
8+
//===----------------------------------------------------------------------===//
9+
//
10+
// Defines a checker for the OOP52-CPP CERT rule: Do not delete a polymorphic
11+
// object without a virtual destructor.
12+
//
13+
// Diagnostic flags -Wnon-virtual-dtor and -Wdelete-non-virtual-dtor report if
14+
// an object with a virtual function but a non-virtual destructor exists or is
15+
// deleted, respectively.
16+
//
17+
// This check exceeds them by comparing the dynamic and static types of the
18+
// object at the point of destruction and only warns if it happens through a
19+
// pointer to a base type without a virtual destructor. The check places a note
20+
// at the last point where the conversion from derived to base happened.
21+
//
22+
//===----------------------------------------------------------------------===//
23+
24+
#include "ClangSACheckers.h"
25+
#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
26+
#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
27+
#include "clang/StaticAnalyzer/Core/Checker.h"
28+
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
29+
#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
30+
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
31+
#include "clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h"
32+
#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
33+
34+
using namespace clang;
35+
using namespace ento;
36+
37+
namespace {
38+
class DeleteWithNonVirtualDtorChecker
39+
: public Checker<check::PreStmt<CXXDeleteExpr>> {
40+
mutable std::unique_ptr<BugType> BT;
41+
42+
class DeleteBugVisitor : public BugReporterVisitorImpl<DeleteBugVisitor> {
43+
public:
44+
DeleteBugVisitor() : Satisfied(false) {}
45+
void Profile(llvm::FoldingSetNodeID &ID) const override {
46+
static int X = 0;
47+
ID.AddPointer(&X);
48+
}
49+
std::shared_ptr<PathDiagnosticPiece> VisitNode(const ExplodedNode *N,
50+
const ExplodedNode *PrevN,
51+
BugReporterContext &BRC,
52+
BugReport &BR) override;
53+
54+
private:
55+
bool Satisfied;
56+
};
57+
58+
public:
59+
void checkPreStmt(const CXXDeleteExpr *DE, CheckerContext &C) const;
60+
};
61+
} // end anonymous namespace
62+
63+
void DeleteWithNonVirtualDtorChecker::checkPreStmt(const CXXDeleteExpr *DE,
64+
CheckerContext &C) const {
65+
const Expr *DeletedObj = DE->getArgument();
66+
const MemRegion *MR = C.getSVal(DeletedObj).getAsRegion();
67+
if (!MR)
68+
return;
69+
70+
const auto *BaseClassRegion = MR->getAs<TypedValueRegion>();
71+
const auto *DerivedClassRegion = MR->getBaseRegion()->getAs<SymbolicRegion>();
72+
if (!BaseClassRegion || !DerivedClassRegion)
73+
return;
74+
75+
const auto *BaseClass = BaseClassRegion->getValueType()->getAsCXXRecordDecl();
76+
const auto *DerivedClass =
77+
DerivedClassRegion->getSymbol()->getType()->getPointeeCXXRecordDecl();
78+
if (!BaseClass || !DerivedClass)
79+
return;
80+
81+
if (!BaseClass->hasDefinition() || !DerivedClass->hasDefinition())
82+
return;
83+
84+
if (BaseClass->getDestructor()->isVirtual())
85+
return;
86+
87+
if (!DerivedClass->isDerivedFrom(BaseClass))
88+
return;
89+
90+
if (!BT)
91+
BT.reset(new BugType(this,
92+
"Destruction of a polymorphic object with no "
93+
"virtual destructor",
94+
"Logic error"));
95+
96+
ExplodedNode *N = C.generateNonFatalErrorNode();
97+
auto R = llvm::make_unique<BugReport>(*BT, BT->getName(), N);
98+
99+
// Mark region of problematic base class for later use in the BugVisitor.
100+
R->markInteresting(BaseClassRegion);
101+
R->addVisitor(llvm::make_unique<DeleteBugVisitor>());
102+
C.emitReport(std::move(R));
103+
}
104+
105+
std::shared_ptr<PathDiagnosticPiece>
106+
DeleteWithNonVirtualDtorChecker::DeleteBugVisitor::VisitNode(
107+
const ExplodedNode *N, const ExplodedNode *PrevN, BugReporterContext &BRC,
108+
BugReport &BR) {
109+
// Stop traversal after the first conversion was found on a path.
110+
if (Satisfied)
111+
return nullptr;
112+
113+
ProgramStateRef State = N->getState();
114+
const LocationContext *LC = N->getLocationContext();
115+
const Stmt *S = PathDiagnosticLocation::getStmt(N);
116+
if (!S)
117+
return nullptr;
118+
119+
const auto *CastE = dyn_cast<CastExpr>(S);
120+
if (!CastE)
121+
return nullptr;
122+
123+
// Only interested in DerivedToBase implicit casts.
124+
// Explicit casts can have different CastKinds.
125+
if (const auto *ImplCastE = dyn_cast<ImplicitCastExpr>(CastE)) {
126+
if (ImplCastE->getCastKind() != CK_DerivedToBase)
127+
return nullptr;
128+
}
129+
130+
// Region associated with the current cast expression.
131+
const MemRegion *M = State->getSVal(CastE, LC).getAsRegion();
132+
if (!M)
133+
return nullptr;
134+
135+
// Check if target region was marked as problematic previously.
136+
if (!BR.isInteresting(M))
137+
return nullptr;
138+
139+
// Stop traversal on this path.
140+
Satisfied = true;
141+
142+
SmallString<256> Buf;
143+
llvm::raw_svector_ostream OS(Buf);
144+
OS << "Conversion from derived to base happened here";
145+
PathDiagnosticLocation Pos(S, BRC.getSourceManager(),
146+
N->getLocationContext());
147+
return std::make_shared<PathDiagnosticEventPiece>(Pos, OS.str(), true,
148+
nullptr);
149+
}
150+
151+
void ento::registerDeleteWithNonVirtualDtorChecker(CheckerManager &mgr) {
152+
mgr.registerChecker<DeleteWithNonVirtualDtorChecker>();
153+
}

‎clang/test/Analysis/DeleteWithNonVirtualDtor.cpp

+187
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.cplusplus.DeleteWithNonVirtualDtor -std=c++11 -verify -analyzer-output=text %s
2+
3+
struct Virtual {
4+
virtual ~Virtual() {}
5+
};
6+
7+
struct VDerived : public Virtual {};
8+
9+
struct NonVirtual {
10+
~NonVirtual() {}
11+
};
12+
13+
struct NVDerived : public NonVirtual {};
14+
struct NVDoubleDerived : public NVDerived {};
15+
16+
struct Base {
17+
virtual void destroy() = 0;
18+
};
19+
20+
class PrivateDtor final : public Base {
21+
public:
22+
void destroy() { delete this; }
23+
private:
24+
~PrivateDtor() {}
25+
};
26+
27+
struct ImplicitNV {
28+
virtual void f();
29+
};
30+
31+
struct ImplicitNVDerived : public ImplicitNV {};
32+
33+
NVDerived *get();
34+
35+
NonVirtual *create() {
36+
NonVirtual *x = new NVDerived(); // expected-note{{Conversion from derived to base happened here}}
37+
return x;
38+
}
39+
40+
void sink(NonVirtual *x) {
41+
delete x; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
42+
// expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
43+
}
44+
45+
void sinkCast(NonVirtual *y) {
46+
delete reinterpret_cast<NVDerived*>(y);
47+
}
48+
49+
void sinkParamCast(NVDerived *z) {
50+
delete z;
51+
}
52+
53+
void singleDerived() {
54+
NonVirtual *sd;
55+
sd = new NVDerived(); // expected-note{{Conversion from derived to base happened here}}
56+
delete sd; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
57+
// expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
58+
}
59+
60+
void singleDerivedArr() {
61+
NonVirtual *sda = new NVDerived[5]; // expected-note{{Conversion from derived to base happened here}}
62+
delete[] sda; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
63+
// expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
64+
}
65+
66+
void doubleDerived() {
67+
NonVirtual *dd = new NVDoubleDerived(); // expected-note{{Conversion from derived to base happened here}}
68+
delete (dd); // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
69+
// expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
70+
}
71+
72+
void assignThroughFunction() {
73+
NonVirtual *atf = get(); // expected-note{{Conversion from derived to base happened here}}
74+
delete atf; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
75+
// expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
76+
}
77+
78+
void assignThroughFunction2() {
79+
NonVirtual *atf2;
80+
atf2 = get(); // expected-note{{Conversion from derived to base happened here}}
81+
delete atf2; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
82+
// expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
83+
}
84+
85+
void createThroughFunction() {
86+
NonVirtual *ctf = create(); // expected-note{{Calling 'create'}}
87+
// expected-note@-1{{Returning from 'create'}}
88+
delete ctf; // expected-warning {{Destruction of a polymorphic object with no virtual destructor}}
89+
// expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
90+
}
91+
92+
void deleteThroughFunction() {
93+
NonVirtual *dtf = new NVDerived(); // expected-note{{Conversion from derived to base happened here}}
94+
sink(dtf); // expected-note{{Calling 'sink'}}
95+
}
96+
97+
void singleCastCStyle() {
98+
NVDerived *sccs = new NVDerived();
99+
NonVirtual *sccs2 = (NonVirtual*)sccs; // expected-note{{Conversion from derived to base happened here}}
100+
delete sccs2; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
101+
// expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
102+
}
103+
104+
void doubleCastCStyle() {
105+
NonVirtual *dccs = new NVDerived();
106+
NVDerived *dccs2 = (NVDerived*)dccs;
107+
dccs = (NonVirtual*)dccs2; // expected-note{{Conversion from derived to base happened here}}
108+
delete dccs; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
109+
// expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
110+
}
111+
112+
void singleCast() {
113+
NVDerived *sc = new NVDerived();
114+
NonVirtual *sc2 = reinterpret_cast<NonVirtual*>(sc); // expected-note{{Conversion from derived to base happened here}}
115+
delete sc2; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
116+
// expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
117+
}
118+
119+
void doubleCast() {
120+
NonVirtual *dd = new NVDerived();
121+
NVDerived *dd2 = reinterpret_cast<NVDerived*>(dd);
122+
dd = reinterpret_cast<NonVirtual*>(dd2); // expected-note {{Conversion from derived to base happened here}}
123+
delete dd; // expected-warning {{Destruction of a polymorphic object with no virtual destructor}}
124+
// expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
125+
}
126+
127+
void implicitNV() {
128+
ImplicitNV *invd = new ImplicitNVDerived(); // expected-note{{Conversion from derived to base happened here}}
129+
delete invd; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
130+
// expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
131+
}
132+
133+
void doubleDecl() {
134+
ImplicitNV *dd1, *dd2;
135+
dd1 = new ImplicitNVDerived(); // expected-note{{Conversion from derived to base happened here}}
136+
delete dd1; // expected-warning{{Destruction of a polymorphic object with no virtual destructor}}
137+
// expected-note@-1{{Destruction of a polymorphic object with no virtual destructor}}
138+
}
139+
140+
void virtualBase() {
141+
Virtual *vb = new VDerived();
142+
delete vb; // no-warning
143+
}
144+
145+
void notDerived() {
146+
NonVirtual *nd = new NonVirtual();
147+
delete nd; // no-warning
148+
}
149+
150+
void notDerivedArr() {
151+
NonVirtual *nda = new NonVirtual[3];
152+
delete[] nda; // no-warning
153+
}
154+
155+
void cast() {
156+
NonVirtual *c = new NVDerived();
157+
delete reinterpret_cast<NVDerived*>(c); // no-warning
158+
}
159+
160+
void deleteThroughFunction2() {
161+
NonVirtual *dtf2 = new NVDerived();
162+
sinkCast(dtf2); // no-warning
163+
}
164+
165+
void deleteThroughFunction3() {
166+
NVDerived *dtf3;
167+
dtf3 = new NVDerived();
168+
sinkParamCast(dtf3); // no-warning
169+
}
170+
171+
void stackVar() {
172+
NonVirtual sv2;
173+
delete &sv2; // no-warning
174+
}
175+
176+
// Deleting a polymorphic object with a non-virtual dtor
177+
// is not a problem if it is referenced by its precise type.
178+
179+
void preciseType() {
180+
NVDerived *pt = new NVDerived();
181+
delete pt; // no-warning
182+
}
183+
184+
void privateDtor() {
185+
Base *pd = new PrivateDtor();
186+
pd->destroy(); // no-warning
187+
}

0 commit comments

Comments
 (0)
Please sign in to comment.