Don't apply obsolete EagerlyAssume info

Author: NagyDonat

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

@@ -3749,9 +3749,10 @@ ExprEngine::getEagerlyAssumeBifurcationTags() {
   return std::make_pair(&TrueTag, &FalseTag);
 }
 
-/// The last expression where EagerlyAssume produced two transitions (i.e. it
-/// activated and the true and false cases were both feasible).
-REGISTER_TRAIT_WITH_PROGRAMSTATE(LastEagerlyAssumeBifurcationAt, const Expr *)
+/// If the last EagerlyAssume attempt was successful (i.e. the true and false
+/// cases were both feasible), this state trait stores the expression where it
+/// happened; otherwise this holds nullptr.
+REGISTER_TRAIT_WITH_PROGRAMSTATE(LastEagerlyAssumeExprIfSuccessful, const Expr *)
 
 void ExprEngine::evalEagerlyAssumeBifurcation(ExplodedNodeSet &Dst,
                                               ExplodedNodeSet &Src,
@@ -3768,6 +3769,7 @@ void ExprEngine::evalEagerlyAssumeBifurcation(ExplodedNodeSet &Dst,
     }
 
     ProgramStateRef State = Pred->getState();
+    State = State->set<LastEagerlyAssumeExprIfSuccessful>(nullptr);
     SVal V = State->getSVal(Ex, Pred->getLocationContext());
     std::optional<nonloc::SymbolVal> SEV = V.getAs<nonloc::SymbolVal>();
     if (SEV && SEV->isExpression()) {
@@ -3776,8 +3778,8 @@ void ExprEngine::evalEagerlyAssumeBifurcation(ExplodedNodeSet &Dst,
       auto [StateTrue, StateFalse] = State->assume(*SEV);
 
       if (StateTrue && StateFalse) {
-        StateTrue = StateTrue->set<LastEagerlyAssumeBifurcationAt>(Ex);
-        StateFalse = StateFalse->set<LastEagerlyAssumeBifurcationAt>(Ex);
+        StateTrue = StateTrue->set<LastEagerlyAssumeExprIfSuccessful>(Ex);
+        StateFalse = StateFalse->set<LastEagerlyAssumeExprIfSuccessful>(Ex);
       }
 
       // First assume that the condition is true.
@@ -3799,7 +3801,7 @@ void ExprEngine::evalEagerlyAssumeBifurcation(ExplodedNodeSet &Dst,
 
 bool ExprEngine::didEagerlyAssumeBifurcateAt(ProgramStateRef State,
                                              const Expr *Ex) const {
-  return Ex && State->get<LastEagerlyAssumeBifurcationAt>() == Ex;
+  return Ex && State->get<LastEagerlyAssumeExprIfSuccessful>() == Ex;
 }
 
 void ExprEngine::VisitGCCAsmStmt(const GCCAsmStmt *A, ExplodedNode *Pred,

clang/test/Analysis/loop-assumptions.c

@@ -65,17 +65,16 @@ void dontRememberOldBifurcation(int arg) {
   // at the first iteration of the loop, but does not make any new assumptions
   // in the subsequent iterations, so the analyzer should continue evaluating
   // the loop.
-  // FIXME: This is mishandled in `eagerly-assume` mode (which is enabled by
-  // default), because `didEagerlyAssumeBifurcateAt()` returns true for the
-  // loop condition -- referring to the bifurcation which happened on an early
-  // iteration.
+  // Previously this was mishandled in `eagerly-assume` mode (which is enabled
+  // by default), because the code remembered that there was a bifurcation on
+  // the first iteration of the loop and didn't realize that this is obsolete.
 
   // NOTE: The variable `i` is introduced to ensure that the iterations of the
   // loop change the state -- otherwise the analyzer stops iterating because it
   // returns to the same `ExplodedNode`.
   int i = 0;
   while (arg > 3) {
-    clang_analyzer_numTimesReached(); // noeagerlyassume-warning {{4}} eagerlyassume-warning {{2}}
+    clang_analyzer_numTimesReached(); // expected-warning {{4}}
     i++;
   }
 
@@ -90,10 +89,8 @@ void dontAssumeFourthIterartion(int arg) {
   // iterations (because it knows that `arg != 2` at that point), so it
   // performs a third iteration, but it does not assume that a fourth iteration
   // is also possible.
-  // FIXME: This test case is also affected by the bug described in
-  // `dontRememberOldBifurcation()`.
   for (int i = 0; i < arg; i++)
-    clang_analyzer_numTimesReached(); // noeagerlyassume-warning {{3}} eagerlyassume-warning {{2}}
+    clang_analyzer_numTimesReached(); // expected-warning {{3}}
 
   clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}} 
 }