diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp --- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp +++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp @@ -32,6 +32,7 @@ #include "llvm/ADT/Optional.h" #include "llvm/ADT/STLExtras.h" #include "llvm/Support/Error.h" +#include "llvm/Support/ErrorHandling.h" namespace clang { namespace dataflow { @@ -106,10 +107,23 @@ if (Env.getValue(Cond, SkipPast::None) == nullptr) transfer(StmtToEnv, Cond, Env); + // FIXME: The flow condition must be an r-value, so `SkipPast::None` should + // suffice. auto *Val = cast_or_null(Env.getValue(Cond, SkipPast::Reference)); - if (Val == nullptr) - return; + // Value merging depends on flow conditions from different environments + // being mutually exclusive -- that is, they cannot both be true in their + // entirety (even if they may share some clauses). So, we need *some* value + // for the condition expression, even if just an atom. + if (Val == nullptr) { + auto *Loc = Env.getStorageLocation(Cond, SkipPast::None); + if (Loc == nullptr) { + Loc = &Env.createStorageLocation(Cond); + Env.setStorageLocation(Cond, *Loc); + } + Val = &Env.makeAtomicBoolValue(); + Env.setValue(*Loc, *Val); + } // The condition must be inverted for the successor that encompasses the // "else" branch, if such exists. diff --git a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp --- a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp @@ -878,4 +878,102 @@ }); } +TEST_F(FlowConditionTest, OpaqueFlowConditionMergesToOpaqueBool) { + std::string Code = R"( + bool foo(); + + void target() { + bool Bar = true; + if (foo()) + Bar = false; + (void)0; + /*[[p]]*/ + } + )"; + runDataflow( + Code, [](llvm::ArrayRef< + std::pair>> + Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results, ElementsAre(Pair("p", _))); + const Environment &Env = Results[0].second.Env; + + const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar"); + ASSERT_THAT(BarDecl, NotNull()); + + auto &BarVal = + *cast(Env.getValue(*BarDecl, SkipPast::Reference)); + + EXPECT_FALSE(Env.flowConditionImplies(BarVal)); + }); +} + +TEST_F(FlowConditionTest, OpaqueFieldFlowConditionMergesToOpaqueBool) { + std::string Code = R"( + struct Rec { + Rec* Next; + }; + + struct Foo { + Rec* X; + }; + + void target(Foo F) { + bool Bar = true; + if (F.X->Next) + Bar = false; + (void)0; + /*[[p]]*/ + } + )"; + runDataflow( + Code, [](llvm::ArrayRef< + std::pair>> + Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results, ElementsAre(Pair("p", _))); + const Environment &Env = Results[0].second.Env; + + const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar"); + ASSERT_THAT(BarDecl, NotNull()); + + auto &BarVal = + *cast(Env.getValue(*BarDecl, SkipPast::Reference)); + + EXPECT_FALSE(Env.flowConditionImplies(BarVal)); + }); +} + +TEST_F(FlowConditionTest, OpaqueFlowConditionInsideBranchMergesToOpaqueBool) { + std::string Code = R"( + bool foo(); + + void target(bool Cond) { + bool Bar = true; + if (Cond) { + if (foo()) + Bar = false; + (void)0; + /*[[p]]*/ + } + } + )"; + runDataflow( + Code, [](llvm::ArrayRef< + std::pair>> + Results, + ASTContext &ASTCtx) { + ASSERT_THAT(Results, ElementsAre(Pair("p", _))); + const Environment &Env = Results[0].second.Env; + + const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar"); + ASSERT_THAT(BarDecl, NotNull()); + + auto &BarVal = + *cast(Env.getValue(*BarDecl, SkipPast::Reference)); + + EXPECT_FALSE(Env.flowConditionImplies(BarVal)); + }); +} + } // namespace