tomasz-kaminski-sonarsource created this revision.
Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, dkrupp,
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware,
xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
tomasz-kaminski-sonarsource requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
We now skip the destruction of array elements for `delete[] p`,
if the value of `p` is UnknownVal and does not have corresponding region.
This eliminate the crash in `getDynamicElementCount` on that
region and matches the behavior for deleting the array of
non-constant range.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D136671
Files:
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
clang/test/Analysis/dtor-array.cpp
Index: clang/test/Analysis/dtor-array.cpp
===================================================================
--- clang/test/Analysis/dtor-array.cpp
+++ clang/test/Analysis/dtor-array.cpp
@@ -344,3 +344,36 @@
// region to a conjured symbol.
clang_analyzer_eval(InlineDtor::dtorCalled == 0); // expected-warning
{{TRUE}} expected-warning {{FALSE}}
}
+
+namespace crash6 {
+
+struct NonTrivialItem {
+ ~NonTrivialItem();
+};
+
+struct WeirdVec {
+ void clear() {
+ delete[] data;
+ size = 0;
+ }
+ NonTrivialItem *data;
+ unsigned size;
+};
+
+void top(int j) {
+ WeirdVec *p = new WeirdVec;
+
+ p[j].size = 0;
+ delete[] p->data; // no-crash
+}
+
+template <typename T>
+T make_unknown() {
+ return reinterpret_cast<T>(static_cast<int>(0.404));
+}
+
+void directUnknownSymbol() {
+ delete[] make_unknown<NonTrivialItem*>(); // no-crash
+}
+
+}
\ No newline at end of file
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1420,31 +1420,32 @@
const MemRegion *ArgR = ArgVal.getAsRegion();
if (DE->isArrayForm()) {
- SVal ElementCount;
- std::tie(State, Idx) =
- prepareStateForArrayDestruction(State, ArgR, DTy, LCtx, &ElementCount);
-
CallOpts.IsArrayCtorOrDtor = true;
// Yes, it may even be a multi-dimensional array.
while (const auto *AT = getContext().getAsArrayType(DTy))
DTy = AT->getElementType();
- // If we're about to destruct a 0 length array, don't run any of the
- // destructors.
- if (ElementCount.isConstant() &&
- ElementCount.getAsInteger()->getLimitedValue() == 0) {
+ if (ArgR) {
+ SVal ElementCount;
+ std::tie(State, Idx) = prepareStateForArrayDestruction(
+ State, ArgR, DTy, LCtx, &ElementCount);
- static SimpleProgramPointTag PT(
- "ExprEngine", "Skipping 0 length array delete destruction");
- PostImplicitCall PP(getDtorDecl(DTy), DE->getBeginLoc(), LCtx, &PT);
- NodeBuilder Bldr(Pred, Dst, *currBldrCtx);
- Bldr.generateNode(PP, Pred->getState(), Pred);
- return;
- }
+ // If we're about to destruct a 0 length array, don't run any of the
+ // destructors.
+ if (ElementCount.isConstant() &&
+ ElementCount.getAsInteger()->getLimitedValue() == 0) {
+
+ static SimpleProgramPointTag PT(
+ "ExprEngine", "Skipping 0 length array delete destruction");
+ PostImplicitCall PP(getDtorDecl(DTy), DE->getBeginLoc(), LCtx, &PT);
+ NodeBuilder Bldr(Pred, Dst, *currBldrCtx);
+ Bldr.generateNode(PP, Pred->getState(), Pred);
+ return;
+ }
- if (ArgR)
ArgR = State->getLValue(DTy, svalBuilder.makeArrayIndex(Idx), ArgVal)
.getAsRegion();
+ }
}
NodeBuilder Bldr(Pred, Dst, getBuilderContext());
Index: clang/test/Analysis/dtor-array.cpp
===================================================================
--- clang/test/Analysis/dtor-array.cpp
+++ clang/test/Analysis/dtor-array.cpp
@@ -344,3 +344,36 @@
// region to a conjured symbol.
clang_analyzer_eval(InlineDtor::dtorCalled == 0); // expected-warning {{TRUE}} expected-warning {{FALSE}}
}
+
+namespace crash6 {
+
+struct NonTrivialItem {
+ ~NonTrivialItem();
+};
+
+struct WeirdVec {
+ void clear() {
+ delete[] data;
+ size = 0;
+ }
+ NonTrivialItem *data;
+ unsigned size;
+};
+
+void top(int j) {
+ WeirdVec *p = new WeirdVec;
+
+ p[j].size = 0;
+ delete[] p->data; // no-crash
+}
+
+template <typename T>
+T make_unknown() {
+ return reinterpret_cast<T>(static_cast<int>(0.404));
+}
+
+void directUnknownSymbol() {
+ delete[] make_unknown<NonTrivialItem*>(); // no-crash
+}
+
+}
\ No newline at end of file
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1420,31 +1420,32 @@
const MemRegion *ArgR = ArgVal.getAsRegion();
if (DE->isArrayForm()) {
- SVal ElementCount;
- std::tie(State, Idx) =
- prepareStateForArrayDestruction(State, ArgR, DTy, LCtx, &ElementCount);
-
CallOpts.IsArrayCtorOrDtor = true;
// Yes, it may even be a multi-dimensional array.
while (const auto *AT = getContext().getAsArrayType(DTy))
DTy = AT->getElementType();
- // If we're about to destruct a 0 length array, don't run any of the
- // destructors.
- if (ElementCount.isConstant() &&
- ElementCount.getAsInteger()->getLimitedValue() == 0) {
+ if (ArgR) {
+ SVal ElementCount;
+ std::tie(State, Idx) = prepareStateForArrayDestruction(
+ State, ArgR, DTy, LCtx, &ElementCount);
- static SimpleProgramPointTag PT(
- "ExprEngine", "Skipping 0 length array delete destruction");
- PostImplicitCall PP(getDtorDecl(DTy), DE->getBeginLoc(), LCtx, &PT);
- NodeBuilder Bldr(Pred, Dst, *currBldrCtx);
- Bldr.generateNode(PP, Pred->getState(), Pred);
- return;
- }
+ // If we're about to destruct a 0 length array, don't run any of the
+ // destructors.
+ if (ElementCount.isConstant() &&
+ ElementCount.getAsInteger()->getLimitedValue() == 0) {
+
+ static SimpleProgramPointTag PT(
+ "ExprEngine", "Skipping 0 length array delete destruction");
+ PostImplicitCall PP(getDtorDecl(DTy), DE->getBeginLoc(), LCtx, &PT);
+ NodeBuilder Bldr(Pred, Dst, *currBldrCtx);
+ Bldr.generateNode(PP, Pred->getState(), Pred);
+ return;
+ }
- if (ArgR)
ArgR = State->getLValue(DTy, svalBuilder.makeArrayIndex(Idx), ArgVal)
.getAsRegion();
+ }
}
NodeBuilder Bldr(Pred, Dst, getBuilderContext());
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
