Fix a few bugs

lib/Core/ExecutionState.cpp

@@ -120,9 +120,8 @@ ExecutionState::ExecutionState()
     : initPC(nullptr), pc(nullptr), prevPC(nullptr), incomingBBIndex(-1),
       depth(0), ptreeNode(nullptr), symbolics(), steppedInstructions(0),
       steppedMemoryInstructions(0), instsSinceCovNew(0),
-      roundingMode(llvm::APFloat::rmNearestTiesToEven),
-      coveredNew(new box<bool>(false)), forkDisabled(false),
-      prevHistory_(TargetsHistory::create()),
+      roundingMode(llvm::APFloat::rmNearestTiesToEven), coveredNew({}),
+      forkDisabled(false), prevHistory_(TargetsHistory::create()),
       history_(TargetsHistory::create()) {
   setID();
 }
@@ -131,9 +130,8 @@ ExecutionState::ExecutionState(KFunction *kf)
     : initPC(kf->instructions), pc(initPC), prevPC(pc), incomingBBIndex(-1),
       depth(0), ptreeNode(nullptr), symbolics(), steppedInstructions(0),
       steppedMemoryInstructions(0), instsSinceCovNew(0),
-      roundingMode(llvm::APFloat::rmNearestTiesToEven),
-      coveredNew(new box<bool>(false)), forkDisabled(false),
-      prevHistory_(TargetsHistory::create()),
+      roundingMode(llvm::APFloat::rmNearestTiesToEven), coveredNew({}),
+      forkDisabled(false), prevHistory_(TargetsHistory::create()),
       history_(TargetsHistory::create()) {
   pushFrame(nullptr, kf);
   setID();
@@ -143,9 +141,8 @@ ExecutionState::ExecutionState(KFunction *kf, KBlock *kb)
     : initPC(kb->instructions), pc(initPC), prevPC(pc), incomingBBIndex(-1),
       depth(0), ptreeNode(nullptr), symbolics(), steppedInstructions(0),
       steppedMemoryInstructions(0), instsSinceCovNew(0),
-      roundingMode(llvm::APFloat::rmNearestTiesToEven),
-      coveredNew(new box<bool>(false)), forkDisabled(false),
-      prevHistory_(TargetsHistory::create()),
+      roundingMode(llvm::APFloat::rmNearestTiesToEven), coveredNew({}),
+      forkDisabled(false), prevHistory_(TargetsHistory::create()),
       history_(TargetsHistory::create()) {
   pushFrame(nullptr, kf);
   setID();

lib/Core/ExecutionState.h

@@ -36,6 +36,7 @@ DISABLE_WARNING_DEPRECATED_DECLARATIONS
 #include "llvm/IR/Function.h"
 DISABLE_WARNING_POP
 
+#include <deque>
 #include <map>
 #include <memory>
 #include <set>
@@ -364,7 +365,7 @@ class ExecutionState {
   std::uint32_t id = 0;
 
   /// @brief Whether a new instruction was covered in this state
-  mutable ref<box<bool>> coveredNew;
+  mutable std::deque<ref<box<bool>>> coveredNew;
 
   /// @brief Disables forking for this state. Set by user code
   bool forkDisabled = false;
@@ -502,6 +503,22 @@ class ExecutionState {
     }
     return false;
   }
+
+  bool isCoveredNew() const {
+    return !coveredNew.empty() && coveredNew.back()->value;
+  }
+  void coverNew() const { coveredNew.push_back(new box<bool>(true)); }
+  void updateCoveredNew() const {
+    while (!coveredNew.empty() && !coveredNew.front()->value) {
+      coveredNew.pop_front();
+    }
+  }
+  void clearCoveredNew() const {
+    for (auto signal : coveredNew) {
+      signal->value = false;
+    }
+    coveredNew.clear();
+  }
 };
 
 struct ExecutionStateIDCompare {

lib/Core/Executor.cpp

@@ -3953,6 +3953,10 @@ void Executor::updateStates(ExecutionState *current) {
     targetedExecutionManager->update(current, addedStates, removedStates);
   }
 
+  if (targetCalculator) {
+    targetCalculator->update(current, addedStates, removedStates);
+  }
+
   if (searcher) {
     searcher->update(current, addedStates, removedStates);
   }
@@ -4084,7 +4088,7 @@ bool Executor::checkMemoryUsage() {
     unsigned idx = theRNG.getInt32() % N;
     // Make two pulls to try and not hit a state that
     // covered new code.
-    if (arr[idx]->coveredNew->value)
+    if (arr[idx]->isCoveredNew())
       idx = theRNG.getInt32() % N;
 
     std::swap(arr[idx], arr[N - 1]);
@@ -4294,11 +4298,6 @@ void Executor::run(std::vector<ExecutionState *> initialStates) {
       KInstruction *prevKI = state.prevPC;
       KFunction *kf = prevKI->parent->parent;
 
-      if (prevKI->inst->isTerminator() &&
-          kmodule->inMainModule(*kf->function)) {
-        targetCalculator->update(state);
-      }
-
       executeStep(state);
     }
 
@@ -4357,8 +4356,8 @@ void Executor::initializeTypeManager() {
 }
 
 static bool shouldWriteTest(const ExecutionState &state) {
-  bool coveredNew = state.coveredNew->value;
-  state.coveredNew->value = false;
+  state.updateCoveredNew();
+  bool coveredNew = state.isCoveredNew();
   return !OnlyOutputStatesCoveringNew || coveredNew;
 }
 
@@ -4380,6 +4379,7 @@ void Executor::executeStep(ExecutionState &state) {
 
   if (CoverOnTheFly && guidanceKind != GuidanceKind::ErrorGuidance &&
       stats::instructions > DelayCoverOnTheFly && shouldWriteTest(state)) {
+    state.clearCoveredNew();
     interpreterHandler->processTestCase(
         state, nullptr,
         terminationTypeFileExtension(StateTerminationType::CoverOnTheFly)
@@ -4565,7 +4565,6 @@ void Executor::terminateState(ExecutionState &state,
 
   interpreterHandler->incPathsExplored();
   state.pc = state.prevPC;
-  targetCalculator->update(state);
   solver->notifyStateTermination(state.id);
 
   removedStates.push_back(&state);
@@ -4574,9 +4573,11 @@ void Executor::terminateState(ExecutionState &state,
 void Executor::terminateStateOnExit(ExecutionState &state) {
   auto terminationType = StateTerminationType::Exit;
   ++stats::terminationExit;
-  if (shouldWriteTest(state) || (AlwaysOutputSeeds && seedMap.count(&state)))
+  if (shouldWriteTest(state) || (AlwaysOutputSeeds && seedMap.count(&state))) {
+    state.clearCoveredNew();
     interpreterHandler->processTestCase(
         state, nullptr, terminationTypeFileExtension(terminationType).c_str());
+  }
 
   interpreterHandler->incPathsCompleted();
   terminateState(state, terminationType);
@@ -4593,6 +4594,7 @@ void Executor::terminateStateEarly(ExecutionState &state, const Twine &message,
         reason == StateTerminationType::MissedAllTargets) &&
        shouldWriteTest(state)) ||
       (AlwaysOutputSeeds && seedMap.count(&state))) {
+    state.clearCoveredNew();
     interpreterHandler->processTestCase(
         state, (message + "\n").str().c_str(),
         terminationTypeFileExtension(reason).c_str(),
@@ -4717,8 +4719,9 @@ void Executor::terminateStateOnError(ExecutionState &state,
   const KInstruction *ki = getLastNonKleeInternalInstruction(state);
   Instruction *lastInst = ki->inst;
 
-  if (EmitAllErrors ||
-      emittedErrors.insert(std::make_pair(lastInst, message)).second) {
+  if ((EmitAllErrors ||
+       emittedErrors.insert(std::make_pair(lastInst, message)).second) &&
+      shouldWriteTest(state)) {
     std::string filepath = ki->getSourceFilepath();
     if (!filepath.empty()) {
       klee_message("ERROR: %s:%zu: %s", filepath.c_str(), ki->getLine(),
@@ -7375,7 +7378,7 @@ void Executor::dumpStates() {
       *os << "{";
       *os << "'depth' : " << es->depth << ", ";
       *os << "'queryCost' : " << es->queryMetaData.queryCost << ", ";
-      *os << "'coveredNew' : " << es->coveredNew->value << ", ";
+      *os << "'coveredNew' : " << es->isCoveredNew() << ", ";
       *os << "'instsSinceCovNew' : " << es->instsSinceCovNew << ", ";
       *os << "'md2u' : " << md2u << ", ";
       *os << "'icnt' : " << icnt << ", ";

lib/Core/TargetCalculator.cpp

@@ -51,17 +51,15 @@ void TargetCalculator::update(const ExecutionState &state) {
       unsigned index = 0;
       if (!coveredBranches[state.prevPC->parent->parent].count(
               state.prevPC->parent)) {
-        state.coveredNew->value = false;
-        state.coveredNew = new box<bool>(true);
+        state.coverNew();
         coveredBranches[state.prevPC->parent->parent][state.prevPC->parent];
       }
       for (auto succ : successors(state.getPrevPCBlock())) {
         if (succ == state.getPCBlock()) {
           if (!coveredBranches[state.prevPC->parent->parent]
                               [state.prevPC->parent]
                                   .count(index)) {
-            state.coveredNew->value = false;
-            state.coveredNew = new box<bool>(true);
+            state.coverNew();
             coveredBranches[state.prevPC->parent->parent][state.prevPC->parent]
                 .insert(index);
           }
@@ -129,6 +127,30 @@ void TargetCalculator::update(const ExecutionState &state) {
   }
 }
 
+void TargetCalculator::update(
+    ExecutionState *current, const std::vector<ExecutionState *> &addedStates,
+    const std::vector<ExecutionState *> &removedStates) {
+  if (current && (std::find(removedStates.begin(), removedStates.end(),
+                            current) == removedStates.end())) {
+    localStates.insert(current);
+  }
+  for (const auto state : addedStates) {
+    localStates.insert(state);
+  }
+  for (const auto state : removedStates) {
+    localStates.insert(state);
+  }
+  for (auto state : localStates) {
+    KFunction *kf = state->prevPC->parent->parent;
+    KModule *km = kf->parent;
+    if (state->prevPC->inst->isTerminator() &&
+        km->inMainModule(*kf->function)) {
+      update(*state);
+    }
+  }
+  localStates.clear();
+}
+
 bool TargetCalculator::differenceIsEmpty(
     const ExecutionState &state,
     const std::unordered_map<llvm::BasicBlock *, VisitedBlocks> &history,

lib/Core/TargetCalculator.h

@@ -42,6 +42,8 @@ typedef std::pair<llvm::BasicBlock *, llvm::BasicBlock *> Transition;
 typedef std::pair<llvm::BasicBlock *, unsigned> Branch;
 
 class TargetCalculator {
+  using StatesSet = std::unordered_set<ExecutionState *>;
+
   typedef std::unordered_set<KBlock *> VisitedBlocks;
   typedef std::unordered_set<Transition, TransitionHash> VisitedTransitions;
   typedef std::unordered_set<Branch, BranchHash> VisitedBranches;
@@ -63,12 +65,15 @@ class TargetCalculator {
   typedef std::unordered_set<KFunction *> CoveredFunctionsBranches;
 
   typedef std::unordered_map<llvm::Function *, VisitedBlocks> CoveredBlocks;
+  void update(const ExecutionState &state);
 
 public:
   TargetCalculator(CodeGraphInfo &codeGraphInfo)
       : codeGraphInfo(codeGraphInfo) {}
 
-  void update(const ExecutionState &state);
+  void update(ExecutionState *current,
+              const std::vector<ExecutionState *> &addedStates,
+              const std::vector<ExecutionState *> &removedStates);
 
   TargetHashSet calculate(ExecutionState &state);
 
@@ -80,6 +85,7 @@ class TargetCalculator {
   CoveredFunctionsBranches coveredFunctionsInBranches;
   CoveredFunctionsBranches fullyCoveredFunctions;
   CoveredBlocks coveredBlocks;
+  StatesSet localStates;
 
   bool differenceIsEmpty(
       const ExecutionState &state,

lib/Module/Optimize.cpp

@@ -75,6 +75,10 @@ static cl::opt<bool>
 static cl::alias A1("S", cl::desc("Alias for --strip-debug"),
                     cl::aliasopt(StripDebug));
 
+static cl::opt<bool> DeleteDeadLoops("delete-dead-loops",
+                                     cl::desc("Use LoopDeletionPass"),
+                                     cl::init(true), cl::cat(klee::ModuleCat));
+
 // A utility function that adds a pass to the pass manager but will also add
 // a verifier pass after if we're supposed to verify.
 static inline void addPass(legacy::PassManager &PM, Pass *P) {
@@ -131,8 +135,9 @@ static void AddStandardCompilePasses(legacy::PassManager &PM) {
   addPass(PM, createLoopUnswitchPass()); // Unswitch loops.
   // FIXME : Removing instcombine causes nestedloop regression.
   addPass(PM, createInstructionCombiningPass());
-  addPass(PM, createIndVarSimplifyPass());       // Canonicalize indvars
-  addPass(PM, createLoopDeletionPass());         // Delete dead loops
+  addPass(PM, createIndVarSimplifyPass()); // Canonicalize indvars
+  if (DeleteDeadLoops)
+    addPass(PM, createLoopDeletionPass());       // Delete dead loops
   addPass(PM, createLoopUnrollPass());           // Unroll small loops
   addPass(PM, createInstructionCombiningPass()); // Clean up after the unroller
   addPass(PM, createGVNPass());                  // Remove redundancies