From 6931d311eaf483b35d5428ef6db15c0ba9e49840 Mon Sep 17 00:00:00 2001
From: isuckatcs <65320245+isuckatcs@users.noreply.github.com>
Date: Sat, 10 Sep 2022 19:04:28 +0200
Subject: [PATCH] [analyzer] Cleanup some artifacts from non-POD array
 evaluation

Most of the state traits used for non-POD array evaluation were
only cleaned up if the ctors/dtors were inlined, since the cleanup
happened in ExprEngine::processCallExit(). This patch makes sure
they are removed even if said functions are not inlined.

Differential Revision: https://reviews.llvm.org/D133643
---
 .../StaticAnalyzer/Core/PathSensitive/ExprEngine.h |  5 +++
 .../Core/ExprEngineCallAndReturn.cpp               | 45 +++++++++++++++-------
 2 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index aafeb5e..a905f90 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -970,6 +970,11 @@ private:
                                                const CXXConstructExpr *E,
                                                const LocationContext *LCtx);
 
+  static ProgramStateRef
+  removeStateTraitsUsedForArrayEvaluation(ProgramStateRef State,
+                                          const CXXConstructExpr *E,
+                                          const LocationContext *LCtx);
+
   /// Store the location of a C++ object corresponding to a statement
   /// until the statement is actually encountered. For example, if a DeclStmt
   /// has CXXConstructExpr as its initializer, the object would be considered
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 953b819..dbfded2 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -222,6 +222,26 @@ static unsigned getElementCountOfArrayBeingDestructed(
   return 0;
 }
 
+ProgramStateRef ExprEngine::removeStateTraitsUsedForArrayEvaluation(
+    ProgramStateRef State, const CXXConstructExpr *E,
+    const LocationContext *LCtx) {
+
+  assert(LCtx && "Location context must be provided!");
+
+  if (E) {
+    if (getPendingInitLoop(State, E, LCtx))
+      State = removePendingInitLoop(State, E, LCtx);
+
+    if (getIndexOfElementToConstruct(State, E, LCtx))
+      State = removeIndexOfElementToConstruct(State, E, LCtx);
+  }
+
+  if (getPendingArrayDestruction(State, LCtx))
+    State = removePendingArrayDestruction(State, LCtx);
+
+  return State;
+}
+
 /// The call exit is simulated with a sequence of nodes, which occur between
 /// CallExitBegin and CallExitEnd. The following operations occur between the
 /// two program points:
@@ -268,9 +288,6 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) {
 
       auto ThisVal = svalBuilder.getCXXThis(DtorDecl->getParent(), calleeCtx);
       state = state->killBinding(ThisVal);
-
-      if (!ShouldRepeatCall)
-        state = removePendingArrayDestruction(state, callerCtx);
     }
   }
 
@@ -304,14 +321,6 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) {
       state = state->BindExpr(CCE, callerCtx, ThisV);
 
       ShouldRepeatCall = shouldRepeatCtorCall(state, CCE, callerCtx);
-
-      if (!ShouldRepeatCall) {
-        if (getIndexOfElementToConstruct(state, CCE, callerCtx))
-          state = removeIndexOfElementToConstruct(state, CCE, callerCtx);
-
-        if (getPendingInitLoop(state, CCE, callerCtx))
-          state = removePendingInitLoop(state, CCE, callerCtx);
-      }
     }
 
     if (const auto *CNE = dyn_cast<CXXNewExpr>(CE)) {
@@ -330,6 +339,11 @@ void ExprEngine::processCallExit(ExplodedNode *CEBNode) {
     }
   }
 
+  if (!ShouldRepeatCall) {
+    state = removeStateTraitsUsedForArrayEvaluation(
+        state, dyn_cast_or_null<CXXConstructExpr>(CE), callerCtx);
+  }
+
   // Step 3: BindedRetNode -> CleanedNodes
   // If we can find a statement and a block in the inlined function, run remove
   // dead bindings before returning from the call. This is important to ensure
@@ -1151,7 +1165,7 @@ bool ExprEngine::shouldInlineArrayConstruction(const ProgramStateRef State,
 
   // Check if we're inside an ArrayInitLoopExpr, and it's sufficiently small.
   if (auto Size = getPendingInitLoop(State, CE, LCtx))
-    return *Size <= AMgr.options.maxBlockVisitOnPath;
+    return shouldInlineArrayDestruction(*Size);
 
   return false;
 }
@@ -1246,7 +1260,12 @@ void ExprEngine::defaultEvalCall(NodeBuilder &Bldr, ExplodedNode *Pred,
     }
   }
 
-  // If we can't inline it, handle the return value and invalidate the regions.
+  // If we can't inline it, clean up the state traits used only if the function
+  // is inlined.
+  State = removeStateTraitsUsedForArrayEvaluation(
+      State, dyn_cast_or_null<CXXConstructExpr>(E), Call->getLocationContext());
+
+  // Also handle the return value and invalidate the regions.
   conservativeEvalCall(*Call, Bldr, Pred, State);
 }
 
-- 
2.7.4