diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h index 8c169005846ef..1b154010bf365 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h @@ -135,7 +135,7 @@ class Environment { /// /// Requirements: /// - /// The callee of `Call` must be a `FunctionDecl`. + /// The callee of `Call` must be a `FunctionDecl` with a body. /// /// The body of the callee must not reference globals. /// @@ -143,7 +143,6 @@ class Environment { /// /// Each argument of `Call` must already have a `StorageLocation`. Environment pushCall(const CallExpr *Call) const; - Environment pushCall(const CXXConstructExpr *Call) const; /// Moves gathered information back into `this` from a `CalleeEnv` created via /// `pushCall`. @@ -382,12 +381,6 @@ class Environment { StorageLocation &skip(StorageLocation &Loc, SkipPast SP) const; const StorageLocation &skip(const StorageLocation &Loc, SkipPast SP) const; - /// Shared implementation of `pushCall` overloads. Note that unlike - /// `pushCall`, this member is invoked on the environment of the callee, not - /// of the caller. - void pushCallInternal(const FunctionDecl *FuncDecl, - ArrayRef Args); - // `DACtx` is not null and not owned by this object. DataflowAnalysisContext *DACtx; diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp index e4af68e53e14e..16c83cad9d9e3 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -207,68 +207,52 @@ Environment::Environment(DataflowAnalysisContext &DACtx, Environment Environment::pushCall(const CallExpr *Call) const { Environment Env(*this); - // FIXME: Support references here. - Env.ReturnLoc = getStorageLocation(*Call, SkipPast::Reference); - - if (const auto *MethodCall = dyn_cast(Call)) { - if (const Expr *Arg = MethodCall->getImplicitObjectArgument()) { - Env.ThisPointeeLoc = getStorageLocation(*Arg, SkipPast::Reference); - } - } - - Env.pushCallInternal(Call->getDirectCallee(), - llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs())); + Env.ReturnLoc = Env.getStorageLocation(*Call, SkipPast::Reference); - return Env; -} - -Environment Environment::pushCall(const CXXConstructExpr *Call) const { - Environment Env(*this); - - // FIXME: Support references here. - Env.ReturnLoc = getStorageLocation(*Call, SkipPast::Reference); + const auto *FuncDecl = Call->getDirectCallee(); + assert(FuncDecl != nullptr); - Env.ThisPointeeLoc = Env.ReturnLoc; - - Env.pushCallInternal(Call->getConstructor(), - llvm::makeArrayRef(Call->getArgs(), Call->getNumArgs())); - - return Env; -} - -void Environment::pushCallInternal(const FunctionDecl *FuncDecl, - ArrayRef Args) { - setDeclCtx(FuncDecl); + Env.setDeclCtx(FuncDecl); // FIXME: In order to allow the callee to reference globals, we probably need // to call `initGlobalVars` here in some way. + if (const auto *MethodCall = dyn_cast(Call)) { + if (const Expr *Arg = MethodCall->getImplicitObjectArgument()) { + Env.ThisPointeeLoc = Env.getStorageLocation(*Arg, SkipPast::Reference); + } + } + auto ParamIt = FuncDecl->param_begin(); + 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 (unsigned ArgIndex = 0; ArgIndex < Args.size(); ++ParamIt, ++ArgIndex) { + for (; ArgIt != ArgEnd; ++ParamIt, ++ArgIt) { assert(ParamIt != FuncDecl->param_end()); - const Expr *Arg = Args[ArgIndex]; - auto *ArgLoc = getStorageLocation(*Arg, SkipPast::Reference); + const Expr *Arg = *ArgIt; + auto *ArgLoc = Env.getStorageLocation(*Arg, SkipPast::Reference); assert(ArgLoc != nullptr); const VarDecl *Param = *ParamIt; - auto &Loc = createStorageLocation(*Param); - setStorageLocation(*Param, Loc); + auto &Loc = Env.createStorageLocation(*Param); + Env.setStorageLocation(*Param, Loc); QualType ParamType = Param->getType(); if (ParamType->isReferenceType()) { - auto &Val = takeOwnership(std::make_unique(*ArgLoc)); - setValue(Loc, Val); - } else if (auto *ArgVal = getValue(*ArgLoc)) { - setValue(Loc, *ArgVal); - } else if (Value *Val = createValue(ParamType)) { - setValue(Loc, *Val); + auto &Val = Env.takeOwnership(std::make_unique(*ArgLoc)); + Env.setValue(Loc, Val); + } else if (auto *ArgVal = Env.getValue(*ArgLoc)) { + Env.setValue(Loc, *ArgVal); + } else if (Value *Val = Env.createValue(ParamType)) { + Env.setValue(Loc, *Val); } } + + return Env; } void Environment::popCall(const Environment &CalleeEnv) { diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index 2f05b08fe9bc2..430dc8d5faa97 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -444,8 +444,6 @@ class TransferVisitor : public ConstStmtVisitor { Env.setStorageLocation(*S, Loc); if (Value *Val = Env.createValue(S->getType())) Env.setValue(Loc, *Val); - - transferInlineCall(S, ConstructorDecl); } void VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *S) { @@ -528,7 +526,45 @@ class TransferVisitor : public ConstStmtVisitor { return; Env.setStorageLocation(*S, *ArgLoc); } else if (const FunctionDecl *F = S->getDirectCallee()) { - transferInlineCall(S, F); + // This case is for context-sensitive analysis. + if (!Options.ContextSensitive) + return; + + const ControlFlowContext *CFCtx = Env.getControlFlowContext(F); + if (!CFCtx) + return; + + // FIXME: We don't support context-sensitive analysis of recursion, so + // we should return early here if `F` is the same as the `FunctionDecl` + // holding `S` itself. + + auto ExitBlock = CFCtx->getCFG().getExit().getBlockID(); + + // Note that it is important for the storage location of `S` to be set + // before `pushCall`, because the latter uses it to set the storage + // location for `return`. + auto &ReturnLoc = Env.createStorageLocation(*S); + Env.setStorageLocation(*S, ReturnLoc); + auto CalleeEnv = Env.pushCall(S); + + // FIXME: Use the same analysis as the caller for the callee. Note, + // though, that doing so would require support for changing the analysis's + // ASTContext. + assert( + CFCtx->getDecl() != nullptr && + "ControlFlowContexts in the environment should always carry a decl"); + auto Analysis = NoopAnalysis(CFCtx->getDecl()->getASTContext(), + DataflowAnalysisOptions()); + + auto BlockToOutputState = + dataflow::runDataflowAnalysis(*CFCtx, Analysis, CalleeEnv); + assert(BlockToOutputState); + assert(ExitBlock < BlockToOutputState->size()); + + auto ExitState = (*BlockToOutputState)[ExitBlock]; + assert(ExitState); + + Env.popCall(ExitState->Env); } } @@ -657,51 +693,6 @@ class TransferVisitor : public ConstStmtVisitor { return Env.makeAtomicBoolValue(); } - // If context sensitivity is enabled, try to analyze the body of the callee - // `F` of `S`. The type `E` must be either `CallExpr` or `CXXConstructExpr`. - template - void transferInlineCall(const E *S, const FunctionDecl *F) { - if (!Options.ContextSensitive) - return; - - const ControlFlowContext *CFCtx = Env.getControlFlowContext(F); - if (!CFCtx) - return; - - // FIXME: We don't support context-sensitive analysis of recursion, so - // we should return early here if `F` is the same as the `FunctionDecl` - // holding `S` itself. - - auto ExitBlock = CFCtx->getCFG().getExit().getBlockID(); - - if (const auto *NonConstructExpr = dyn_cast(S)) { - // Note that it is important for the storage location of `S` to be set - // before `pushCall`, because the latter uses it to set the storage - // location for `return`. - auto &ReturnLoc = Env.createStorageLocation(*S); - Env.setStorageLocation(*S, ReturnLoc); - } - auto CalleeEnv = Env.pushCall(S); - - // FIXME: Use the same analysis as the caller for the callee. Note, - // though, that doing so would require support for changing the analysis's - // ASTContext. - assert(CFCtx->getDecl() != nullptr && - "ControlFlowContexts in the environment should always carry a decl"); - auto Analysis = NoopAnalysis(CFCtx->getDecl()->getASTContext(), - DataflowAnalysisOptions()); - - auto BlockToOutputState = - dataflow::runDataflowAnalysis(*CFCtx, Analysis, CalleeEnv); - assert(BlockToOutputState); - assert(ExitBlock < BlockToOutputState->size()); - - auto ExitState = (*BlockToOutputState)[ExitBlock]; - assert(ExitState); - - Env.popCall(ExitState->Env); - } - const StmtToEnvMap &StmtToEnv; Environment &Env; TransferOptions Options; diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index af06021abccfd..ebca4b3ea7f15 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -4368,106 +4368,4 @@ TEST(TransferTest, ContextSensitiveMethodGetterAndSetter) { /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}}); } -TEST(TransferTest, ContextSensitiveConstructorBody) { - std::string Code = R"( - class MyClass { - public: - MyClass() { MyField = true; } - - bool MyField; - }; - - void target() { - MyClass MyObj; - bool Foo = MyObj.MyField; - // [[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, ContextSensitiveConstructorInitializer) { - std::string Code = R"( - class MyClass { - public: - MyClass() : MyField(true) {} - - bool MyField; - }; - - void target() { - MyClass MyObj; - bool Foo = MyObj.MyField; - // [[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, ContextSensitiveConstructorDefault) { - std::string Code = R"( - class MyClass { - public: - MyClass() = default; - - bool MyField = true; - }; - - void target() { - MyClass MyObj; - bool Foo = MyObj.MyField; - // [[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}}); -} - } // namespace