diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h index 2e9c088d0e5c35..f17df36f6a4a34 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h @@ -128,21 +128,6 @@ class Environment { /// with a symbolic representation of the `this` pointee. Environment(DataflowAnalysisContext &DACtx, const DeclContext &DeclCtx); - /// Creates and returns an environment to use for an inline analysis of the - /// callee. Uses the storage location from each argument in the `Call` as the - /// storage location for the corresponding parameter in the callee. - /// - /// Requirements: - /// - /// The callee of `Call` must be a `FunctionDecl` with a body. - /// - /// The body of the callee must not reference globals. - /// - /// The arguments of `Call` must map 1:1 to the callee's parameters. - /// - /// Each argument of `Call` must already have a `StorageLocation`. - Environment pushCall(const CallExpr *Call) const; - /// Returns true if and only if the environment is equivalent to `Other`, i.e /// the two environments: /// - have the same mappings from declarations to storage locations, diff --git a/clang/include/clang/Analysis/FlowSensitive/Transfer.h b/clang/include/clang/Analysis/FlowSensitive/Transfer.h index cbb625487c1ebf..25afa01f307c0b 100644 --- a/clang/include/clang/Analysis/FlowSensitive/Transfer.h +++ b/clang/include/clang/Analysis/FlowSensitive/Transfer.h @@ -20,12 +20,6 @@ namespace clang { namespace dataflow { -struct TransferOptions { - /// Determines whether to analyze function bodies when present in the - /// translation unit. - bool ContextSensitive = false; -}; - /// Maps statements to the environments of basic blocks that contain them. class StmtToEnvMap { public: @@ -42,8 +36,7 @@ class StmtToEnvMap { /// Requirements: /// /// `S` must not be `ParenExpr` or `ExprWithCleanups`. -void transfer(const StmtToEnvMap &StmtToEnv, const Stmt &S, Environment &Env, - TransferOptions Options); +void transfer(const StmtToEnvMap &StmtToEnv, const Stmt &S, Environment &Env); } // namespace dataflow } // namespace clang diff --git a/clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h b/clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h index 92700f164e7bd6..b043062459e4e6 100644 --- a/clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h +++ b/clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h @@ -23,7 +23,6 @@ #include "clang/Analysis/FlowSensitive/ControlFlowContext.h" #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" #include "clang/Analysis/FlowSensitive/DataflowLattice.h" -#include "clang/Analysis/FlowSensitive/Transfer.h" #include "llvm/ADT/Any.h" #include "llvm/ADT/Optional.h" #include "llvm/Support/Error.h" @@ -37,9 +36,6 @@ struct DataflowAnalysisOptions { // (at which point the built-in transfer functions can be simply a standalone // analysis). bool ApplyBuiltinTransfer = true; - - /// Only has an effect if `ApplyBuiltinTransfer` is true. - TransferOptions BuiltinTransferOptions; }; /// Type-erased lattice element container. @@ -61,7 +57,7 @@ class TypeErasedDataflowAnalysis : public Environment::ValueModel { /// Deprecated. Use the `DataflowAnalysisOptions` constructor instead. TypeErasedDataflowAnalysis(bool ApplyBuiltinTransfer) - : Options({ApplyBuiltinTransfer, TransferOptions{}}) {} + : Options({ApplyBuiltinTransfer}) {} TypeErasedDataflowAnalysis(DataflowAnalysisOptions Options) : Options(Options) {} @@ -94,11 +90,6 @@ class TypeErasedDataflowAnalysis : public Environment::ValueModel { /// Determines whether to apply the built-in transfer functions, which model /// the heap and stack in the `Environment`. bool applyBuiltinTransfer() const { return Options.ApplyBuiltinTransfer; } - - /// Returns the options to be passed to the built-in transfer functions. - TransferOptions builtinTransferOptions() const { - return Options.BuiltinTransferOptions; - } }; /// Type-erased model of the program at a given program point. diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp index 79962477025bb7..d1af293a9b3e46 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -200,44 +200,6 @@ Environment::Environment(DataflowAnalysisContext &DACtx, } } -Environment Environment::pushCall(const CallExpr *Call) const { - Environment Env(*this); - - // FIXME: Currently this only works if the callee is never a method and the - // same callee is never analyzed from multiple separate callsites. To - // generalize this, we'll need to store a "context" field (probably a stack of - // `const CallExpr *`s) in the `Environment`, and then change the - // `DataflowAnalysisContext` class to hold a map from contexts to "frames", - // where each frame stores its own version of what are currently the - // `DeclToLoc`, `ExprToLoc`, and `ThisPointeeLoc` fields. - - const auto *FuncDecl = Call->getDirectCallee(); - assert(FuncDecl != nullptr); - const auto *Body = FuncDecl->getBody(); - assert(Body != nullptr); - // FIXME: In order to allow the callee to reference globals, we probably need - // to call `initGlobalVars` here in some way. - - auto ParamIt = FuncDecl->param_begin(); - auto ParamEnd = FuncDecl->param_end(); - auto ArgIt = Call->arg_begin(); - auto ArgEnd = Call->arg_end(); - - // FIXME: Parameters don't always map to arguments 1:1; examples include - // overloaded operators implemented as member functions, and parameter packs. - for (; ArgIt != ArgEnd; ++ParamIt, ++ArgIt) { - assert(ParamIt != ParamEnd); - - const VarDecl *Param = *ParamIt; - const Expr *Arg = *ArgIt; - auto *ArgLoc = Env.getStorageLocation(*Arg, SkipPast::Reference); - assert(ArgLoc != nullptr); - Env.setStorageLocation(*Param, *ArgLoc); - } - - return Env; -} - bool Environment::equivalentTo(const Environment &Other, Environment::ValueModel &Model) const { assert(DACtx == Other.DACtx); diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index bbf7526adce923..2e77920850b82b 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -20,9 +20,7 @@ #include "clang/AST/OperationKinds.h" #include "clang/AST/Stmt.h" #include "clang/AST/StmtVisitor.h" -#include "clang/Analysis/FlowSensitive/ControlFlowContext.h" #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" -#include "clang/Analysis/FlowSensitive/NoopAnalysis.h" #include "clang/Analysis/FlowSensitive/Value.h" #include "clang/Basic/Builtins.h" #include "clang/Basic/OperatorKinds.h" @@ -48,9 +46,8 @@ static BoolValue &evaluateBooleanEquality(const Expr &LHS, const Expr &RHS, class TransferVisitor : public ConstStmtVisitor { public: - TransferVisitor(const StmtToEnvMap &StmtToEnv, Environment &Env, - TransferOptions Options) - : StmtToEnv(StmtToEnv), Env(Env), Options(Options) {} + TransferVisitor(const StmtToEnvMap &StmtToEnv, Environment &Env) + : StmtToEnv(StmtToEnv), Env(Env) {} void VisitBinaryOperator(const BinaryOperator *S) { const Expr *LHS = S->getLHS(); @@ -506,35 +503,6 @@ class TransferVisitor : public ConstStmtVisitor { if (ArgLoc == nullptr) return; Env.setStorageLocation(*S, *ArgLoc); - } else if (const FunctionDecl *F = S->getDirectCallee()) { - // This case is for context-sensitive analysis, which we only do if we - // have the callee body available in the translation unit. - if (!Options.ContextSensitive || F->getBody() == nullptr) - return; - - auto &ASTCtx = F->getASTContext(); - - // FIXME: Cache these CFGs. - auto CFCtx = ControlFlowContext::build(F, F->getBody(), &ASTCtx); - // FIXME: Handle errors here and below. - assert(CFCtx); - auto ExitBlock = CFCtx->getCFG().getExit().getBlockID(); - - auto CalleeEnv = Env.pushCall(S); - - // FIXME: Use the same analysis as the caller for the callee. - DataflowAnalysisOptions Options; - auto Analysis = NoopAnalysis(ASTCtx, Options); - - auto BlockToOutputState = - dataflow::runDataflowAnalysis(*CFCtx, Analysis, CalleeEnv); - assert(BlockToOutputState); - assert(ExitBlock < BlockToOutputState->size()); - - auto ExitState = (*BlockToOutputState)[ExitBlock]; - assert(ExitState); - - Env = ExitState->Env; } } @@ -665,12 +633,10 @@ class TransferVisitor : public ConstStmtVisitor { const StmtToEnvMap &StmtToEnv; Environment &Env; - TransferOptions Options; }; -void transfer(const StmtToEnvMap &StmtToEnv, const Stmt &S, Environment &Env, - TransferOptions Options) { - TransferVisitor(StmtToEnv, Env, Options).Visit(&S); +void transfer(const StmtToEnvMap &StmtToEnv, const Stmt &S, Environment &Env) { + TransferVisitor(StmtToEnv, Env).Visit(&S); } } // namespace dataflow diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp index a63b43e2e4ad53..6ed37d26941efc 100644 --- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp +++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp @@ -74,9 +74,8 @@ static int blockIndexInPredecessor(const CFGBlock &Pred, class TerminatorVisitor : public ConstStmtVisitor { public: TerminatorVisitor(const StmtToEnvMap &StmtToEnv, Environment &Env, - int BlockSuccIdx, TransferOptions TransferOptions) - : StmtToEnv(StmtToEnv), Env(Env), BlockSuccIdx(BlockSuccIdx), - TransferOptions(TransferOptions) {} + int BlockSuccIdx) + : StmtToEnv(StmtToEnv), Env(Env), BlockSuccIdx(BlockSuccIdx) {} void VisitIfStmt(const IfStmt *S) { auto *Cond = S->getCond(); @@ -119,7 +118,7 @@ class TerminatorVisitor : public ConstStmtVisitor { void extendFlowCondition(const Expr &Cond) { // The terminator sub-expression might not be evaluated. if (Env.getStorageLocation(Cond, SkipPast::None) == nullptr) - transfer(StmtToEnv, Cond, Env, TransferOptions); + transfer(StmtToEnv, Cond, Env); // FIXME: The flow condition must be an r-value, so `SkipPast::None` should // suffice. @@ -151,7 +150,6 @@ class TerminatorVisitor : public ConstStmtVisitor { const StmtToEnvMap &StmtToEnv; Environment &Env; int BlockSuccIdx; - TransferOptions TransferOptions; }; /// Computes the input state for a given basic block by joining the output @@ -219,8 +217,7 @@ static TypeErasedDataflowAnalysisState computeBlockInputState( if (const Stmt *PredTerminatorStmt = Pred->getTerminatorStmt()) { const StmtToEnvMapImpl StmtToEnv(CFCtx, BlockStates); TerminatorVisitor(StmtToEnv, PredState.Env, - blockIndexInPredecessor(*Pred, Block), - Analysis.builtinTransferOptions()) + blockIndexInPredecessor(*Pred, Block)) .Visit(PredTerminatorStmt); } } @@ -256,8 +253,7 @@ static void transferCFGStmt( assert(S != nullptr); if (Analysis.applyBuiltinTransfer()) - transfer(StmtToEnvMapImpl(CFCtx, BlockStates), *S, State.Env, - Analysis.builtinTransferOptions()); + transfer(StmtToEnvMapImpl(CFCtx, BlockStates), *S, State.Env); Analysis.transferTypeErased(S, State.Lattice, State.Env); if (HandleTransferredStmt != nullptr) diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index 41e4252669bd9d..bf3aab10eabf38 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -39,14 +39,14 @@ using ::testing::SizeIs; template void runDataflow(llvm::StringRef Code, Matcher Match, - DataflowAnalysisOptions Options, - LangStandard::Kind Std = LangStandard::lang_cxx17, - llvm::StringRef TargetFun = "target") { + LangStandard::Kind Std = LangStandard::lang_cxx17, + bool ApplyBuiltinTransfer = true, + llvm::StringRef TargetFun = "target") { ASSERT_THAT_ERROR( test::checkDataflow( Code, TargetFun, - [Options](ASTContext &C, Environment &) { - return NoopAnalysis(C, Options); + [ApplyBuiltinTransfer](ASTContext &C, Environment &) { + return NoopAnalysis(C, ApplyBuiltinTransfer); }, [&Match]( llvm::ArrayRef< @@ -54,19 +54,12 @@ void runDataflow(llvm::StringRef Code, Matcher Match, Results, ASTContext &ASTCtx) { Match(Results, ASTCtx); }, {"-fsyntax-only", "-fno-delayed-template-parsing", - "-std=" + std::string( - LangStandard::getLangStandardForKind(Std).getName())}), + "-std=" + + std::string( + LangStandard::getLangStandardForKind(Std).getName())}), llvm::Succeeded()); } -template -void runDataflow(llvm::StringRef Code, Matcher Match, - LangStandard::Kind Std = LangStandard::lang_cxx17, - bool ApplyBuiltinTransfer = true, - llvm::StringRef TargetFun = "target") { - runDataflow(Code, Match, {ApplyBuiltinTransfer, {}}, Std, TargetFun); -} - TEST(TransferTest, IntVarDeclNotTrackedWhenTransferDisabled) { std::string Code = R"( void target() { @@ -3869,95 +3862,4 @@ TEST(TransferTest, ForStmtBranchWithoutConditionDoesNotExtendFlowCondition) { }); } -TEST(TransferTest, ContextSensitiveOptionDisabled) { - std::string Code = R"( - bool GiveBool(); - void SetBool(bool &Var) { Var = true; } - - void target() { - bool Foo = GiveBool(); - SetBool(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()); - - auto &FooVal = - *cast(Env.getValue(*FooDecl, SkipPast::None)); - EXPECT_FALSE(Env.flowConditionImplies(FooVal)); - EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(FooVal))); - }, - {/*.ApplyBuiltinTransfer=*/true, - /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/false}}); -} - -TEST(TransferTest, ContextSensitiveSetTrue) { - std::string Code = R"( - bool GiveBool(); - void SetBool(bool &Var) { Var = true; } - - void target() { - bool Foo = GiveBool(); - SetBool(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()); - - auto &FooVal = - *cast(Env.getValue(*FooDecl, SkipPast::None)); - EXPECT_TRUE(Env.flowConditionImplies(FooVal)); - }, - {/*.ApplyBuiltinTransfer=*/true, - /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}}); -} - -TEST(TransferTest, ContextSensitiveSetFalse) { - std::string Code = R"( - bool GiveBool(); - void SetBool(bool &Var) { Var = false; } - - void target() { - bool Foo = GiveBool(); - SetBool(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()); - - auto &FooVal = - *cast(Env.getValue(*FooDecl, SkipPast::None)); - EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(FooVal))); - }, - {/*.ApplyBuiltinTransfer=*/true, - /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}}); -} - } // namespace