NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet.
Herald added subscribers: cfe-commits, rnkovacs, baloghadamsoftware.
When operator `delete[]` receives a sub-expression of array type, it destroys
the array correctly. Even if it's multi-dimensional, simply because in this
case it's an array of array-type objects and destroying such array would mean
properly destroying each object, where the object itself is an array, so
properly destroying it means destroying its objects, etc.
In this case the AST is saying that the destroyed type is an array type - and
we weren't prepared for that.
For now there's no actual calling multiple destructors; at least we're trying
not to crash.
Repository:
rC Clang
https://reviews.llvm.org/D46146
Files:
lib/StaticAnalyzer/Core/ExprEngine.cpp
test/Analysis/new.cpp
Index: test/Analysis/new.cpp
===================================================================
--- test/Analysis/new.cpp
+++ test/Analysis/new.cpp
@@ -274,6 +274,24 @@
clang_analyzer_eval(true); // expected-warning{{TRUE}}
}
+void test_array_delete() {
+ class C {
+ public:
+ ~C() {}
+ };
+
+ auto c1 = new C[2][3];
+ delete[] c1; // no-crash // no-warning
+
+ C c2[4];
+ // FIXME: Should warn.
+ delete[] &c2; // no-crash
+
+ C c3[7][6];
+ // FIXME: Should warn.
+ delete[] &c3; // no-crash
+}
+
void testDeleteNull() {
NoReturnDtor *foo = 0;
delete foo; // should not call destructor, checked below
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1086,12 +1086,14 @@
// This workaround will just run the first destructor (which will still
// invalidate the entire array).
CallOpts.IsArrayCtorOrDtor = true;
+ // Yes, it may even be a multi-dimensional array.
+ while (const auto *AT = getContext().getAsArrayType(DTy))
+ DTy = AT->getElementType();
if (ArgR)
ArgR = getStoreManager().GetElementZeroRegion(cast<SubRegion>(ArgR),
DTy);
}
- VisitCXXDestructor(DE->getDestroyedType(), ArgR, DE, /*IsBase=*/false,
- Pred, Dst, CallOpts);
+ VisitCXXDestructor(DTy, ArgR, DE, /*IsBase=*/false, Pred, Dst, CallOpts);
}
void ExprEngine::ProcessBaseDtor(const CFGBaseDtor D,
Index: test/Analysis/new.cpp
===================================================================
--- test/Analysis/new.cpp
+++ test/Analysis/new.cpp
@@ -274,6 +274,24 @@
clang_analyzer_eval(true); // expected-warning{{TRUE}}
}
+void test_array_delete() {
+ class C {
+ public:
+ ~C() {}
+ };
+
+ auto c1 = new C[2][3];
+ delete[] c1; // no-crash // no-warning
+
+ C c2[4];
+ // FIXME: Should warn.
+ delete[] &c2; // no-crash
+
+ C c3[7][6];
+ // FIXME: Should warn.
+ delete[] &c3; // no-crash
+}
+
void testDeleteNull() {
NoReturnDtor *foo = 0;
delete foo; // should not call destructor, checked below
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1086,12 +1086,14 @@
// This workaround will just run the first destructor (which will still
// invalidate the entire array).
CallOpts.IsArrayCtorOrDtor = true;
+ // Yes, it may even be a multi-dimensional array.
+ while (const auto *AT = getContext().getAsArrayType(DTy))
+ DTy = AT->getElementType();
if (ArgR)
ArgR = getStoreManager().GetElementZeroRegion(cast<SubRegion>(ArgR), DTy);
}
- VisitCXXDestructor(DE->getDestroyedType(), ArgR, DE, /*IsBase=*/false,
- Pred, Dst, CallOpts);
+ VisitCXXDestructor(DTy, ArgR, DE, /*IsBase=*/false, Pred, Dst, CallOpts);
}
void ExprEngine::ProcessBaseDtor(const CFGBaseDtor D,
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
