diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp index 6b167891c1a3a..72d807fc36705 100644 --- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp +++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp @@ -12,7 +12,6 @@ //===----------------------------------------------------------------------===// #include -#include #include #include #include @@ -33,8 +32,8 @@ #include "clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h" #include "clang/Analysis/FlowSensitive/Value.h" #include "llvm/ADT/ArrayRef.h" -#include "llvm/ADT/DenseSet.h" #include "llvm/ADT/STLExtras.h" +#include "llvm/ADT/SmallBitVector.h" #include "llvm/Support/Debug.h" #include "llvm/Support/Error.h" @@ -53,19 +52,14 @@ static int blockIndexInPredecessor(const CFGBlock &Pred, return BlockPos - Pred.succ_begin(); } -static bool isLoopHead(const CFGBlock &B) { - if (const auto *T = B.getTerminatorStmt()) - switch (T->getStmtClass()) { - case Stmt::WhileStmtClass: - case Stmt::DoStmtClass: - case Stmt::ForStmtClass: - case Stmt::CXXForRangeStmtClass: - return true; - default: - return false; - } - - return false; +// A "backedge" node is a block introduced in the CFG exclusively to indicate a +// loop backedge. They are exactly identified by the presence of a non-null +// pointer to the entry block of the loop condition. Note that this is not +// necessarily the block with the loop statement as terminator, because +// short-circuit operators will result in multiple blocks encoding the loop +// condition, only one of which will contain the loop statement as terminator. +static bool isBackedgeNode(const CFGBlock &B) { + return B.getLoopTarget() != nullptr; } namespace { @@ -502,14 +496,15 @@ runTypeErasedDataflowAnalysis( PostVisitCFG) { PrettyStackTraceAnalysis CrashInfo(CFCtx, "runTypeErasedDataflowAnalysis"); - PostOrderCFGView POV(&CFCtx.getCFG()); - ForwardDataflowWorklist Worklist(CFCtx.getCFG(), &POV); + const clang::CFG &CFG = CFCtx.getCFG(); + PostOrderCFGView POV(&CFG); + ForwardDataflowWorklist Worklist(CFG, &POV); std::vector> BlockStates( - CFCtx.getCFG().size()); + CFG.size()); // The entry basic block doesn't contain statements so it can be skipped. - const CFGBlock &Entry = CFCtx.getCFG().getEntry(); + const CFGBlock &Entry = CFG.getEntry(); BlockStates[Entry.getBlockID()] = {Analysis.typeErasedInitialElement(), InitEnv.fork()}; Worklist.enqueueSuccessors(&Entry); @@ -553,7 +548,7 @@ runTypeErasedDataflowAnalysis( llvm::errs() << "Old Env:\n"; OldBlockState->Env.dump(); }); - if (isLoopHead(*Block)) { + if (isBackedgeNode(*Block)) { LatticeJoinEffect Effect1 = Analysis.widenTypeErased( NewBlockState.Lattice, OldBlockState->Lattice); LatticeJoinEffect Effect2 = diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index 632632a1b30e7..ea36a3f705ee9 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -4099,6 +4099,20 @@ TEST(TransferTest, LoopDereferencingChangingRecordPointerConverges) { ASSERT_THAT_ERROR(checkDataflowWithNoopAnalysis(Code), llvm::Succeeded()); } +TEST(TransferTest, LoopWithShortCircuitedConditionConverges) { + std::string Code = R"cc( + bool foo(); + + void target() { + bool c = false; + while (foo() || foo()) { + c = true; + } + } + )cc"; + ASSERT_THAT_ERROR(checkDataflowWithNoopAnalysis(Code), llvm::Succeeded()); +} + TEST(TransferTest, DoesNotCrashOnUnionThisExpr) { std::string Code = R"( union Union { diff --git a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp index edd87b798198b..8422f3804db54 100644 --- a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp @@ -912,6 +912,29 @@ TEST_F(FlowConditionTest, WhileStmt) { }); } +TEST_F(FlowConditionTest, WhileStmtWithAssignmentInCondition) { + std::string Code = R"( + void target(bool Foo) { + // This test checks whether the analysis preserves the connection between + // the value of `Foo` and the assignment expression, despite widening. + // The equality operator generates a fresh boolean variable on each + // interpretation, which forces use of widening. + while ((Foo = (3 == 4))) { + (void)0; + /*[[p]]*/ + } + } + )"; + runDataflow( + Code, + [](const llvm::StringMap> &Results, + ASTContext &ASTCtx) { + const Environment &Env = getEnvironmentAtAnnotation(Results, "p"); + auto &FooVal = getValueForDecl(ASTCtx, Env, "Foo").formula(); + EXPECT_TRUE(Env.flowConditionImplies(FooVal)); + }); +} + TEST_F(FlowConditionTest, Conjunction) { std::string Code = R"( void target(bool Foo, bool Bar) {