[clang] cf63e9d - [clang][dataflow] Add support for nested composite bool expressions
Stanislav Gatev via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 14 10:18:51 PDT 2022
Author: Stanislav Gatev
Date: 2022-03-14T17:18:30Z
New Revision: cf63e9d4caccbd540df083c63a5217ac50b5e1f9
URL: https://github.com/llvm/llvm-project/commit/cf63e9d4caccbd540df083c63a5217ac50b5e1f9
DIFF: https://github.com/llvm/llvm-project/commit/cf63e9d4caccbd540df083c63a5217ac50b5e1f9.diff
LOG: [clang][dataflow] Add support for nested composite bool expressions
This is part of the implementation of the dataflow analysis framework.
See "[RFC] A dataflow analysis framework for Clang AST" on cfe-dev.
Differential Revision: https://reviews.llvm.org/D121455
Added:
Modified:
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
Removed:
################################################################################
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index df4d919b6006a..0430bc3c6eb18 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -43,25 +43,23 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
: StmtToEnv(StmtToEnv), Env(Env) {}
void VisitBinaryOperator(const BinaryOperator *S) {
+ // The CFG does not contain `ParenExpr` as top-level statements in basic
+ // blocks, however sub-expressions can still be of that type.
+ assert(S->getLHS() != nullptr);
+ const Expr *LHS = S->getLHS()->IgnoreParens();
+ assert(LHS != nullptr);
+
+ assert(S->getRHS() != nullptr);
+ const Expr *RHS = S->getRHS()->IgnoreParens();
+ assert(RHS != nullptr);
+
switch (S->getOpcode()) {
case BO_Assign: {
- // The CFG does not contain `ParenExpr` as top-level statements in basic
- // blocks, however sub-expressions can still be of that type.
- assert(S->getLHS() != nullptr);
- const Expr *LHS = S->getLHS()->IgnoreParens();
-
- assert(LHS != nullptr);
auto *LHSLoc = Env.getStorageLocation(*LHS, SkipPast::Reference);
if (LHSLoc == nullptr)
break;
- // The CFG does not contain `ParenExpr` as top-level statements in basic
- // blocks, however sub-expressions can still be of that type.
- assert(S->getRHS() != nullptr);
- const Expr *RHS = S->getRHS()->IgnoreParens();
-
- assert(RHS != nullptr);
- Value *RHSVal = Env.getValue(*RHS, SkipPast::Reference);
+ auto *RHSVal = Env.getValue(*RHS, SkipPast::Reference);
if (RHSVal == nullptr)
break;
@@ -74,38 +72,15 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
}
case BO_LAnd:
case BO_LOr: {
- const Expr *LHS = S->getLHS();
- assert(LHS != nullptr);
-
- const Expr *RHS = S->getRHS();
- assert(RHS != nullptr);
-
- BoolValue *LHSVal =
- dyn_cast_or_null<BoolValue>(Env.getValue(*LHS, SkipPast::Reference));
-
- // `RHS` and `S` might be part of
diff erent basic blocks. We need to
- // access their values from the corresponding environments.
- BoolValue *RHSVal = nullptr;
- const Environment *RHSEnv = StmtToEnv.getEnvironment(*RHS);
- if (RHSEnv != nullptr)
- RHSVal = dyn_cast_or_null<BoolValue>(
- RHSEnv->getValue(*RHS, SkipPast::Reference));
-
- // Create fresh values for unknown boolean expressions.
- // FIXME: Consider providing a `GetOrCreateFresh` util in case this style
- // is expected to be common or make sure that all expressions are assigned
- // values and drop this.
- if (LHSVal == nullptr)
- LHSVal = &Env.takeOwnership(std::make_unique<AtomicBoolValue>());
- if (RHSVal == nullptr)
- RHSVal = &Env.takeOwnership(std::make_unique<AtomicBoolValue>());
+ BoolValue &LHSVal = getLogicOperatorSubExprValue(*LHS);
+ BoolValue &RHSVal = getLogicOperatorSubExprValue(*RHS);
auto &Loc = Env.createStorageLocation(*S);
Env.setStorageLocation(*S, Loc);
if (S->getOpcode() == BO_LAnd)
- Env.setValue(Loc, Env.makeAnd(*LHSVal, *RHSVal));
+ Env.setValue(Loc, Env.makeAnd(LHSVal, RHSVal));
else
- Env.setValue(Loc, Env.makeOr(*LHSVal, *RHSVal));
+ Env.setValue(Loc, Env.makeOr(LHSVal, RHSVal));
break;
}
default:
@@ -525,6 +500,31 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
}
private:
+ BoolValue &getLogicOperatorSubExprValue(const Expr &SubExpr) {
+ // `SubExpr` and its parent logic operator might be part of
diff erent basic
+ // blocks. We try to access the value that is assigned to `SubExpr` in the
+ // corresponding environment.
+ if (const Environment *SubExprEnv = StmtToEnv.getEnvironment(SubExpr)) {
+ if (auto *Val = dyn_cast_or_null<BoolValue>(
+ SubExprEnv->getValue(SubExpr, SkipPast::Reference)))
+ return *Val;
+ }
+
+ // Sub-expressions that are logic operators are not added in basic blocks
+ // (e.g. see CFG for `bool d = a && (b || c);`). If `SubExpr` is a logic
+ // operator, it isn't evaluated and assigned a value yet. In that case, we
+ // need to first visit `SubExpr` and then try to get the value that gets
+ // assigned to it.
+ Visit(&SubExpr);
+ if (auto *Val = dyn_cast_or_null<BoolValue>(
+ Env.getValue(SubExpr, SkipPast::Reference)))
+ return *Val;
+
+ // If the value of `SubExpr` is still unknown, we create a fresh symbolic
+ // boolean value for it.
+ return Env.makeAtomicBoolValue();
+ }
+
const StmtToEnvMap &StmtToEnv;
Environment &Env;
};
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 49aaca9a7588c..f1335fbd49fa9 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2060,86 +2060,109 @@ TEST_F(TransferTest, AssignFromBoolLiteral) {
});
}
-TEST_F(TransferTest, AssignFromBoolConjunction) {
- std::string Code = R"(
- void target(bool Foo, bool Bar) {
- bool Baz = (Foo) && (Bar);
+TEST_F(TransferTest, AssignFromCompositeBoolExpression) {
+ {
+ std::string Code = R"(
+ void target(bool Foo, bool Bar, bool Qux) {
+ bool Baz = (Foo) && (Bar || Qux);
// [[p]]
}
)";
- runDataflow(
- Code, [](llvm::ArrayRef<
- std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
- Results,
- ASTContext &ASTCtx) {
- ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
- const Environment &Env = Results[0].second.Env;
+ runDataflow(
+ Code, [](llvm::ArrayRef<
+ std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+ Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+ const Environment &Env = Results[0].second.Env;
- const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
- ASSERT_THAT(FooDecl, NotNull());
+ const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ ASSERT_THAT(FooDecl, NotNull());
- const auto *FooVal =
- dyn_cast_or_null<BoolValue>(Env.getValue(*FooDecl, SkipPast::None));
- ASSERT_THAT(FooVal, NotNull());
+ const auto *FooVal = dyn_cast_or_null<BoolValue>(
+ Env.getValue(*FooDecl, SkipPast::None));
+ ASSERT_THAT(FooVal, NotNull());
- const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
- ASSERT_THAT(BarDecl, NotNull());
+ const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ ASSERT_THAT(BarDecl, NotNull());
- const auto *BarVal =
- dyn_cast_or_null<BoolValue>(Env.getValue(*BarDecl, SkipPast::None));
- ASSERT_THAT(BarVal, NotNull());
+ const auto *BarVal = dyn_cast_or_null<BoolValue>(
+ Env.getValue(*BarDecl, SkipPast::None));
+ ASSERT_THAT(BarVal, NotNull());
- const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
- ASSERT_THAT(BazDecl, NotNull());
+ const ValueDecl *QuxDecl = findValueDecl(ASTCtx, "Qux");
+ ASSERT_THAT(QuxDecl, NotNull());
- const auto *BazVal = dyn_cast_or_null<ConjunctionValue>(
- Env.getValue(*BazDecl, SkipPast::None));
- ASSERT_THAT(BazVal, NotNull());
+ const auto *QuxVal = dyn_cast_or_null<BoolValue>(
+ Env.getValue(*QuxDecl, SkipPast::None));
+ ASSERT_THAT(QuxVal, NotNull());
- EXPECT_EQ(&BazVal->getLeftSubValue(), FooVal);
- EXPECT_EQ(&BazVal->getRightSubValue(), BarVal);
- });
-}
+ const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ ASSERT_THAT(BazDecl, NotNull());
-TEST_F(TransferTest, AssignFromBoolDisjunction) {
- std::string Code = R"(
- void target(bool Foo, bool Bar) {
- bool Baz = (Foo) || (Bar);
+ const auto *BazVal = dyn_cast_or_null<ConjunctionValue>(
+ Env.getValue(*BazDecl, SkipPast::None));
+ ASSERT_THAT(BazVal, NotNull());
+ EXPECT_EQ(&BazVal->getLeftSubValue(), FooVal);
+
+ const auto *BazRightSubValVal =
+ cast<DisjunctionValue>(&BazVal->getRightSubValue());
+ EXPECT_EQ(&BazRightSubValVal->getLeftSubValue(), BarVal);
+ EXPECT_EQ(&BazRightSubValVal->getRightSubValue(), QuxVal);
+ });
+ }
+
+ {
+ std::string Code = R"(
+ void target(bool Foo, bool Bar, bool Qux) {
+ bool Baz = (Foo && Qux) || (Bar);
// [[p]]
}
)";
- runDataflow(
- Code, [](llvm::ArrayRef<
- std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
- Results,
- ASTContext &ASTCtx) {
- ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
- const Environment &Env = Results[0].second.Env;
+ runDataflow(
+ Code, [](llvm::ArrayRef<
+ std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
+ Results,
+ ASTContext &ASTCtx) {
+ ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+ const Environment &Env = Results[0].second.Env;
- const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
- ASSERT_THAT(FooDecl, NotNull());
+ const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ ASSERT_THAT(FooDecl, NotNull());
- const auto *FooVal =
- dyn_cast_or_null<BoolValue>(Env.getValue(*FooDecl, SkipPast::None));
- ASSERT_THAT(FooVal, NotNull());
+ const auto *FooVal = dyn_cast_or_null<BoolValue>(
+ Env.getValue(*FooDecl, SkipPast::None));
+ ASSERT_THAT(FooVal, NotNull());
- const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
- ASSERT_THAT(BarDecl, NotNull());
+ const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ ASSERT_THAT(BarDecl, NotNull());
- const auto *BarVal =
- dyn_cast_or_null<BoolValue>(Env.getValue(*BarDecl, SkipPast::None));
- ASSERT_THAT(BarVal, NotNull());
+ const auto *BarVal = dyn_cast_or_null<BoolValue>(
+ Env.getValue(*BarDecl, SkipPast::None));
+ ASSERT_THAT(BarVal, NotNull());
- const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
- ASSERT_THAT(BazDecl, NotNull());
+ const ValueDecl *QuxDecl = findValueDecl(ASTCtx, "Qux");
+ ASSERT_THAT(QuxDecl, NotNull());
- const auto *BazVal = dyn_cast_or_null<DisjunctionValue>(
- Env.getValue(*BazDecl, SkipPast::None));
- ASSERT_THAT(BazVal, NotNull());
+ const auto *QuxVal = dyn_cast_or_null<BoolValue>(
+ Env.getValue(*QuxDecl, SkipPast::None));
+ ASSERT_THAT(QuxVal, NotNull());
- EXPECT_EQ(&BazVal->getLeftSubValue(), FooVal);
- EXPECT_EQ(&BazVal->getRightSubValue(), BarVal);
- });
+ const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ ASSERT_THAT(BazDecl, NotNull());
+
+ const auto *BazVal = dyn_cast_or_null<DisjunctionValue>(
+ Env.getValue(*BazDecl, SkipPast::None));
+ ASSERT_THAT(BazVal, NotNull());
+
+ const auto *BazLeftSubValVal =
+ cast<ConjunctionValue>(&BazVal->getLeftSubValue());
+ EXPECT_EQ(&BazLeftSubValVal->getLeftSubValue(), FooVal);
+ EXPECT_EQ(&BazLeftSubValVal->getRightSubValue(), QuxVal);
+
+ EXPECT_EQ(&BazVal->getRightSubValue(), BarVal);
+ });
+ }
}
TEST_F(TransferTest, AssignFromBoolNegation) {
More information about the cfe-commits
mailing list