diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h --- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h @@ -34,8 +34,8 @@ class DataflowAnalysisContext { public: DataflowAnalysisContext() - : TrueVal(&takeOwnership(std::make_unique())), - FalseVal(&takeOwnership(std::make_unique())) {} + : TrueVal(takeOwnership(std::make_unique())), + FalseVal(takeOwnership(std::make_unique())) {} /// Takes ownership of `Loc` and returns a reference to it. /// @@ -115,8 +115,8 @@ /// Returns a symbolic boolean value that models a boolean literal equal to /// `Value`. - BoolValue &getBoolLiteralValue(bool Value) const { - return Value ? *TrueVal : *FalseVal; + AtomicBoolValue &getBoolLiteralValue(bool Value) const { + return Value ? TrueVal : FalseVal; } private: @@ -135,8 +135,8 @@ StorageLocation *ThisPointeeLoc = nullptr; // FIXME: Add support for boolean expressions. - BoolValue *TrueVal; - BoolValue *FalseVal; + AtomicBoolValue &TrueVal; + AtomicBoolValue &FalseVal; }; } // namespace dataflow diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h --- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h @@ -226,7 +226,7 @@ /// Returns a symbolic boolean value that models a boolean literal equal to /// `Value` - BoolValue &getBoolLiteralValue(bool Value) const { + AtomicBoolValue &getBoolLiteralValue(bool Value) const { return DACtx->getBoolLiteralValue(Value); } diff --git a/clang/include/clang/Analysis/FlowSensitive/Transfer.h b/clang/include/clang/Analysis/FlowSensitive/Transfer.h --- a/clang/include/clang/Analysis/FlowSensitive/Transfer.h +++ b/clang/include/clang/Analysis/FlowSensitive/Transfer.h @@ -20,12 +20,23 @@ namespace clang { namespace dataflow { +/// Maps statements to the environments of basic blocks that contain them. +class StmtToEnvMap { +public: + virtual ~StmtToEnvMap() = default; + + /// Returns the environment of the basic block that contains `S` or nullptr if + /// there isn't one. + /// FIXME: Ensure that the result can't be null and return a const reference. + virtual const Environment *getEnvironment(const Stmt &S) const = 0; +}; + /// Evaluates `S` and updates `Env` accordingly. /// /// Requirements: /// /// The type of `S` must not be `ParenExpr`. -void transfer(const Stmt &S, Environment &Env); +void transfer(const StmtToEnvMap &StmtToEnv, const Stmt &S, Environment &Env); } // namespace dataflow } // namespace clang diff --git a/clang/include/clang/Analysis/FlowSensitive/Value.h b/clang/include/clang/Analysis/FlowSensitive/Value.h --- a/clang/include/clang/Analysis/FlowSensitive/Value.h +++ b/clang/include/clang/Analysis/FlowSensitive/Value.h @@ -28,7 +28,19 @@ /// Base class for all values computed by abstract interpretation. class Value { public: - enum class Kind { Bool, Integer, Reference, Pointer, Struct }; + enum class Kind { + Integer, + Reference, + Pointer, + Struct, + + // Synthetic boolean values are either atomic values or composites that + // represent conjunctions, disjunctions, and negations. + AtomicBoolValue, + BoolConjunctionValue, + BoolDisjunctionValue, + BoolNegationValue + }; explicit Value(Kind ValKind) : ValKind(ValKind) {} @@ -43,9 +55,85 @@ /// Models a boolean. class BoolValue : public Value { public: - explicit BoolValue() : Value(Kind::Bool) {} + explicit BoolValue(Kind ValueKind) : Value(ValueKind) {} - static bool classof(const Value *Val) { return Val->getKind() == Kind::Bool; } + static bool classof(const Value *Val) { + return Val->getKind() == Kind::AtomicBoolValue || + Val->getKind() == Kind::BoolNegationValue || + Val->getKind() == Kind::BoolConjunctionValue || + Val->getKind() == Kind::BoolDisjunctionValue; + } +}; + +/// Models an atomic boolean. +class AtomicBoolValue : public BoolValue { +public: + explicit AtomicBoolValue() : BoolValue(Kind::AtomicBoolValue) {} + + static bool classof(const Value *Val) { + return Val->getKind() == Kind::AtomicBoolValue; + } +}; + +/// Models a boolean conjunction. +class BoolConjunctionValue : public BoolValue { +public: + explicit BoolConjunctionValue(BoolValue &LeftSubVal, BoolValue &RightSubVal) + : BoolValue(Kind::BoolConjunctionValue), LeftSubVal(LeftSubVal), + RightSubVal(RightSubVal) {} + + static bool classof(const Value *Val) { + return Val->getKind() == Kind::BoolConjunctionValue; + } + + /// Returns the left sub-value of the conjunction. + BoolValue &getLeftSubValue() const { return LeftSubVal; } + + /// Returns the right sub-value of the conjunction. + BoolValue &getRightSubValue() const { return RightSubVal; } + +private: + BoolValue &LeftSubVal; + BoolValue &RightSubVal; +}; + +/// Models a boolean disjunction. +class BoolDisjunctionValue : public BoolValue { +public: + explicit BoolDisjunctionValue(BoolValue &LeftSubVal, BoolValue &RightSubVal) + : BoolValue(Kind::BoolDisjunctionValue), LeftSubVal(LeftSubVal), + RightSubVal(RightSubVal) {} + + static bool classof(const Value *Val) { + return Val->getKind() == Kind::BoolDisjunctionValue; + } + + /// Returns the left sub-value of the disjunction. + BoolValue &getLeftSubValue() const { return LeftSubVal; } + + /// Returns the right sub-value of the disjunction. + BoolValue &getRightSubValue() const { return RightSubVal; } + +private: + BoolValue &LeftSubVal; + BoolValue &RightSubVal; +}; + +/// Models a boolean negation. +class BoolNegationValue : public BoolValue { +public: + explicit BoolNegationValue(BoolValue &SubVal) + : BoolValue(Kind::BoolNegationValue), SubVal(SubVal) {} + + static bool classof(const Value *Val) { + return Val->getKind() == Kind::BoolNegationValue; + } + + /// Returns the sub-value of the negation. + BoolValue &getSubVal() const { return SubVal; } + +private: + BoolValue &SubVal; }; /// Models an integer. diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -39,10 +39,12 @@ class TransferVisitor : public ConstStmtVisitor { public: - TransferVisitor(Environment &Env) : Env(Env) {} + TransferVisitor(const StmtToEnvMap &StmtToEnv, Environment &Env) + : StmtToEnv(StmtToEnv), Env(Env) {} void VisitBinaryOperator(const BinaryOperator *S) { - if (S->getOpcode() == BO_Assign) { + 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); @@ -51,7 +53,7 @@ assert(LHS != nullptr); auto *LHSLoc = Env.getStorageLocation(*LHS, SkipPast::Reference); if (LHSLoc == nullptr) - return; + break; // The CFG does not contain `ParenExpr` as top-level statements in basic // blocks, however sub-expressions can still be of that type. @@ -61,15 +63,56 @@ assert(RHS != nullptr); Value *RHSVal = Env.getValue(*RHS, SkipPast::Reference); if (RHSVal == nullptr) - return; + break; // Assign a value to the storage location of the left-hand side. Env.setValue(*LHSLoc, *RHSVal); // Assign a storage location for the whole expression. Env.setStorageLocation(*S, *LHSLoc); + break; + } + 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(Env.getValue(*LHS, SkipPast::Reference)); + + // `RHS` and `S` might be part of different 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( + RHSEnv->getValue(*RHS, SkipPast::Reference)); + + // Create fresh values for unknown boolean expressions. + if (LHSVal == nullptr) + LHSVal = &Env.takeOwnership(std::make_unique()); + if (RHSVal == nullptr) + RHSVal = &Env.takeOwnership(std::make_unique()); + + auto &Loc = Env.createStorageLocation(*S); + Env.setStorageLocation(*S, Loc); + if (S->getOpcode() == BO_LAnd) + Env.setValue( + Loc, Env.takeOwnership( + std::make_unique(*LHSVal, *RHSVal))); + else + Env.setValue( + Loc, Env.takeOwnership( + std::make_unique(*LHSVal, *RHSVal))); + break; + } + default: + // FIXME: Add support for BO_EQ, BO_NE. + break; } - // FIXME: Add support for BO_EQ, BO_NE. } void VisitDeclRefExpr(const DeclRefExpr *S) { @@ -212,8 +255,19 @@ Env.setValue(PointerLoc, PointerVal); break; } + case UO_LNot: { + auto *SubExprVal = + dyn_cast_or_null(Env.getValue(*SubExpr, SkipPast::None)); + if (SubExprVal == nullptr) + return; + + auto &ExprLoc = Env.createStorageLocation(*S); + Env.setStorageLocation(*S, ExprLoc); + Env.setValue( + ExprLoc, + Env.takeOwnership(std::make_unique(*SubExprVal))); + } default: - // FIXME: Add support for UO_LNot. break; } } @@ -450,12 +504,13 @@ } private: + const StmtToEnvMap &StmtToEnv; Environment &Env; }; -void transfer(const Stmt &S, Environment &Env) { +void transfer(const StmtToEnvMap &StmtToEnv, const Stmt &S, Environment &Env) { assert(!isa(&S)); - TransferVisitor(Env).Visit(&S); + TransferVisitor(StmtToEnv, Env).Visit(&S); } } // namespace dataflow 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 @@ -24,6 +24,7 @@ #include "clang/Analysis/FlowSensitive/Transfer.h" #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/None.h" #include "llvm/ADT/Optional.h" @@ -32,6 +33,27 @@ namespace clang { namespace dataflow { +class StmtToEnvMapImpl : public StmtToEnvMap { +public: + StmtToEnvMapImpl( + const ControlFlowContext &CFCtx, + llvm::ArrayRef> + BlockToState) + : CFCtx(CFCtx), BlockToState(BlockToState) {} + + const Environment *getEnvironment(const Stmt &S) const override { + auto BlockIT = CFCtx.getStmtToBlock().find(&S); + assert(BlockIT != CFCtx.getStmtToBlock().end()); + const auto &State = BlockToState[BlockIT->getSecond()->getBlockID()]; + assert(State.hasValue()); + return &State.getValue().Env; + } + +private: + const ControlFlowContext &CFCtx; + llvm::ArrayRef> BlockToState; +}; + /// Computes the input state for a given basic block by joining the output /// states of its predecessors. /// @@ -42,7 +64,7 @@ /// `llvm::None` represent basic blocks that are not evaluated yet. static TypeErasedDataflowAnalysisState computeBlockInputState( const ControlFlowContext &CFCtx, - std::vector> &BlockStates, + llvm::ArrayRef> BlockStates, const CFGBlock &Block, const Environment &InitEnv, TypeErasedDataflowAnalysis &Analysis) { llvm::DenseSet Preds; @@ -111,17 +133,19 @@ /// Transfers `State` by evaluating `CfgStmt` in the context of `Analysis`. /// `HandleTransferredStmt` (if provided) will be applied to `CfgStmt`, after it /// is evaluated. -static void -transferCFGStmt(const CFGStmt &CfgStmt, TypeErasedDataflowAnalysis &Analysis, - TypeErasedDataflowAnalysisState &State, - std::function - HandleTransferredStmt) { +static void transferCFGStmt( + const ControlFlowContext &CFCtx, + llvm::ArrayRef> BlockStates, + const CFGStmt &CfgStmt, TypeErasedDataflowAnalysis &Analysis, + TypeErasedDataflowAnalysisState &State, + std::function + HandleTransferredStmt) { const Stmt *S = CfgStmt.getStmt(); assert(S != nullptr); if (Analysis.applyBuiltinTransfer()) - transfer(*S, State.Env); + transfer(StmtToEnvMapImpl(CFCtx, BlockStates), *S, State.Env); Analysis.transferTypeErased(S, State.Lattice, State.Env); if (HandleTransferredStmt != nullptr) @@ -176,8 +200,8 @@ for (const CFGElement &Element : Block) { switch (Element.getKind()) { case CFGElement::Statement: - transferCFGStmt(*Element.getAs(), Analysis, State, - HandleTransferredStmt); + transferCFGStmt(CFCtx, BlockStates, *Element.getAs(), Analysis, + State, HandleTransferredStmt); break; case CFGElement::Initializer: if (Analysis.applyBuiltinTransfer()) diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -2006,6 +2006,42 @@ // [[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 *FooDecl = findValueDecl(ASTCtx, "Foo"); + ASSERT_THAT(FooDecl, NotNull()); + + const auto *FooVal = dyn_cast_or_null( + Env.getValue(*FooDecl, SkipPast::None)); + ASSERT_THAT(FooVal, NotNull()); + + const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar"); + ASSERT_THAT(BarDecl, NotNull()); + + const auto *BarVal = dyn_cast_or_null( + Env.getValue(*BarDecl, SkipPast::None)); + ASSERT_THAT(BarVal, NotNull()); + + EXPECT_EQ(FooVal, &Env.getBoolLiteralValue(true)); + EXPECT_EQ(BarVal, &Env.getBoolLiteralValue(false)); + }); +} + +TEST_F(TransferTest, AssignFromBoolConjunction) { + std::string Code = R"( + void target() { + bool Foo = true; + bool Bar = true; + bool Baz = (Foo) && (Bar); + // [[p]] + } + )"; runDataflow( Code, [](llvm::ArrayRef< std::pair>> @@ -2028,9 +2064,93 @@ dyn_cast_or_null(Env.getValue(*BarDecl, SkipPast::None)); ASSERT_THAT(BarVal, NotNull()); - EXPECT_EQ(FooVal, &Env.getBoolLiteralValue(true)); - EXPECT_EQ(BarVal, &Env.getBoolLiteralValue(false)); + const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz"); + ASSERT_THAT(BazDecl, NotNull()); + + const auto *BazVal = dyn_cast_or_null( + Env.getValue(*BazDecl, SkipPast::None)); + ASSERT_THAT(BazVal, NotNull()); + + EXPECT_EQ(&BazVal->getLeftSubValue(), FooVal); + EXPECT_EQ(&BazVal->getRightSubValue(), BarVal); }); } +TEST_F(TransferTest, AssignFromBoolDisjunction) { + std::string Code = R"( + void target() { + bool Foo = true; + bool Bar = true; + bool Baz = (Foo) || (Bar); + // [[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 *FooDecl = findValueDecl(ASTCtx, "Foo"); + ASSERT_THAT(FooDecl, NotNull()); + + const auto *FooVal = + dyn_cast_or_null(Env.getValue(*FooDecl, SkipPast::None)); + ASSERT_THAT(FooVal, NotNull()); + + const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar"); + ASSERT_THAT(BarDecl, NotNull()); + + const auto *BarVal = + dyn_cast_or_null(Env.getValue(*BarDecl, SkipPast::None)); + ASSERT_THAT(BarVal, NotNull()); + + const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz"); + ASSERT_THAT(BazDecl, NotNull()); + + const auto *BazVal = dyn_cast_or_null( + Env.getValue(*BazDecl, SkipPast::None)); + ASSERT_THAT(BazVal, NotNull()); + + EXPECT_EQ(&BazVal->getLeftSubValue(), FooVal); + EXPECT_EQ(&BazVal->getRightSubValue(), BarVal); + }); +} + +TEST_F(TransferTest, AssignFromBoolNegation) { + std::string Code = R"( + void target() { + bool Foo = true; + bool Bar = !(Foo); + // [[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 *FooDecl = findValueDecl(ASTCtx, "Foo"); + ASSERT_THAT(FooDecl, NotNull()); + + const auto *FooVal = dyn_cast_or_null( + Env.getValue(*FooDecl, SkipPast::None)); + ASSERT_THAT(FooVal, NotNull()); + + const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar"); + ASSERT_THAT(BarDecl, NotNull()); + + const auto *BarVal = dyn_cast_or_null( + Env.getValue(*BarDecl, SkipPast::None)); + ASSERT_THAT(BarVal, NotNull()); + + EXPECT_EQ(&BarVal->getSubVal(), FooVal); + }); +} + } // namespace 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 @@ -387,7 +387,8 @@ Code, "target", [this](ASTContext &Context, Environment &Env) { assert(HasValueTop == nullptr); - HasValueTop = &Env.takeOwnership(std::make_unique()); + HasValueTop = + &Env.takeOwnership(std::make_unique()); return OptionalIntAnalysis(Context, *HasValueTop); }, [&Match](