https://github.com/ymand updated https://github.com/llvm/llvm-project/pull/68923
>From 83486a35e6d0e754dd99369fc546d04afedbe923 Mon Sep 17 00:00:00 2001 From: Yitzhak Mandelbaum <yitzh...@google.com> Date: Thu, 12 Oct 2023 19:35:54 +0000 Subject: [PATCH 1/2] [clang][dataflow] Check for backedges directly (instead of loop statements). Widen on backedge nodes, instead of nodes with a loop statement as terminator. This fixes #67834 and a precision loss from assignment in a loop condition. The commit contains tests for both of these issues. --- .../TypeErasedDataflowAnalysis.cpp | 29 ++++++------------- .../Analysis/FlowSensitive/TransferTest.cpp | 14 +++++++++ .../TypeErasedDataflowAnalysisTest.cpp | 28 ++++++++++++++++++ 3 files changed, 51 insertions(+), 20 deletions(-) diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp index 6b167891c1a3ac8..5da3a5b1ccf8a2c 100644 --- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp +++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp @@ -12,7 +12,6 @@ //===----------------------------------------------------------------------===// #include <algorithm> -#include <memory> #include <optional> #include <system_error> #include <utility> @@ -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,8 @@ 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; +static bool isBackedgeNode(const CFGBlock &B) { + return B.getLoopTarget() != nullptr; } namespace { @@ -502,14 +490,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<std::optional<TypeErasedDataflowAnalysisState>> 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 +542,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 632632a1b30e78b..7c697fa5f87d941 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, LoopWithDisjunctiveConditionConverges) { + 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 2425bb8711bdbaf..6800d736afd9b08 100644 --- a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp @@ -913,6 +913,34 @@ 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<DataflowAnalysisState<NoopLattice>> &Results, + ASTContext &ASTCtx) { + const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo"); + ASSERT_THAT(FooDecl, NotNull()); + + ASSERT_THAT(Results.keys(), UnorderedElementsAre("p")); + const Environment &Env = getEnvironmentAtAnnotation(Results, "p"); + + auto &FooVal = cast<BoolValue>(Env.getValue(*FooDecl))->formula(); + EXPECT_TRUE(Env.flowConditionImplies(FooVal)); + }); +} + TEST_F(FlowConditionTest, Conjunction) { std::string Code = R"( void target(bool Foo, bool Bar) { >From bdda46db3376cffbceed2de392783456b4fcc62c Mon Sep 17 00:00:00 2001 From: Yitzhak Mandelbaum <yitzh...@google.com> Date: Thu, 12 Oct 2023 20:36:38 +0000 Subject: [PATCH 2/2] fixup! [clang][dataflow] Check for backedges directly (instead of loop statements). --- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp index 5da3a5b1ccf8a2c..d96ad66dd6749c4 100644 --- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp +++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp @@ -53,7 +53,7 @@ static int blockIndexInPredecessor(const CFGBlock &Pred, } static bool isBackedgeNode(const CFGBlock &B) { - return B.getLoopTarget() != nullptr; + return B.getLoopTarget() != nullptr; } namespace { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits