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 @@ -107,6 +107,20 @@ return ThisPointeeLoc; } + /// Assigns `Loc` as the storage location of the return value. + /// + /// Requirements: + /// + /// `return` must not be assigned a storage location. + void setReturnStorageLocation(StorageLocation &Loc) { + assert(ReturnLoc == nullptr); + ReturnLoc = &Loc; + } + + /// Returns the storage location of the returned value or null if it was not + /// set yet. + StorageLocation *getReturnStorageLocation() const { return ReturnLoc; } + private: // Maps from program declarations and statements to storage locations that are // assigned to them. These assignments are global (aggregated across all basic @@ -117,6 +131,7 @@ llvm::DenseMap ExprToLoc; StorageLocation *ThisPointeeLoc = nullptr; + StorageLocation *ReturnLoc = nullptr; }; /// Owns objects that encompass the state of a program and stores context that 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 @@ -221,6 +221,10 @@ /// in the environment. StorageLocation *getThisPointeeStorageLocation() const; + /// Returns the storage location of the return value or null if it was not set + /// yet. + StorageLocation *getReturnStorageLocation() const; + /// Returns a pointer value that represents a null pointer. Calls with /// `PointeeType` that are canonically equivalent will return the same result. PointerValue &getOrCreateNullPointerValue(QualType PointeeType); diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -204,6 +204,16 @@ Environment Env(*this); Env.CallStr.push_back(Call); + const auto *FuncDecl = Call->getDirectCallee(); + assert(FuncDecl != nullptr); + + auto *ReturnLoc = DACtx->getFrame(Env.CallStr).getReturnStorageLocation(); + if (ReturnLoc == nullptr) { + QualType ReturnType = FuncDecl->getReturnType(); + DACtx->getFrame(Env.CallStr) + .setReturnStorageLocation(Env.createStorageLocation(ReturnType)); + } + // 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 @@ -212,8 +222,6 @@ // 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 @@ -380,6 +388,10 @@ return DACtx->getFrame(CallStr).getThisPointeeStorageLocation(); } +StorageLocation *Environment::getReturnStorageLocation() const { + return DACtx->getFrame(CallStr).getReturnStorageLocation(); +} + PointerValue &Environment::getOrCreateNullPointerValue(QualType PointeeType) { return DACtx->getOrCreateNullPointerValue(PointeeType); } 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 @@ -332,6 +332,22 @@ std::make_unique(*ThisPointeeLoc))); } + void VisitReturnStmt(const ReturnStmt *S) { + auto *Loc = Env.getReturnStorageLocation(); + if (Loc == nullptr) { + // The outermost context does not set a storage location for `return`, so + // in that case we just ignore `return` statements. + return; + } + + auto *Ret = S->getRetValue(); + assert(Ret != nullptr); + auto *Val = Env.getValue(*Ret, SkipPast::Reference); + assert(Val != nullptr); + // FIXME: Model NRVO. + Env.setValue(*Loc, *Val); + } + void VisitMemberExpr(const MemberExpr *S) { ValueDecl *Member = S->getMemberDecl(); assert(Member != nullptr); @@ -534,7 +550,11 @@ auto ExitState = (*BlockToOutputState)[ExitBlock]; assert(ExitState); - Env.popCall(ExitState->Env); + auto &ExitEnv = ExitState->Env; + auto *ReturnLoc = ExitEnv.getReturnStorageLocation(); + assert(ReturnLoc != nullptr); + Env.setStorageLocation(*S, *ReturnLoc); + Env.popCall(ExitEnv); } } 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 @@ -4001,4 +4001,60 @@ /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}}); } +TEST(TransferTest, ContextSensitiveReturnTrue) { + std::string Code = R"( + bool GiveBool() { return true; } + + void target() { + bool Foo = GiveBool(); + // [[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, ContextSensitiveReturnFalse) { + std::string Code = R"( + bool GiveBool() { return false; } + + void target() { + bool Foo = GiveBool(); + // [[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