[clang] 2fb3bec - [analyzer] Fix crash for array-delete of UnknownVal values.

[Tomasz Kamiński via cfe-commits]

Author: Tomasz Kamiński
Date: 2022-11-09T15:06:46+01:00
New Revision: 2fb3bec932ede7704724abb71e8270e0b4c201f6

URL: https://github.com/llvm/llvm-project/commit/2fb3bec932ede7704724abb71e8270e0b4c201f6
DIFF: https://github.com/llvm/llvm-project/commit/2fb3bec932ede7704724abb71e8270e0b4c201f6.diff

LOG: [analyzer] Fix crash for array-delete of UnknownVal values.

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.

Reviewed By: isuckatcs

Differential Revision: https://reviews.llvm.org/D136671

Added: 
    

Modified: 
    clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp
    clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
    clang/test/Analysis/dtor-array.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp b/clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp
index db9698b4086e9..6a86536492cd3 100644
--- a/clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp
+++ b/clang/lib/StaticAnalyzer/Core/DynamicExtent.cpp
@@ -44,6 +44,7 @@ DefinedOrUnknownSVal getDynamicElementCount(ProgramStateRef State,
                                             const MemRegion *MR,
                                             SValBuilder &SVB,
                                             QualType ElementTy) {
+  assert(MR != nullptr && "Not-null region expected");
   MR = MR->StripCasts();
 
   DefinedOrUnknownSVal Size = getDynamicExtent(State, MR, SVB);

diff  --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index a90e17fc496d2..4528dbc0cc36f 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1241,6 +1241,7 @@ ExprEngine::prepareStateForArrayDestruction(const ProgramStateRef State,
                                             const QualType &ElementTy,
                                             const LocationContext *LCtx,
                                             SVal *ElementCountVal) {
+  assert(Region != nullptr && "Not-null region expected");	
 
   QualType Ty = ElementTy.getDesugaredType(getContext());
   while (const auto *NTy = dyn_cast<ArrayType>(Ty))
@@ -1420,31 +1421,32 @@ void ExprEngine::ProcessDeleteDtor(const CFGDeleteDtor Dtor,
   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());

diff  --git a/clang/test/Analysis/dtor-array.cpp b/clang/test/Analysis/dtor-array.cpp
index ab0a939232173..84a34af922516 100644
--- a/clang/test/Analysis/dtor-array.cpp
+++ b/clang/test/Analysis/dtor-array.cpp
@@ -344,3 +344,36 @@ void nonConstantRegionExtent(){
   // 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
+}
+
+}