[StaticAnalyzer] LoopUnrolling fixes

1. The LoopUnrolling feature needs the LoopExit included in the CFG so added this
dependency via the config options
2. The LoopExit element can be encountered even if we haven't encountered the 
block of the corresponding LoopStmt. So the asserts were not right.
3. If we are caching out the Node then we get a nullptr from generateNode which
case was not handled.


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

llvm-svn: 311880

clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp

@@ -27,7 +27,9 @@ AnalysisManager::AnalysisManager(ASTContext &ctx, DiagnosticsEngine &diags,
               /*AddInitializers=*/true,
               Options.includeTemporaryDtorsInCFG(),
 	            Options.includeLifetimeInCFG(),
-              Options.includeLoopExitInCFG(),
+              // Adding LoopExit elements to the CFG is a requirement for loop
+              // unrolling.
+              Options.includeLoopExitInCFG() || Options.shouldUnrollLoops(),
               Options.shouldSynthesizeBodies(),
               Options.shouldConditionalizeStaticInitializers(),
               /*addCXXNewAllocator=*/true,

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

@@ -1527,8 +1527,11 @@ void ExprEngine::processCFGBlockEntrance(const BlockEdge &L,
     if (Term) {
       ProgramStateRef NewState = updateLoopStack(Term, AMgr.getASTContext(),
                                                  Pred);
-      if (NewState != Pred->getState()){
-        Pred = nodeBuilder.generateNode(NewState, Pred);
+      if (NewState != Pred->getState()) {
+        ExplodedNode *UpdatedNode = nodeBuilder.generateNode(NewState, Pred);
+        if (!UpdatedNode)
+          return;
+        Pred = UpdatedNode;
       }
     }
     // Is we are inside an unrolled loop then no need the check the counters.

clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp

@@ -13,15 +13,11 @@
 ///
 //===----------------------------------------------------------------------===//
 
-#include "clang/Analysis/CFGStmtMap.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/AST/ParentMap.h"
-#include "clang/AST/StmtVisitor.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/LoopUnrolling.h"
-#include "llvm/ADT/Statistic.h"
 
 using namespace clang;
 using namespace ento;
@@ -72,11 +68,8 @@ static bool isLoopStmt(const Stmt *S) {
 
 ProgramStateRef processLoopEnd(const Stmt *LoopStmt, ProgramStateRef State) {
   auto LS = State->get<LoopStack>();
-  assert(!LS.isEmpty() && "Loop not added to the stack.");
-  assert(LoopStmt == LS.getHead().getLoopStmt() &&
-         "Loop is not on top of the stack.");
-
-  State = State->set<LoopStack>(LS.getTail());
+  if (!LS.isEmpty() && LS.getHead().getLoopStmt() == LoopStmt)
+    State = State->set<LoopStack>(LS.getTail());
   return State;
 }
 

clang/test/Analysis/loop-unrolling.cpp

@@ -272,3 +272,10 @@ int recursion_unroll4(bool b) {
   int a = 22 / k;
   return 0;
 }
+
+int loop_exit_while_empty_loop_stack() {
+  if (getNum())
+    for (int i = 1; i < 8; i++)
+      ;
+  return 0;
+}